From 2351369a3092603cbba5bb15b6ab8597e71d9604 Mon Sep 17 00:00:00 2001
From: Jesse Yang <jesse.yang@airbnb.com>
Date: Fri, 6 Mar 2020 19:32:10 -0800
Subject: [PATCH 1/9] fix(legacy-table): avoid React DOM

`jquery.datatables` will manipulate DOMs, sometimes remove them. In case
of component being reloaded with updated props, React will not be able
to find those removed nodes, causing a `cannot removeChild` error.

Because of the the way to assign row keys, if table shape changes (add
or remove columns), React may also have difficulty match the cached
nodes via keys.

In general it's a bad idea to directly manipulate React rendered DOM
nodes, so we better just let `jquery.datatables` handle everything.

In the future, once we removed `jquery.datatables`, a pure React
component will not have such issues.
---
 .../src/ReactDataTable.tsx                    | 96 ++++++++++---------
 .../legacy-plugin-chart-table/Stories.tsx     | 75 +++++++++++----
 2 files changed, 112 insertions(+), 59 deletions(-)

diff --git a/packages/superset-ui-legacy-plugin-chart-table/src/ReactDataTable.tsx b/packages/superset-ui-legacy-plugin-chart-table/src/ReactDataTable.tsx
index e36d76709..e95d99095 100644
--- a/packages/superset-ui-legacy-plugin-chart-table/src/ReactDataTable.tsx
+++ b/packages/superset-ui-legacy-plugin-chart-table/src/ReactDataTable.tsx
@@ -18,6 +18,7 @@
  */
 import { t } from '@superset-ui/translation';
 import React, { useEffect, createRef } from 'react';
+import ReactDOMServer from 'react-dom/server';
 import { formatNumber, NumberFormats } from '@superset-ui/number-format';
 import { getTimeFormatter } from '@superset-ui/time-format';
 import { filterXSS } from 'xss';
@@ -202,49 +203,58 @@ export default function ReactDataTable(props: DataTableProps) {
     };
   });
 
-  return (
-    <div ref={rootElem} className="superset-legacy-chart-table">
-      <table className="table table-striped table-condensed table-hover">
-        <thead>
-          <tr>
-            {columns.map(col => (
-              // by default all columns will have sorting
-              <th key={col.key} className="sorting" title={col.label}>
-                {col.label}
-              </th>
-            ))}
-          </tr>
-        </thead>
-        <tbody>
-          {data.map((record, i) => (
-            // hide rows after first page makes the initial render faster (less layout computation)
-            // eslint-disable-next-line react/no-array-index-key
-            <tr key={i} style={{ display: pageLength > 0 && i >= pageLength ? 'none' : undefined }}>
-              {columns.map(({ key, format }) => {
-                const val = record[key];
-                const keyIsMetric = metricsSet.has(key);
-                const text = cellText(key, format, val);
-                const isHtml = !keyIsMetric && isProbablyHTML(text);
-                return (
-                  <td
-                    key={key}
-                    // only set innerHTML for actual html content, this saves time
-                    dangerouslySetInnerHTML={isHtml ? { __html: text } : undefined}
-                    data-sort={val}
-                    className={keyIsMetric ? 'text-right' : ''}
-                    style={{
-                      backgroundImage: keyIsMetric ? cellBar(key, val as number) : undefined,
-                    }}
-                    title={keyIsMetric || percentMetricsSet.has(key) ? (val as string) : ''}
-                  >
-                    {isHtml ? null : text}
-                  </td>
-                );
-              })}
-            </tr>
+  const tableElement = (
+    <table className="table table-striped table-condensed table-hover">
+      <thead>
+        <tr>
+          {columns.map(col => (
+            // by default all columns will have sorting
+            <th key={col.key} className="sorting" title={col.label}>
+              {col.label}
+            </th>
           ))}
-        </tbody>
-      </table>
-    </div>
+        </tr>
+      </thead>
+      <tbody>
+        {data.map((record, i) => (
+          <tr
+            // eslint-disable-next-line react/no-array-index-key
+            key={i}
+            // hide rows after first page makes the initial render faster (less layout computation)
+            style={{ display: pageLength > 0 && i >= pageLength ? 'none' : undefined }}
+          >
+            {columns.map(({ key, format }) => {
+              const val = record[key];
+              const keyIsMetric = metricsSet.has(key);
+              const text = cellText(key, format, val);
+              const isHtml = !keyIsMetric && isProbablyHTML(text);
+              return (
+                <td
+                  key={key}
+                  // only set innerHTML for actual html content, this saves time
+                  dangerouslySetInnerHTML={isHtml ? { __html: text } : undefined}
+                  data-sort={val}
+                  className={keyIsMetric ? 'text-right' : ''}
+                  style={{
+                    backgroundImage: keyIsMetric ? cellBar(key, val as number) : undefined,
+                  }}
+                  title={keyIsMetric || percentMetricsSet.has(key) ? (val as string) : ''}
+                >
+                  {isHtml ? null : text}
+                </td>
+              );
+            })}
+          </tr>
+        ))}
+      </tbody>
+    </table>
+  );
+
+  return (
+    <div
+      ref={rootElem}
+      className="superset-legacy-chart-table"
+      dangerouslySetInnerHTML={{ __html: ReactDOMServer.renderToStaticMarkup(tableElement) }}
+    />
   );
 }
diff --git a/packages/superset-ui-plugins-demo/storybook/stories/legacy-plugin-chart-table/Stories.tsx b/packages/superset-ui-plugins-demo/storybook/stories/legacy-plugin-chart-table/Stories.tsx
index c0869ec21..eb60a29b8 100644
--- a/packages/superset-ui-plugins-demo/storybook/stories/legacy-plugin-chart-table/Stories.tsx
+++ b/packages/superset-ui-plugins-demo/storybook/stories/legacy-plugin-chart-table/Stories.tsx
@@ -25,13 +25,32 @@ function paginated(props: SuperChartProps, pageSize = 50) {
   };
 }
 
+function adjustNumCols(props: SuperChartProps, numCols = 7) {
+  const newProps = { ...props };
+  if (props.queryData) {
+    const { columns } = props.queryData.data;
+    const curSize = columns.length;
+    const newColumns = [...Array(numCols)].map((_, i) => {
+      return columns[i % curSize];
+    });
+    newProps.queryData = {
+      ...props.queryData,
+      data: {
+        ...props.queryData.data,
+        columns: newColumns,
+      },
+    };
+  }
+  return newProps;
+}
+
 /**
  * Load sample data for testing
  * @param props the original props passed to SuperChart
  * @param pageSize number of records perpage
  * @param targetSize the target total number of records
  */
-function loadData(props: SuperChartProps, pageSize = 50, targetSize = 10042) {
+function loadData(props: SuperChartProps, pageSize = 50, targetSize = 5042) {
   if (!props.queryData) return props;
   const data = props.queryData && props.queryData.data;
   if (data.records.length > 0) {
@@ -79,27 +98,51 @@ export default [
   },
   {
     renderStory() {
-      const [chartProps, setChartProps] = useState(loadData(birthNames));
+      const initialProps = loadData(birthNames);
+      const [chartProps, setChartProps] = useState(initialProps);
+
       const updatePageSize = (size: number) => {
-        setChartProps(paginated(chartProps, size));
+        setChartProps(paginated(initialProps, size));
+      };
+      const updateNumCols = (numCols: number) => {
+        setChartProps(adjustNumCols(initialProps, numCols));
       };
+
       return (
         <div className="superset-body">
           <div className="panel">
             <div className="panel-heading form-inline">
-              Initial page size:{' '}
-              <div className="btn-group">
-                {[10, 25, 40, 50, 100, -1].map(pageSize => {
-                  return (
-                    <button
-                      key={pageSize}
-                      className="btn btn-default"
-                      onClick={() => updatePageSize(pageSize)}
-                    >
-                      {pageSize > 0 ? pageSize : 'All'}
-                    </button>
-                  );
-                })}
+              <div className="form-group">
+                Initial page size:{' '}
+                <div className="btn-group btn-group-sm">
+                  {[10, 25, 40, 50, 100, -1].map(pageSize => {
+                    return (
+                      <button
+                        key={pageSize}
+                        className="btn btn-default"
+                        onClick={() => updatePageSize(pageSize)}
+                      >
+                        {pageSize > 0 ? pageSize : 'All'}
+                      </button>
+                    );
+                  })}
+                </div>
+              </div>
+              <div className="form-group" style={{ marginLeft: 20 }}>
+                Number of columns:{' '}
+                <div className="btn-group btn-group-sm">
+                  {[1, 3, 5, 7, 9].map(numCols => {
+                    return (
+                      <button
+                        key={numCols}
+                        className="btn btn-default"
+                        onClick={() => updateNumCols(numCols)}
+                      >
+                        {numCols}
+                      </button>
+                    );
+                  })}
+                </div>
               </div>
             </div>
             <div className="panel-body">

From 623f4cf17f5c2805d8124b85e2f4c2007ef2d697 Mon Sep 17 00:00:00 2001
From: Jesse Yang <jesse.yang@airbnb.com>
Date: Fri, 6 Mar 2020 19:47:38 -0800
Subject: [PATCH 2/9] fix(legacy-table): handle the case when percentMetrics is
 null

---
 .../src/transformProps.ts                                   | 6 ++++--
 .../storybook/stories/legacy-plugin-chart-table/Stories.tsx | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/packages/superset-ui-legacy-plugin-chart-table/src/transformProps.ts b/packages/superset-ui-legacy-plugin-chart-table/src/transformProps.ts
index ab26f3b3e..a095eb437 100644
--- a/packages/superset-ui-legacy-plugin-chart-table/src/transformProps.ts
+++ b/packages/superset-ui-legacy-plugin-chart-table/src/transformProps.ts
@@ -78,9 +78,11 @@ export default function transformProps(chartProps: ChartProps): DataTableProps {
   } = formData;
   const { columnFormats, verboseMap } = datasource;
   const { records, columns: columns_ } = queryData.data;
-  const metrics = metrics_.map(consolidateMetricShape);
+  const metrics = (metrics_ ?? []).map(consolidateMetricShape);
   // percent metrics always starts with a '%' sign.
-  const percentMetrics = percentMetrics_.map(consolidateMetricShape).map((x: string) => `%${x}`);
+  const percentMetrics = (percentMetrics_ ?? [])
+    .map(consolidateMetricShape)
+    .map((x: string) => `%${x}`);
   const columns = columns_.map((key: string) => {
     let label = verboseMap[key] || key;
 
diff --git a/packages/superset-ui-plugins-demo/storybook/stories/legacy-plugin-chart-table/Stories.tsx b/packages/superset-ui-plugins-demo/storybook/stories/legacy-plugin-chart-table/Stories.tsx
index eb60a29b8..46caee75b 100644
--- a/packages/superset-ui-plugins-demo/storybook/stories/legacy-plugin-chart-table/Stories.tsx
+++ b/packages/superset-ui-plugins-demo/storybook/stories/legacy-plugin-chart-table/Stories.tsx
@@ -86,7 +86,7 @@ export default [
           metrics: ['sum__num'],
           orderDesc: true,
           pageLength: 0,
-          percentMetrics: [],
+          percentMetrics: null,
           tableFilter: false,
           tableTimestampFormat: '%Y-%m-%d %H:%M:%S',
           timeseriesLimitMetric: null,

From 0e95808e4481af1d94b141552b4d9caecf410990 Mon Sep 17 00:00:00 2001
From: Jesse Yang <jesse.yang@airbnb.com>
Date: Fri, 6 Mar 2020 21:49:32 -0800
Subject: [PATCH 3/9] fix(legacy-table): linting errors

---
 .../superset-ui-legacy-plugin-chart-table/package.json |  3 ++-
 .../src/ReactDataTable.tsx                             |  2 +-
 scripts/build.js                                       | 10 +++++++---
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/packages/superset-ui-legacy-plugin-chart-table/package.json b/packages/superset-ui-legacy-plugin-chart-table/package.json
index 39971d9b8..5347025d2 100644
--- a/packages/superset-ui-legacy-plugin-chart-table/package.json
+++ b/packages/superset-ui-legacy-plugin-chart-table/package.json
@@ -41,6 +41,7 @@
     "@superset-ui/time-format": "^0.12.0",
     "@superset-ui/translation": "^0.12.0",
     "jquery": "^3.4.1",
-    "react": "^16.8.0"
+    "react": "^16.8.0",
+    "react-dom": "^16.8.0"
   }
 }
diff --git a/packages/superset-ui-legacy-plugin-chart-table/src/ReactDataTable.tsx b/packages/superset-ui-legacy-plugin-chart-table/src/ReactDataTable.tsx
index e95d99095..eb28f3df8 100644
--- a/packages/superset-ui-legacy-plugin-chart-table/src/ReactDataTable.tsx
+++ b/packages/superset-ui-legacy-plugin-chart-table/src/ReactDataTable.tsx
@@ -252,9 +252,9 @@ export default function ReactDataTable(props: DataTableProps) {
 
   return (
     <div
+      dangerouslySetInnerHTML={{ __html: ReactDOMServer.renderToStaticMarkup(tableElement) }}
       ref={rootElem}
       className="superset-legacy-chart-table"
-      dangerouslySetInnerHTML={{ __html: ReactDOMServer.renderToStaticMarkup(tableElement) }}
     />
   );
 }
diff --git a/scripts/build.js b/scripts/build.js
index 008b75f9d..aa63480cc 100644
--- a/scripts/build.js
+++ b/scripts/build.js
@@ -7,14 +7,18 @@ const glob = process.argv[2];
 
 process.env.PATH = `./node_modules/.bin:${process.env.PATH}`;
 
-const run = (cmd, async = false) => {
+const run = (cmd) => {
   console.log(`>> ${cmd}`);
   const [p, ...args] = cmd.split(' ');
-  const runner = async ? spawn : spawnSync;
-  runner(p, args, { stdio: 'inherit' });
+  const runner = spawnSync;
+  const { status } = runner(p, args, { stdio: 'inherit' });
+  if (status !== 0) {
+    process.exit(status);
+  }
 };
 
 if (glob) {
+  run(`eslint --color --quiet packages/${glob}/{src,test}/**/*.{js,jsx,ts,tsx}`);
   run(`nimbus prettier --check --workspaces=\"@superset-ui/${glob}"`);
   run(`nimbus babel --clean --workspaces=\"@superset-ui/${glob}"`);
   run(`nimbus babel --clean --workspaces=\"@superset-ui/${glob}" --esm`);

From a6701b51be99bc4503ab3726452f76b28314ff00 Mon Sep 17 00:00:00 2001
From: Jesse Yang <jesse.yang@airbnb.com>
Date: Sat, 7 Mar 2020 21:29:09 -0800
Subject: [PATCH 4/9] refactor: use nimbus build

---
 scripts/build.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/build.js b/scripts/build.js
index aa63480cc..284babd23 100644
--- a/scripts/build.js
+++ b/scripts/build.js
@@ -18,7 +18,7 @@ const run = (cmd) => {
 };
 
 if (glob) {
-  run(`eslint --color --quiet packages/${glob}/{src,test}/**/*.{js,jsx,ts,tsx}`);
+  run(`nimbus eslint packages/${glob}/{src,test}`);
   run(`nimbus prettier --check --workspaces=\"@superset-ui/${glob}"`);
   run(`nimbus babel --clean --workspaces=\"@superset-ui/${glob}"`);
   run(`nimbus babel --clean --workspaces=\"@superset-ui/${glob}" --esm`);

From cdaf494242fda65de641638db7b0a298cc7718d2 Mon Sep 17 00:00:00 2001
From: Jesse Yang <jesse.yang@airbnb.com>
Date: Sun, 8 Mar 2020 01:23:29 -0800
Subject: [PATCH 5/9] test(legacy-table): add React component tests

---
 package.json                                  |   8 +-
 .../test/ReactDataTable.test.tsx              |  57 ++++++++++
 .../test/test_data.ts                         | 102 ++++++++++++++++++
 scripts/setupJest.js                          |   6 ++
 yarn.lock                                     |  12 +++
 5 files changed, 184 insertions(+), 1 deletion(-)
 create mode 100644 packages/superset-ui-legacy-plugin-chart-table/test/ReactDataTable.test.tsx
 create mode 100644 packages/superset-ui-legacy-plugin-chart-table/test/test_data.ts
 create mode 100644 scripts/setupJest.js

diff --git a/package.json b/package.json
index adf2c982c..096748b0f 100644
--- a/package.json
+++ b/package.json
@@ -62,6 +62,7 @@
     "fast-glob": "^3.0.1",
     "fs-extra": "^8.0.1",
     "husky": "^4.2.1",
+    "identity-obj-proxy": "^3.0.0",
     "jest-mock-console": "^1.0.0",
     "lerna": "^3.15.0",
     "lint-staged": "^10.0.7",
@@ -102,7 +103,8 @@
     "jest": {
       "timers": "real",
       "setupFilesAfterEnv": [
-        "@airbnb/config-jest/enzyme"
+        "@airbnb/config-jest/enzyme",
+        "./scripts/setupJest.js"
       ],
       "coverageThreshold": {
         "global": {
@@ -111,6 +113,10 @@
           "lines": 1,
           "statements": 1
         }
+      },
+      "moduleNameMapper": {
+        "\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$": "<rootDir>/__mocks__/fileMock.js",
+        "\\.(css|less)$": "identity-obj-proxy"
       }
     },
     "eslint": {
diff --git a/packages/superset-ui-legacy-plugin-chart-table/test/ReactDataTable.test.tsx b/packages/superset-ui-legacy-plugin-chart-table/test/ReactDataTable.test.tsx
new file mode 100644
index 000000000..1b64eef7c
--- /dev/null
+++ b/packages/superset-ui-legacy-plugin-chart-table/test/ReactDataTable.test.tsx
@@ -0,0 +1,57 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { mount } from 'enzyme';
+import ReactDataTable from '../src/ReactDataTable';
+import transformProps from '../src/transformProps';
+import * as testData from './test_data';
+
+describe('legacy-table', () => {
+  // Can test more prop transformation here. Not needed for now.
+  describe('transformProps', () => {});
+
+  describe('ReactDataTable', () => {
+    // Jest throw an error at `console.warn`, this disables that behavior
+    beforeAll(() => {
+      console.warn = console.log;
+    });
+    afterAll(() => {
+      console.warn = console.error;
+    });
+
+    let wrap: any; // the SuperChart wraper
+    let tree: any; // helper to dive into DOM tree
+
+    it('render basic data', () => {
+      wrap = mount(<ReactDataTable {...transformProps(testData.basic)} />);
+      tree = wrap.render(); // returns a CheerioWrapper with jQuery-like API
+      expect(tree.hasClass('superset-legacy-chart-table')).toEqual(true);
+      expect(tree.find('td[data-sort="Michael"]')).toHaveLength(1);
+    });
+
+    it('render advanced data', () => {
+      // should successfull rerender with new props
+      wrap.setProps(transformProps(testData.advanced));
+      tree = wrap.render();
+      const text = tree.text();
+      expect(text).toContain('Sum of Num');
+      expect(text).toContain('12.346%');
+    });
+  });
+});
diff --git a/packages/superset-ui-legacy-plugin-chart-table/test/test_data.ts b/packages/superset-ui-legacy-plugin-chart-table/test/test_data.ts
new file mode 100644
index 000000000..2656c24d1
--- /dev/null
+++ b/packages/superset-ui-legacy-plugin-chart-table/test/test_data.ts
@@ -0,0 +1,102 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { ChartProps } from '@superset-ui/chart';
+
+const basicFormData = {
+  alignPn: false,
+  colorPn: false,
+  includeSearch: false,
+  orderDesc: true,
+  pageLength: 0,
+  metrics: [],
+  percentMetrics: null,
+  timeseriesLimitMetric: null,
+  tableFilter: false,
+  tableTimestampFormat: '%Y-%m-%d %H:%M:%S',
+};
+
+const basicChartProps = {
+  width: 200,
+  height: 500,
+  annotationData: {},
+  datasource: {
+    columnFormats: {},
+    verboseMap: {},
+  },
+  rawDatasource: {},
+  rawFormData: {},
+  hooks: {},
+  initialValues: {},
+  queryData: {
+    data: {
+      columns: [],
+      records: [],
+    },
+  },
+  formData: basicFormData,
+};
+
+/**
+ * Basic data input
+ */
+export const basic: ChartProps = {
+  ...basicChartProps,
+  queryData: {
+    data: {
+      columns: ['name', 'sum__num'],
+      records: [
+        {
+          name: 'Michael',
+          sum__num: 2467063,
+          '%pct_nice': 0.123456,
+        },
+        {
+          name: 'Joe',
+          sum__num: 2467,
+          '%pct_nice': 0.00001,
+        },
+      ],
+    },
+  },
+};
+
+/**
+ * Advanced data input with
+ *   - verbose map
+ *   - percent metrics
+ */
+export const advanced: ChartProps = {
+  ...basic,
+  datasource: {
+    columnFormats: {},
+    verboseMap: {
+      sum__num: 'Sum of Num',
+    },
+  },
+  formData: {
+    ...basicFormData,
+    percentMetrics: ['pct_nice'],
+  },
+  queryData: {
+    data: {
+      columns: ['name', 'sum__num', '%pct_nice'],
+      records: [...basic.queryData.data.records],
+    },
+  },
+};
diff --git a/scripts/setupJest.js b/scripts/setupJest.js
new file mode 100644
index 000000000..4483e0aff
--- /dev/null
+++ b/scripts/setupJest.js
@@ -0,0 +1,6 @@
+import { configure as configureTranslation } from '@superset-ui/translation';
+import { configure as configureEnzyme } from 'enzyme';
+import EnzymeReactAdapter from 'enzyme-adapter-react-16';
+
+configureTranslation();
+configureEnzyme({ adapter: new EnzymeReactAdapter() });
diff --git a/yarn.lock b/yarn.lock
index 5163c5646..251a9677c 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -10053,6 +10053,11 @@ har-validator@~5.1.3:
     ajv "^6.5.5"
     har-schema "^2.0.0"
 
+harmony-reflect@^1.4.6:
+  version "1.6.1"
+  resolved "https://registry.yarnpkg.com/harmony-reflect/-/harmony-reflect-1.6.1.tgz#c108d4f2bb451efef7a37861fdbdae72c9bdefa9"
+  integrity sha512-WJTeyp0JzGtHcuMsi7rw2VwtkvLa+JyfEKJCFyfcS0+CDkjQ5lHPu7zEhFZP+PDSRrEgXa5Ah0l1MbgbE41XjA==
+
 has-ansi@^2.0.0:
   version "2.0.0"
   resolved "https://registry.yarnpkg.com/has-ansi/-/has-ansi-2.0.0.tgz#34f5049ce1ecdf2b0649af3ef24e45ed35416d91"
@@ -10407,6 +10412,13 @@ icss-utils@^4.0.0, icss-utils@^4.1.1:
   dependencies:
     postcss "^7.0.14"
 
+identity-obj-proxy@^3.0.0:
+  version "3.0.0"
+  resolved "https://registry.yarnpkg.com/identity-obj-proxy/-/identity-obj-proxy-3.0.0.tgz#94d2bda96084453ef36fbc5aaec37e0f79f1fc14"
+  integrity sha1-lNK9qWCERT7zb7xarsN+D3nx/BQ=
+  dependencies:
+    harmony-reflect "^1.4.6"
+
 ieee754@^1.1.12, ieee754@^1.1.4:
   version "1.1.13"
   resolved "https://registry.yarnpkg.com/ieee754/-/ieee754-1.1.13.tgz#ec168558e95aa181fd87d37f55c32bbcb6708b84"

From 2f2555571ee25fc5584557481ddfb31591908a10 Mon Sep 17 00:00:00 2001
From: Jesse Yang <jesse.yang@airbnb.com>
Date: Sun, 8 Mar 2020 13:38:08 -0700
Subject: [PATCH 6/9] test(legacy-table): more sophisticated cases

---
 .../test/ReactDataTable.test.tsx              | 22 +++++++++++++------
 .../test/test_data.ts                         |  3 ++-
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/packages/superset-ui-legacy-plugin-chart-table/test/ReactDataTable.test.tsx b/packages/superset-ui-legacy-plugin-chart-table/test/ReactDataTable.test.tsx
index 1b64eef7c..ce5c90d3c 100644
--- a/packages/superset-ui-legacy-plugin-chart-table/test/ReactDataTable.test.tsx
+++ b/packages/superset-ui-legacy-plugin-chart-table/test/ReactDataTable.test.tsx
@@ -36,22 +36,30 @@ describe('legacy-table', () => {
     });
 
     let wrap: any; // the SuperChart wraper
-    let tree: any; // helper to dive into DOM tree
 
     it('render basic data', () => {
       wrap = mount(<ReactDataTable {...transformProps(testData.basic)} />);
-      tree = wrap.render(); // returns a CheerioWrapper with jQuery-like API
+      const tree = wrap.render(); // returns a CheerioWrapper with jQuery-like API
+      const cells = tree.find('td');
       expect(tree.hasClass('superset-legacy-chart-table')).toEqual(true);
-      expect(tree.find('td[data-sort="Michael"]')).toHaveLength(1);
+      expect(cells).toHaveLength(4);
+      expect(cells.eq(0).text()).toEqual('Michael');
+      expect(cells.eq(3).attr('data-sort')).toEqual('2467');
     });
 
     it('render advanced data', () => {
       // should successfull rerender with new props
       wrap.setProps(transformProps(testData.advanced));
-      tree = wrap.render();
-      const text = tree.text();
-      expect(text).toContain('Sum of Num');
-      expect(text).toContain('12.346%');
+      const tree = wrap.render();
+      const cells = tree.find('td');
+      expect(
+        tree
+          .find('th')
+          .eq(1)
+          .text(),
+      ).toEqual('Sum of Num');
+      expect(cells.eq(2).text()).toEqual('12.346%');
+      expect(cells.eq(4).text()).toEqual('2.47k');
     });
   });
 });
diff --git a/packages/superset-ui-legacy-plugin-chart-table/test/test_data.ts b/packages/superset-ui-legacy-plugin-chart-table/test/test_data.ts
index 2656c24d1..e51d66880 100644
--- a/packages/superset-ui-legacy-plugin-chart-table/test/test_data.ts
+++ b/packages/superset-ui-legacy-plugin-chart-table/test/test_data.ts
@@ -79,7 +79,7 @@ export const basic: ChartProps = {
 /**
  * Advanced data input with
  *   - verbose map
- *   - percent metrics
+ *   - metric columns
  */
 export const advanced: ChartProps = {
   ...basic,
@@ -91,6 +91,7 @@ export const advanced: ChartProps = {
   },
   formData: {
     ...basicFormData,
+    metrics: ['sum__num'],
     percentMetrics: ['pct_nice'],
   },
   queryData: {

From c8a58c9193fc78a5b17b4c48937fd8824de2cdcc Mon Sep 17 00:00:00 2001
From: Jesse Yang <jesse.yang@airbnb.com>
Date: Mon, 9 Mar 2020 10:46:36 -0700
Subject: [PATCH 7/9] fix: address PR #392 comments

---
 .../test/ReactDataTable.test.tsx                   |  2 +-
 .../test/{test_data.ts => testData.ts}             |  9 +++++++--
 scripts/build.js                                   | 14 +++++++++-----
 3 files changed, 17 insertions(+), 8 deletions(-)
 rename packages/superset-ui-legacy-plugin-chart-table/test/{test_data.ts => testData.ts} (95%)

diff --git a/packages/superset-ui-legacy-plugin-chart-table/test/ReactDataTable.test.tsx b/packages/superset-ui-legacy-plugin-chart-table/test/ReactDataTable.test.tsx
index ce5c90d3c..3a21e2d4e 100644
--- a/packages/superset-ui-legacy-plugin-chart-table/test/ReactDataTable.test.tsx
+++ b/packages/superset-ui-legacy-plugin-chart-table/test/ReactDataTable.test.tsx
@@ -20,7 +20,7 @@ import React from 'react';
 import { mount } from 'enzyme';
 import ReactDataTable from '../src/ReactDataTable';
 import transformProps from '../src/transformProps';
-import * as testData from './test_data';
+import testData from './testData';
 
 describe('legacy-table', () => {
   // Can test more prop transformation here. Not needed for now.
diff --git a/packages/superset-ui-legacy-plugin-chart-table/test/test_data.ts b/packages/superset-ui-legacy-plugin-chart-table/test/testData.ts
similarity index 95%
rename from packages/superset-ui-legacy-plugin-chart-table/test/test_data.ts
rename to packages/superset-ui-legacy-plugin-chart-table/test/testData.ts
index e51d66880..fabb82d20 100644
--- a/packages/superset-ui-legacy-plugin-chart-table/test/test_data.ts
+++ b/packages/superset-ui-legacy-plugin-chart-table/test/testData.ts
@@ -55,7 +55,7 @@ const basicChartProps = {
 /**
  * Basic data input
  */
-export const basic: ChartProps = {
+const basic: ChartProps = {
   ...basicChartProps,
   queryData: {
     data: {
@@ -81,7 +81,7 @@ export const basic: ChartProps = {
  *   - verbose map
  *   - metric columns
  */
-export const advanced: ChartProps = {
+const advanced: ChartProps = {
   ...basic,
   datasource: {
     columnFormats: {},
@@ -101,3 +101,8 @@ export const advanced: ChartProps = {
     },
   },
 };
+
+export default {
+  basic,
+  advanced,
+};
diff --git a/scripts/build.js b/scripts/build.js
index 284babd23..0326d0f71 100644
--- a/scripts/build.js
+++ b/scripts/build.js
@@ -4,6 +4,7 @@
 const { spawnSync, spawn } = require('child_process');
 
 const glob = process.argv[2];
+const extraArgs = process.argv.slice(2);
 
 process.env.PATH = `./node_modules/.bin:${process.env.PATH}`;
 
@@ -18,11 +19,14 @@ const run = (cmd) => {
 };
 
 if (glob) {
-  run(`nimbus eslint packages/${glob}/{src,test}`);
-  run(`nimbus prettier --check --workspaces=\"@superset-ui/${glob}"`);
-  run(`nimbus babel --clean --workspaces=\"@superset-ui/${glob}"`);
-  run(`nimbus babel --clean --workspaces=\"@superset-ui/${glob}" --esm`);
-  run(`nimbus typescript --build --workspaces=\"@superset-ui/${glob}"`);
+  run(`nimbus prettier packages/${glob}/{src,test}/**/*.{js,jsx,ts,tsx,css}"`);
+  // lint is slow, so not turning it on by default
+  if (extraArgs.includes('--lint')) {
+    run(`nimbus eslint packages/${glob}/{src,test}`);
+  }
+  run(`nimbus babel --clean --workspaces="@superset-ui/${glob}"`);
+  run(`nimbus babel --clean --workspaces="@superset-ui/${glob}" --esm`);
+  run(`nimbus typescript --build --workspaces="@superset-ui/${glob}"`);
   require('./buildAssets');
 } else {
   run('yarn build');

From 933c3f9100eb4ce33aceb9bdf19f1788964f29e2 Mon Sep 17 00:00:00 2001
From: Jesse Yang <jesse.yang@airbnb.com>
Date: Mon, 9 Mar 2020 11:20:23 -0700
Subject: [PATCH 8/9] chore(legacy-table): clean up tests case setups

Not seeing the console.warn errors anymore. So cleaning it up.

Previously it was from `<SuperChart />` component, but since we have
updated the test case to not use <SuperChart>, we are good now.
---
 .../test/ReactDataTable.test.tsx                          | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/packages/superset-ui-legacy-plugin-chart-table/test/ReactDataTable.test.tsx b/packages/superset-ui-legacy-plugin-chart-table/test/ReactDataTable.test.tsx
index 3a21e2d4e..cb8e05b9a 100644
--- a/packages/superset-ui-legacy-plugin-chart-table/test/ReactDataTable.test.tsx
+++ b/packages/superset-ui-legacy-plugin-chart-table/test/ReactDataTable.test.tsx
@@ -27,14 +27,6 @@ describe('legacy-table', () => {
   describe('transformProps', () => {});
 
   describe('ReactDataTable', () => {
-    // Jest throw an error at `console.warn`, this disables that behavior
-    beforeAll(() => {
-      console.warn = console.log;
-    });
-    afterAll(() => {
-      console.warn = console.error;
-    });
-
     let wrap: any; // the SuperChart wraper
 
     it('render basic data', () => {

From b7596d052a17b0124a4acbd65638fe861f2d5622 Mon Sep 17 00:00:00 2001
From: Jesse Yang <jesse.yang@airbnb.com>
Date: Mon, 9 Mar 2020 11:22:57 -0700
Subject: [PATCH 9/9] fix(legacy-table): misleading comment

---
 .../test/ReactDataTable.test.tsx                                | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/superset-ui-legacy-plugin-chart-table/test/ReactDataTable.test.tsx b/packages/superset-ui-legacy-plugin-chart-table/test/ReactDataTable.test.tsx
index cb8e05b9a..7b7fb3b76 100644
--- a/packages/superset-ui-legacy-plugin-chart-table/test/ReactDataTable.test.tsx
+++ b/packages/superset-ui-legacy-plugin-chart-table/test/ReactDataTable.test.tsx
@@ -27,7 +27,7 @@ describe('legacy-table', () => {
   describe('transformProps', () => {});
 
   describe('ReactDataTable', () => {
-    let wrap: any; // the SuperChart wraper
+    let wrap: any; // the ReactDataTable wraper
 
     it('render basic data', () => {
       wrap = mount(<ReactDataTable {...transformProps(testData.basic)} />);