fix(plugin-chart-table): value undefined when table name has dot (apa…

…che#686) * fix(plugin-chart-table): value undefined when table name has dot * Upgrade react everywhere * add comment
zhaoyongjie · Nov 25, 2021 · d855d2b · d855d2b
1 parent 6a31c7e
commit d855d2b
Show file tree

Hide file tree

Showing 14 changed files with 43 additions and 24 deletions.
diff --git a/...end/temporary_superset_ui/superset-ui/packages/superset-ui-chart-composition/package.json b/...end/temporary_superset_ui/superset-ui/packages/superset-ui-chart-composition/package.json
@@ -26,7 +26,7 @@
     "access": "public"
   },
   "dependencies": {
-    "@types/react": "^16.9.38",
+    "@types/react": "^16.9.43",
     "@vx/responsive": "^0.0.197",
     "csstype": "^2.6.4"
   },

diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/package.json b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/package.json
@@ -27,7 +27,7 @@
     "access": "public"
   },
   "dependencies": {
-    "@types/react": "^16.9.38",
+    "@types/react": "^16.9.43",
     "@types/react-loadable": "^5.4.2",
     "@vx/responsive": "^0.0.197",
     "prop-types": "^15.6.2",

diff --git a/...set-ui/packages/superset-ui-demo/storybook/stories/plugins/plugin-chart-table/testData.ts b/...set-ui/packages/superset-ui-demo/storybook/stories/plugins/plugin-chart-table/testData.ts
@@ -35,57 +35,67 @@ export const basicFormData = {
   timeseriesLimitMetric: null,
 };
 export const basicData = {
-  columns: ['name', 'sum__num', 'MAX(ds)'],
+  columns: ['name', 'sum__num', 'MAX(ds)', 'Abc.com'],
   records: [
     {
       name: 'Michael',
       sum__num: 2467063,
       'MAX(ds)': '2008-01-01T00:00:00',
+      'Abc.com': 110,
     },
     {
       name: 'Christopher',
       sum__num: 1725265,
       'MAX(ds)': '2008-01-01T00:00:00',
+      'Abc.com': 119,
     },
     {
       name: 'David',
       sum__num: 1570516,
       'MAX(ds)': '2008-01-01T00:00:00',
+      'Abc.com': 120,
     },
     {
       name: 'James',
       sum__num: 1506025,
       'MAX(ds)': '2008-01-01T00:00:00',
+      'Abc.com': 120,
     },
     {
       name: 'John',
       sum__num: 1426074,
       'MAX(ds)': '2008-01-01T00:00:00',
+      'Abc.com': 120,
     },
     {
       name: 'Matthew',
       sum__num: 1355803,
       'MAX(ds)': '2008-01-01T00:00:00',
+      'Abc.com': 120,
     },
     {
       name: 'Robert',
       sum__num: 1314800,
       'MAX(ds)': '2008-01-01T00:00:00',
+      'Abc.com': 120,
     },
     {
       name: 'Daniel',
       sum__num: 1159354,
       'MAX(ds)': '2008-01-01T00:00:00',
+      'Abc.com': 120,
     },
     {
       name: 'Joseph',
       sum__num: 1114098,
       'MAX(ds)': '2008-01-01T00:00:00',
+      'Abc.com': 120,
     },
     {
       name: 'William',
       sum__num: 1113701,
       'MAX(ds)': '2008-01-01T00:00:00',
+      'Abc.com': 120,
     },
   ],
 };
diff --git a/...end/temporary_superset_ui/superset-ui/plugins/legacy-plugin-chart-time-table/package.json b/...end/temporary_superset_ui/superset-ui/plugins/legacy-plugin-chart-time-table/package.json
@@ -30,7 +30,7 @@
   "dependencies": {
     "@data-ui/sparkline": "^0.0.84",
     "@types/d3-scale": "^2.0.2",
-    "@types/react": "^16.9.38",
+    "@types/react": "^16.9.43",
     "d3-scale": "^3.2.1",
     "emotion-theming": "^10.0.27",
     "moment": "^2.26.0",

diff --git a/...ontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-choropleth-map/package.json b/...ontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-choropleth-map/package.json
@@ -29,7 +29,7 @@
   "dependencies": {
     "@types/d3-geo": "^1.11.1",
     "@types/geojson": "^7946.0.3",
-    "@types/react": "^16.9.38",
+    "@types/react": "^16.9.43",
     "@types/topojson-client": "^3.0.0",
     "@types/topojson-specification": "^1.0.0",
     "@vx/clip-path": "^0.0.197",

diff --git a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/package.json b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/package.json
@@ -29,8 +29,7 @@
     "@emotion/core": "^10.0.28",
     "@types/d3-array": "^2.0.0",
     "@types/match-sorter": "^4.0.0",
-    "@types/react": "^16.9.35",
-    "@types/react-dom": "^16.9.8",
+    "@types/react": "^16.9.43",
     "@types/react-table": "^7.0.19",
     "d3-array": "^2.4.0",
     "match-sorter": "^4.1.0",
@@ -49,7 +48,6 @@
     "@superset-ui/time-format": "^0.14.0",
     "@superset-ui/translation": "^0.14.0",
     "@superset-ui/validator": "^0.14.0",
-    "jquery": "^3.4.1",
     "react": "^16.13.1",
     "react-dom": "^16.13.1"
   }

diff --git a/...-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/TableChart.tsx b/...-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/TableChart.tsx
@@ -211,7 +211,10 @@ export default function TableChart<D extends DataRecord = DataRecord>(
       };
       return {
         id: String(i), // to allow duplicate column keys
-        accessor: key,
+        // must use custom accessor to allow `.` in column names
+        // typing is incorrect in current version of `@types/react-table`
+        // so we ask TS not to check.
+        accessor: ((datum: D) => datum[key]) as never,
         Header: label,
         SortIcon,
         sortDescFirst: sortDesc,

diff --git a/...end/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/test/TableChart.test.tsx b/...end/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/test/TableChart.test.tsx
@@ -67,12 +67,15 @@ describe('plugin-chart-table', () => {
       wrap = mount(<TableChart {...transformProps(testData.basic)} sticky={false} />);
       tree = wrap.render(); // returns a CheerioWrapper with jQuery-like API
       const cells = tree.find('td');
-      expect(cells).toHaveLength(6);
+      expect(cells).toHaveLength(8);
       expect(cells.eq(0).text()).toEqual('2020-01-01 12:34:56');
       expect(cells.eq(1).text()).toEqual('Michael');
       // number is not in `metrics` list, so it should output raw value
       // (in real world Superset, this would mean the column is used in GROUP BY)
-      expect(cells.eq(5).text()).toEqual('2467');
+      expect(cells.eq(2).text()).toEqual('2467063');
+      // should not render column with `.` in name as `undefined`
+      expect(cells.eq(3).text()).toEqual('foo');
+      expect(cells.eq(6).text()).toEqual('2467');
     });
 
     it('render advanced data', () => {

diff --git a/...et-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/test/testData.ts b/...et-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/test/testData.ts
@@ -63,19 +63,21 @@ const basic = {
   ...new ChartProps(basicChartProps),
   queryData: {
     data: {
-      columns: ['__timestamp', 'name', 'sum__num'],
+      columns: ['__timestamp', 'name', 'sum__num', 'abc.com'],
       records: [
         {
           __timestamp: '2020-01-01T12:34:56',
           name: 'Michael',
           sum__num: 2467063,
           '%pct_nice': 0.123456,
+          'abc.com': 'foo',
         },
         {
           __timestamp: 1585932584140,
           name: 'Joe',
           sum__num: 2467,
           '%pct_nice': 0.00001,
+          'abc.com': 'bar',
         },
       ],
     },

diff --git a/...t-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-word-cloud/package.json b/...t-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-word-cloud/package.json
@@ -30,7 +30,7 @@
   "dependencies": {
     "@types/d3-cloud": "^1.2.1",
     "@types/d3-scale": "^2.0.2",
-    "@types/react": "^16.9.38",
+    "@types/react": "^16.9.43",
     "d3-cloud": "^1.2.5",
     "d3-scale": "^3.0.1",
     "emotion-theming": "^10.0.27",

diff --git a/superset-frontend/temporary_superset_ui/superset-ui/scripts/build.js b/superset-frontend/temporary_superset_ui/superset-ui/scripts/build.js
@@ -28,10 +28,13 @@ if (glob) {
   if (extraArgs.includes('--lint')) {
     run(`nimbus eslint {packages,plugins}/${glob}/{src,test}`);
   }
-  rimraf.sync(`./{packages,plugins}/${glob}/{lib,esm,tsconfig.tsbuildinfo}`);
-  run(`nimbus babel --clean --workspaces="@superset-ui/${glob}" ${BABEL_CONFIG}`);
-  run(`nimbus babel --clean --workspaces="@superset-ui/${glob}" --esm ${BABEL_CONFIG}`);
-  run(`nimbus typescript --build --workspaces="@superset-ui/${glob}"`);
+  rimraf.sync(
+    `./{packages,plugins}/${glob}/{lib,esm,tsconfig.tsbuildinfo,node_modules/@types/react}`,
+  );
+  const packageName = glob.replace(/^superset-ui-/, '');
+  run(`nimbus babel --clean --workspaces="@superset-ui/${packageName}" ${BABEL_CONFIG}`);
+  run(`nimbus babel --clean --workspaces="@superset-ui/${packageName}" --esm ${BABEL_CONFIG}`);
+  run(`nimbus typescript --build --workspaces="@superset-ui/${packageName}"`);
   // eslint-disable-next-line global-require
   require('./copyAssets');
 } else {

diff --git a/superset-frontend/temporary_superset_ui/superset-ui/temporary-plugins/package.json b/superset-frontend/temporary_superset_ui/superset-ui/temporary-plugins/package.json
@@ -53,7 +53,7 @@
     "@types/enzyme": "^3.10.3",
     "@types/jest": "^25.1.3",
     "@types/jsdom": "^12.2.4",
-    "@types/react": "^16.9.38",
+    "@types/react": "^16.9.43",
     "@types/react-test-renderer": "^16.9.2",
     "csstype": "^2.6.3",
     "enzyme": "^3.11.0",

diff --git a/...temporary_superset_ui/superset-ui/temporary-plugins/superset-ui-plugins-demo/package.json b/...temporary_superset_ui/superset-ui/temporary-plugins/superset-ui-plugins-demo/package.json
@@ -37,7 +37,7 @@
     "@storybook/addon-links": "^5.0.9",
     "@storybook/addons": "^5.0.9",
     "@storybook/react": "^5.0.9",
-    "@types/react": "^16.9.38",
+    "@types/react": "^16.9.43",
     "@types/storybook__react": "4.0.2",
     "bootstrap": "^3.4.1",
     "jquery": "^3.4.1",

diff --git a/superset-frontend/temporary_superset_ui/superset-ui/yarn.lock b/superset-frontend/temporary_superset_ui/superset-ui/yarn.lock
@@ -4024,7 +4024,7 @@
   dependencies:
     "@types/react" "*"
 
-"@types/react-dom@*", "@types/react-dom@^16.9.8":
+"@types/react-dom@*":
   version "16.9.8"
   resolved "https://registry.yarnpkg.com/@types/react-dom/-/react-dom-16.9.8.tgz#fe4c1e11dfc67155733dfa6aa65108b4971cb423"
   integrity sha512-ykkPQ+5nFknnlU6lDd947WbQ6TE3NNzbQAkInC2EKY1qeYdTKp7onFusmYZb+ityzx2YviqT6BXSu+LyWWJwcA==
@@ -4074,10 +4074,10 @@
   dependencies:
     "@types/react" "*"
 
-"@types/react@*", "@types/react@^16.9.11", "@types/react@^16.9.35", "@types/react@^16.9.38":
-  version "16.9.38"
-  resolved "https://registry.yarnpkg.com/@types/react/-/react-16.9.38.tgz#868405dace93a4095d3e054f4c4a1de7a1ac0680"
-  integrity sha512-pHAeZbjjNRa/hxyNuLrvbxhhnKyKNiLC6I5fRF2Zr/t/S6zS41MiyzH4+c+1I9vVfvuRt1VS2Lodjr4ZWnxrdA==
+"@types/react@*", "@types/react@^16.9.11", "@types/react@^16.9.43":
+  version "16.9.43"
+  resolved "https://registry.yarnpkg.com/@types/react/-/react-16.9.43.tgz#c287f23f6189666ee3bebc2eb8d0f84bcb6cdb6b"
+  integrity sha512-PxshAFcnJqIWYpJbLPriClH53Z2WlJcVZE+NP2etUtWQs2s7yIMj3/LDKZT/5CHJ/F62iyjVCDu2H3jHEXIxSg==
   dependencies:
     "@types/prop-types" "*"
     csstype "^2.2.0"