Regression fix multi table typing (#468)

packages/dash-table/demo/App.tsx

@@ -0,0 +1,84 @@
+/* eslint no-magic-numbers: 0 */
+import * as R from 'ramda';
+import React, {Component} from 'react';
+import { DataTable } from 'dash-table/index';
+import Environment from 'core/environment';
+import { memoizeOne } from 'core/memoizer';
+import Logger from 'core/Logger';
+import AppState, { AppMode } from './AppMode';
+import memoizerCache from 'core/cache/memoizer';
+
+import './style.less';
+
+class App extends Component<any, any> {
+    constructor(props: any) {
+        super(props);
+
+        this.state = {
+            ...AppState,
+            temp_filtering: ''
+        };
+    }
+
+    renderMode() {
+        const mode = Environment.searchParams.get('mode');
+
+        if (mode === AppMode.Filtering) {
+            return (<div>
+                <button
+                    className='clear-filters'
+                    onClick={() => {
+                        const tableProps = R.clone(this.state.tableProps);
+                        tableProps.filter_query = '';
+
+                        this.setState({ tableProps });
+                    }}
+                >Clear Filter</button>
+                <input
+                    style={{ width: '500px' }}
+                    value={this.state.temp_filtering}
+                    onChange={
+                        e => this.setState({ temp_filtering: e.target.value })
+                    }
+                    onBlur={e => {
+                        const tableProps = R.clone(this.state.tableProps);
+                        tableProps.filter_query = e.target.value;
+
+                        this.setState({ tableProps });
+                    }} />
+            </div>);
+        } else if (mode === AppMode.TaleOfTwoTables) {
+            const props: any = {};
+            Object.entries(this.state.tableProps).forEach(([key, value]) => {
+                props[key] = this.propCache.get(key)(value);
+            });
+
+            return (<DataTable
+                {...props}
+            />);
+        }
+    }
+
+    render() {
+        return (<div>
+            {this.renderMode()}
+            <DataTable
+                setProps={this.setProps()}
+                {...this.state.tableProps}
+            />
+        </div>);
+    }
+
+    private propCache = memoizerCache<[string]>()(R.clone);
+
+    private setProps = memoizeOne(() => {
+        return (newProps: any) => {
+            Logger.debug('--->', newProps);
+            this.setState((prevState: any) => ({
+                tableProps: R.merge(prevState.tableProps, newProps)
+            }));
+        };
+    });
+}
+
+export default App;

packages/dash-table/demo/AppMode.ts

@@ -22,6 +22,7 @@ export enum AppMode {
     Formatting = 'formatting',
     ReadOnly = 'readonly',
     ColumnsInSpace = 'columnsInSpace',
+    TaleOfTwoTables = 'taleOfTwoTables',
     Tooltips = 'tooltips',
     Typed = 'typed',
     Virtualized = 'virtualized'
@@ -335,6 +336,7 @@ function getState() {
             return getVirtualizedState();
         case AppMode.Typed:
             return getTypedState();
+        case AppMode.TaleOfTwoTables:
         case AppMode.Default:
         default:
             return getDefaultState();

packages/dash-table/src/dash-table/dash/DataTable.js

@@ -8,21 +8,22 @@ import Logger from 'core/Logger';
 
 import genRandomId from 'dash-table/utils/generate';
 import isValidProps from './validate';
-import sanitizeProps from './sanitize';
+import Sanitizer from './Sanitizer';
 
 export default class DataTable extends Component {
     constructor(props) {
         super(props);
         let id;
         this.getId = () => (id = id || genRandomId('table-'));
+        this.sanitizer = new Sanitizer();
     }
 
     render() {
         if (!isValidProps(this.props)) {
             return (<div>Invalid props combination</div>);
         }
 
-        const sanitizedProps = sanitizeProps(this.props);
+        const sanitizedProps = this.sanitizer.sanitize(this.props);
         return this.props.id ?
             (<RealTable {...sanitizedProps} />) :
             (<RealTable {...sanitizedProps} id={this.getId()} />);

packages/dash-table/src/dash-table/dash/sanitize.ts → packages/dash-table/src/dash-table/dash/Sanitizer.ts

@@ -29,23 +29,6 @@ const D3_DEFAULT_LOCALE: INumberLocale = {
 const DEFAULT_NULLY = '';
 const DEFAULT_SPECIFIER = '';
 
-const applyDefaultToLocale = memoizeOne((locale: INumberLocale) => getLocale(locale));
-
-const applyDefaultsToColumns = memoizeOne(
-    (defaultLocale: INumberLocale, defaultSort: SortAsNull, columns: Columns, editable: boolean ) => R.map(column => {
-        const c = R.clone(column);
-        c.editable = isEditable(editable, column.editable);
-        c.sort_as_null = c.sort_as_null || defaultSort;
-
-        if (c.type === ColumnType.Numeric && c.format) {
-            c.format.locale = getLocale(defaultLocale, c.format.locale);
-            c.format.nully = getNully(c.format.nully);
-            c.format.specifier = getSpecifier(c.format.specifier);
-        }
-        return c;
-    }, columns)
-);
-
 const data2number = (data?: any) => +data || 0;
 
 const getFixedColumns = (
@@ -64,16 +47,37 @@ const getFixedRows = (
         0 :
         headerRows(columns) + (filter_action !== TableAction.None ? 1 : 0) + data2number(fixed.data);
 
-export default (props: PropsWithDefaults): SanitizedProps => {
-    const locale_format = applyDefaultToLocale(props.locale_format);
+const applyDefaultsToColumns = (defaultLocale: INumberLocale, defaultSort: SortAsNull, columns: Columns, editable: boolean) => R.map(column => {
+    const c = R.clone(column);
+    c.editable = isEditable(editable, column.editable);
+    c.sort_as_null = c.sort_as_null || defaultSort;
 
-    return R.merge(props, {
-        columns: applyDefaultsToColumns(locale_format, props.sort_as_null, props.columns, props.editable),
-        fixed_columns: getFixedColumns(props.fixed_columns, props.row_deletable, props.row_selectable),
-        fixed_rows: getFixedRows(props.fixed_rows, props.columns, props.filter_action),
-        locale_format
-    });
-};
+    if (c.type === ColumnType.Numeric && c.format) {
+        c.format.locale = getLocale(defaultLocale, c.format.locale);
+        c.format.nully = getNully(c.format.nully);
+        c.format.specifier = getSpecifier(c.format.specifier);
+    }
+    return c;
+}, columns);
+
+const applyDefaultToLocale = (locale: INumberLocale) => getLocale(locale);
+
+export default class Sanitizer {
+    sanitize(props: PropsWithDefaults): SanitizedProps {
+        const locale_format = this.applyDefaultToLocale(props.locale_format);
+
+        return R.merge(props, {
+            columns: this.applyDefaultsToColumns(locale_format, props.sort_as_null, props.columns, props.editable),
+            fixed_columns: getFixedColumns(props.fixed_columns, props.row_deletable, props.row_selectable),
+            fixed_rows: getFixedRows(props.fixed_rows, props.columns, props.filter_action),
+            locale_format
+        });
+    }
+
+    private readonly applyDefaultToLocale = memoizeOne(applyDefaultToLocale);
+
+    private readonly applyDefaultsToColumns = memoizeOne(applyDefaultsToColumns);
+}
 
 export const getLocale = (...locales: Partial<INumberLocale>[]): INumberLocale =>
     R.mergeAll([

packages/dash-table/tests/cypress/tests/standalone/edit_cell_test.ts

@@ -4,6 +4,43 @@ import Key from 'cypress/Key';
 
 import { AppMode, ReadWriteModes } from 'demo/AppMode';
 
+Object.values([
+    ...ReadWriteModes,
+    AppMode.TaleOfTwoTables
+]).forEach(mode => {
+    describe(`edit, mode=${mode}`, () => {
+        beforeEach(() => {
+            cy.visit(`http://localhost:8080?mode=${mode}`);
+            DashTable.toggleScroll(false);
+        });
+
+        // Case: "Pressing enter to confirm change does not work on the last row"
+        // Issue: https://github.com/plotly/dash-table/issues/50
+        it('can edit on "enter"', () => {
+            DashTable.getCell(0, 1).click();
+            // Case: 2+ tables results in infinite rendering loop b/c of shared cache
+            // Issue: https://github.com/plotly/dash-table/pull/468
+            //
+            // Artificial delay added to make sure re-rendering has time to occur.
+            cy.wait(1000);
+            DOM.focused.type(`abc${Key.Enter}`);
+            DashTable.getCell(0, 1).within(() => cy.get('.dash-cell-value').should('have.html', `abc`));
+        });
+
+        it('can edit when clicking outside of cell', () => {
+            DashTable.getCell(0, 1).click();
+            DOM.focused.type(`abc`);
+            // Case: 2+ tables results in infinite rendering loop b/c of shared cache
+            // Issue: https://github.com/plotly/dash-table/pull/468
+            //
+            // Artificial delay added to make sure re-rendering has time to occur.
+            cy.wait(1000);
+            DashTable.getCell(0, 0).click();
+            DashTable.getCell(0, 1).within(() => cy.get('.dash-cell-value').should('have.html', `abc`));
+        });
+    });
+});
+
 Object.values(ReadWriteModes).forEach(mode => {
     describe(`edit, mode=${mode}`, () => {
         beforeEach(() => {
@@ -106,20 +143,6 @@ Object.values(ReadWriteModes).forEach(mode => {
                 cy.get('.Select-value-label').should('have.html', expectedValue);
             });
         });
-
-        // https://github.com/plotly/dash-table/issues/50
-        it('can edit on "enter"', () => {
-            DashTable.getCell(0, 1).click();
-            DOM.focused.type(`abc${Key.Enter}`);
-            DashTable.getCell(0, 1).within(() => cy.get('.dash-cell-value').should('have.html', `abc`));
-        });
-
-        it('can edit when clicking outside of cell', () => {
-            DashTable.getCell(0, 1).click();
-            DOM.focused.type(`abc`);
-            DashTable.getCell(0, 0).click();
-            DashTable.getCell(0, 1).within(() => cy.get('.dash-cell-value').should('have.html', `abc`));
-        });
     });
 });
 

packages/dash-table/tests/cypress/tests/unit/formatting_test.ts

@@ -1,5 +1,5 @@
 import { getFormatter } from 'dash-table/type/number';
-import { getLocale, getNully, getSpecifier } from 'dash-table/dash/sanitize';
+import { getLocale, getNully, getSpecifier } from 'dash-table/dash/Sanitizer';
 
 describe('formatting', () => {
     describe('number', () => {