refactor: merge/upgrade superset-ui packages #10790

ktmud · 2020-09-04T04:06:29Z

SUMMARY

Merge core @superset-ui/* packages into one single package. For details, see: apache-superset/superset-ui#768

This PR does two things and two things only:

Bump dependency versions for @superset-ui/core, @superset-ui/chart-controls and all the viz plugins.
Update the corresponding import paths.

Any changes not related to this two should be called out. Everything should work exactly like before.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

CI + Manual verification on major visualization related pages.

ADDITIONAL INFORMATION

ktmud · 2020-09-09T01:16:39Z

superset-frontend/package.json

@@ -22,6 +22,8 @@
    "prettier-check": "prettier --check '{src,stylesheets}/**/*.{css,less,sass,scss}'",
    "lint-fix": "eslint --fix --ignore-path=.eslintignore --ext .js,.jsx,.ts,tsx . && npm run clean-css && npm run type",
    "clean-css": "prettier --write '{src,stylesheets}/**/*.{css,less,sass,scss}'",
+    "format": "prettier --write './{src,spec,stylesheets,cypress-base}/**/*{.js,.jsx,.ts,.tsx,.css,.less,.scss,.sass}'",
+    "prettier": "npm run format",


Adding a shortcut command to format files with Prettier, to be consistent with superset-ui

ktmud · 2020-09-09T01:17:22Z

superset-frontend/package.json

@@ -118,6 +108,7 @@
    "dnd-core": "^2.6.0",
    "dom-to-image": "^2.6.0",
    "geolib": "^2.0.24",
+    "global-box": "^1.2.0",


A new peerDependency from the updated encodable, which is a dependency of preset-chart-xy.

ktmud · 2020-09-09T01:21:33Z

superset-frontend/package.json

@@ -149,6 +140,7 @@
    "react-hot-loader": "^4.12.20",
    "react-json-tree": "^0.11.2",
    "react-jsonschema-form": "^1.2.0",
+    "react-loadable": "^5.5.0",


Previously a dependency of @superset-ui/chart, adding it here because it was moved to peerDependencies of @superset-ui/core.

codecov-commenter · 2020-09-09T01:46:51Z

Codecov Report

Merging #10790 into master will decrease coverage by 0.02%.
The diff coverage is 87.50%.

@@            Coverage Diff             @@
##           master   #10790      +/-   ##
==========================================
- Coverage   61.30%   61.27%   -0.03%     
==========================================
  Files         802      802              
  Lines       37870    37845      -25     
  Branches     3561     3561              
==========================================
- Hits        23215    23190      -25     
  Misses      14469    14469              
  Partials      186      186

Flag	Coverage Δ
#javascript	`61.58% <87.50%> (-0.07%)`	⬇️
#python	`61.09% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset-frontend/src/SqlLab/App.jsx	`0.00% <ø> (ø)`
superset-frontend/src/SqlLab/actions/sqlLab.js	`59.61% <ø> (ø)`
superset-frontend/src/SqlLab/components/App.jsx	`77.77% <ø> (ø)`
.../src/SqlLab/components/EstimateQueryCostButton.jsx	`17.39% <ø> (ø)`
...src/SqlLab/components/ExploreCtasResultsButton.jsx	`13.33% <ø> (ø)`
...end/src/SqlLab/components/ExploreResultsButton.jsx	`74.07% <ø> (ø)`
...-frontend/src/SqlLab/components/HighlightedSql.jsx	`90.00% <ø> (ø)`
...rontend/src/SqlLab/components/QueryAutoRefresh.jsx	`65.90% <ø> (ø)`
...et-frontend/src/SqlLab/components/QueryHistory.jsx	`83.33% <ø> (ø)`
...set-frontend/src/SqlLab/components/QuerySearch.jsx	`57.54% <ø> (ø)`
... and 205 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 50672bb...a3ce1e4. Read the comment docs.

villebro · 2020-09-09T06:05:47Z

superset-frontend/src/setup/setupColors.js

+import airbnb from '@superset-ui/core/esm/color/colorSchemes/categorical/airbnb';
+import categoricalD3 from '@superset-ui/core/esm/color/colorSchemes/categorical/d3';
+import echarts from '@superset-ui/core/esm/color/colorSchemes/categorical/echarts';
+import google from '@superset-ui/core/esm/color/colorSchemes/categorical/google';
+import lyft from '@superset-ui/core/esm/color/colorSchemes/categorical/lyft';
+import preset from '@superset-ui/core/esm/color/colorSchemes/categorical/preset';
+import sequentialCommon from '@superset-ui/core/esm/color/colorSchemes/sequential/common';
+import sequentialD3 from '@superset-ui/core/esm/color/colorSchemes/sequential/d3';


Not that this behavior is being changed, but is there some reason these have to be pulled in from /esm/ and aren't exported like the other components in superset-ui/core? It feels like these should be exported as categoricalColorSchemes.* and sequentialColorSchemes.* or similar.

I don't feel like doing additional refactor in this already humongous PR. For this specific case, I almost feel these colors should probably be move out of the @superset-ui/core package and placed under incubator-superset instead so that adding a new color wouldn't require publishing the package and custom Superset installations can also opt out colors more thoroughly.

Yeah, moving these to incubator-superset probably makes more sense. Also not recommending changing this now, this was mostly a question and proposal for future changes post merge.

we could also add the export from index and get rid of deep import later.
Agree not doing it in this humongous PR

ktmud · 2020-09-09T02:26:53Z

superset-frontend/cypress-base/cypress/integration/explore/visualizations/big_number.test.js

@@ -43,7 +43,7 @@ describe('Visualization > Big Number with Trendline', () => {
    cy.visitChartByParams(JSON.stringify(formData));
    cy.verifySliceSuccess({
      waitAlias: '@getJson',
-      chartSelector: '.big_number',
+      chartSelector: '.superset-legacy-chart-big-number',


SupersetChart may render before the chart content is rendered, so Cypress will find an empty div.big_number (the wrapper of the actual chart), breaking the test.

ktmud · 2020-09-09T02:27:43Z

superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts

@@ -30,12 +30,12 @@ describe('AdhocFilters', () => {

    cy.get('[data-test=adhoc_filters]').within(() => {
      cy.get('.Select__control').click();
-      cy.get('input[type=text]').type('name{enter}');
+      cy.get('input[type=text]').focus().type('name{enter}');


Fixing an input is covered by "Choose a metric..." placeholder error.

ktmud · 2020-09-09T19:20:26Z

@kristw @evans @villebro This is ready for review, even though an unrelated python test was failing for no reason.

kristw

even though an unrelated python test was failing for no reason.

CI is green now. Is it just flaky test?

ktmud · 2020-09-09T19:44:18Z

even though an unrelated python test was failing for no reason.

CI is green now. Is it just flaky test?

I think so

* refactor: merge/upgrade superset-ui packages * Fix flaky big number test * Fix Flaky AdhocFilters test

ktmud marked this pull request as draft September 4, 2020 04:09

ktmud force-pushed the upgrade-superset-ui branch from 2165a08 to ab12b53 Compare September 9, 2020 01:08

ktmud marked this pull request as ready for review September 9, 2020 01:13

ktmud commented Sep 9, 2020

View reviewed changes

ktmud force-pushed the upgrade-superset-ui branch from c96638c to 9127442 Compare September 9, 2020 01:39

ktmud force-pushed the upgrade-superset-ui branch from 9127442 to 389d75a Compare September 9, 2020 02:15

villebro reviewed Sep 9, 2020

View reviewed changes

ktmud commented Sep 9, 2020

View reviewed changes

ktmud force-pushed the upgrade-superset-ui branch from a3ce1e4 to b6bddf8 Compare September 9, 2020 17:09

ktmud added 3 commits September 9, 2020 10:22

refactor: merge/upgrade superset-ui packages

06c9bb6

Fix flacky big number test

3ef00cb

Fix Flacky AdhocFilters test

b963251

ktmud force-pushed the upgrade-superset-ui branch from b6bddf8 to b963251 Compare September 9, 2020 17:23

kristw approved these changes Sep 9, 2020

View reviewed changes

villebro approved these changes Sep 9, 2020

View reviewed changes

ktmud merged commit 9a59bdd into apache:master Sep 9, 2020

ktmud deleted the upgrade-superset-ui branch September 9, 2020 20:19

villebro added v0.38 and removed v0.38 labels Sep 10, 2020

auxten pushed a commit to auxten/incubator-superset that referenced this pull request Nov 20, 2020

refactor: merge/upgrade superset-ui packages (apache#10790)

32fea9f

* refactor: merge/upgrade superset-ui packages * Fix flaky big number test * Fix Flaky AdhocFilters test

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.38.0 labels Mar 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: merge/upgrade superset-ui packages #10790

refactor: merge/upgrade superset-ui packages #10790

ktmud commented Sep 4, 2020

ktmud Sep 9, 2020

ktmud Sep 9, 2020

ktmud Sep 9, 2020

codecov-commenter commented Sep 9, 2020 •

edited

Loading

villebro Sep 9, 2020

ktmud Sep 9, 2020 •

edited

Loading

villebro Sep 9, 2020

kristw Sep 9, 2020

ktmud Sep 9, 2020

ktmud Sep 9, 2020

ktmud commented Sep 9, 2020

kristw left a comment

ktmud commented Sep 9, 2020

refactor: merge/upgrade superset-ui packages #10790

refactor: merge/upgrade superset-ui packages #10790

Conversation

ktmud commented Sep 4, 2020

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Sep 9, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

ktmud Sep 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ktmud commented Sep 9, 2020

kristw left a comment

Choose a reason for hiding this comment

ktmud commented Sep 9, 2020

codecov-commenter commented Sep 9, 2020 •

edited

Loading

ktmud Sep 9, 2020 •

edited

Loading