chore: Upgrade Webpack to v5 #16701

kgabryje · 2021-09-14T12:58:26Z

SUMMARY

What's been done:

Bump webpack from 4.42.0 to 5.52.1
Bump webpack-cli, webpack-dev-server, plugins and loaders to latest versions
Remove redundant plugins and loaders
Refactor webpack config file according to the migration guide (https://webpack.js.org/migrate/5/) and bumped plugins changelogs
Replace url-loaders and file-loaders with Asset Modules
Replace mathjs with math-expression-evaluator. Mathjs was causing an import error, but we've been using it in only 1 component (AnnotationLayer), so I replaced it instead of investigating the error. Pulling an 11MB lib just to parse 1 expression was a waste anyway

Next steps:

Bump other plugins (especially those marked by dependabot to be security vulnerabilities)
Attempt to simplify our webpack config file - it's quite complex and some parts of it might be unnecessary

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

no visual changes

TESTING INSTRUCTIONS

Everything should build like before
The whole app should work like before - verify that images, fonts, libs are getting loaded properly and there are no runtime errors anywhere
Linking superset-ui libraries should work like before

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

codecov · 2021-09-14T13:46:09Z

Codecov Report

Merging #16701 (46595d2) into master (b92358b) will increase coverage by 0.00%.
The diff coverage is 81.25%.

@@           Coverage Diff           @@
##           master   #16701   +/-   ##
=======================================
  Coverage   76.98%   76.99%           
=======================================
  Files        1007     1007           
  Lines       54174    54180    +6     
  Branches     7463     7467    +4     
=======================================
+ Hits        41708    41714    +6     
  Misses      12226    12226           
  Partials      240      240

Flag	Coverage Δ
javascript	`71.31% <81.25%> (+<0.01%)`	⬆️

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

Impacted Files	Coverage Δ
...c/visualizations/FilterBox/FilterBoxChartPlugin.js	`75.00% <0.00%> (ø)`
...c/visualizations/TimeTable/TimeTableChartPlugin.js	`75.00% <0.00%> (ø)`
...ontrols/AnnotationLayerControl/AnnotationLayer.jsx	`83.84% <91.66%> (+0.43%)`	⬆️
superset-frontend/src/components/Icons/Icon.tsx	`100.00% <100.00%> (ø)`
...rset-frontend/src/components/ListView/ListView.tsx	`96.19% <100.00%> (ø)`

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 b92358b...46595d2. Read the comment docs.

kgabryje · 2021-09-14T14:15:13Z

/testenv up

github-actions · 2021-09-14T14:17:36Z

@kgabryje Ephemeral environment spinning up at http://35.163.128.192:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

etr2460

amazing, thanks for tackling this! let's let it soak for a couple days, but we can probably merge by EOW if no one has any concerns

etr2460 · 2021-09-14T14:45:12Z

superset-frontend/package.json

@@ -129,7 +129,7 @@
    "lodash": "^4.17.21",
    "lodash-es": "^4.17.21",
    "match-sorter": "^6.1.0",
-    "mathjs": "^8.0.1",


this will be a great perf win too!

superset-frontend/webpack.config.js

junlincc · 2021-09-16T17:51:11Z

/testenv up

github-actions · 2021-09-16T17:56:01Z

@junlincc Ephemeral environment spinning up at http://54.200.57.4:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

junlincc · 2021-09-16T18:18:12Z

export sample dashboard from homescreen is loading forever. double checked on production, working properly. not sure this is related to this PR, please confirm. also the file i downloaded is in JSON format, should be .zip instead

Screen.Recording.2021-09-16.at.11.15.38.AM.mov

villebro · 2021-09-17T07:08:41Z

export sample dashboard from homescreen is loading forever. double checked on production, working properly. not sure this is related to this PR, please confirm. also the file i downloaded is in JSON format, should be .zip instead

Screen.Recording.2021-09-16.at.11.15.38.AM.mov

I was able to repro this locally. This seems to be an unrelated bug affecting json exports, and I was able to repro this on master, too. Exporting zip files requires enabling the VERSIONED_EXPORT feature flag, and with this FF enabled exporting works as expected. I believe we should enable VERSIONED_EXPORT by default, as the legacy export feature appears to not work properly anymore.

villebro

Some comments, but in general LGTM and seems to work great!

villebro · 2021-09-17T07:10:16Z

superset-frontend/cypress-base/cypress/integration/dashboard/markdown.test.ts

-    let numScripts = 0;
-    cy.get('script').then(nodes => {
-      numScripts = nodes.length;
-    });


nit: we should maybe update the test name to reflect the fact that we're testing the edit feature.

villebro · 2021-09-17T07:12:02Z

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

-  xit('Should not load mathjs when not needed', () => {
-    cy.get('script[src*="mathjs"]').should('have.length', 0);
-  });
-


Note to other reviewers: we're fully removing mathjs as a dependency right now due to bundle size and compatibility problems with Webpack 5. Related PR removing mathjs from superset-ui: apache-superset/superset-ui#1362

superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx

villebro · 2021-09-17T07:24:42Z

superset-frontend/webpack.config.js

-// clean up built assets if not from dev-server
-if (!isDevServer) {
-  plugins.push(
-    new CleanWebpackPlugin({
-      dry: false,
-      // required because the build directory is outside the frontend directory:
-      dangerouslyAllowCleanPatternsOutsideProject: true,
-    }),
-  );
-}
-


Is this no longer needed? I remember this having been a problem before causing lots of accumulated old bundles in the build directory.

Webpack 5 has built-in cleaning capabilities. This line replaces the plugin:

jinghua-qa · 2021-09-17T07:45:17Z

export sample dashboard from homescreen is loading forever. double checked on production, working properly. not sure this is related to this PR, please confirm. also the file i downloaded is in JSON format, should be .zip instead

Screen.Recording.2021-09-16.at.11.15.38.AM.mov

Also saw loading forever issue when export datast

villebro · 2021-09-17T08:35:42Z

/testenv up FEATURE_DASHBOARD_NATIVE_FILTERS=true FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_UX_BETA=true FEATURE_ENABLE_EXPLORE_DRAG_AND_DROP=true FEATURE_ENABLE_TEMPLATE_PROCESSING=true FEATURE_SQLLAB_BACKEND_PERSISTENCE=true

github-actions · 2021-09-17T08:40:10Z

@villebro Ephemeral environment spinning up at http://34.219.36.79:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

villebro · 2021-09-17T08:40:46Z

/testenv up FEATURE_DASHBOARD_NATIVE_FILTERS=true FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_UX_BETA=true FEATURE_ENABLE_EXPLORE_DRAG_AND_DROP=true FEATURE_ENABLE_TEMPLATE_PROCESSING=true FEATURE_SQLLAB_BACKEND_PERSISTENCE=true FEATURE_ENABLE_DND_WITH_CLICK_UX=true

github-actions · 2021-09-17T08:45:09Z

@villebro Ephemeral environment spinning up at http://34.217.195.114:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

kgabryje · 2021-09-19T08:27:18Z

@etr2460 Tests added. Would be great to improve them later with your real-life examples!
CC @villebro

github-actions · 2021-09-22T11:25:19Z

Ephemeral environment shutdown and build artifacts deleted.

* Upgrade Webpack to v5 * Remove mapbox hack * Replace url-loaders and file-loaders with asset modules * Remove 1 rule * Change --colors to --color * Remove invalid option (--no-progress) * Remove url-loader, bump plugin * Fix AnnotationLayer formula check * Remove redundant tests * Bump cypress packages * Remove old comment * Fix tests * Remove checks for number of scripts in markdown test * Cosmetic changes * Add tests * Fix test * Fix test * Fixes test warnings * disable flaky test Co-authored-by: Ville Brofeldt <ville.v.brofeldt@gmail.com> Co-authored-by: Michael S. Molina <michael.s.molina@gmail.com>

pull-request-size bot added the size/L label Sep 14, 2021

kgabryje requested review from ktmud, rusackas, michael-s-molina, etr2460, suddjian and villebro and removed request for ktmud and rusackas September 14, 2021 13:47

etr2460 reviewed Sep 14, 2021

View reviewed changes

kgabryje force-pushed the chore/webpack5 branch from 671b8bf to c34d5fb Compare September 16, 2021 09:28

villebro mentioned this pull request Sep 16, 2021

chore: replace mathjs with mexp apache-superset/superset-ui#1362

Merged

junlincc added hold:testing! On hold for testing hold! On hold test_priority:high labels Sep 16, 2021

villebro reviewed Sep 17, 2021

View reviewed changes

kgabryje added 8 commits September 19, 2021 10:22

Fix AnnotationLayer formula check

e84c8b4

Remove redundant tests

6684bf8

Bump cypress packages

c7b4cbc

Remove old comment

c241f4c

Fix tests

5373437

Remove checks for number of scripts in markdown test

086e279

Cosmetic changes

a7575bb

Add tests

169a2b3

kgabryje force-pushed the chore/webpack5 branch from 645bfe3 to 169a2b3 Compare September 19, 2021 08:26

kgabryje and others added 3 commits September 19, 2021 12:30

Fix test

832aa9f

Fix test

05b1679

Merge branch 'master' into chore/webpack5

5235135

villebro mentioned this pull request Sep 20, 2021

ci: bump npm to version 7 #16748

Merged

9 tasks

villebro and others added 2 commits September 20, 2021 15:02

Merge branch 'master' into chore/webpack5

39811bf

Fixes test warnings

29940b5

junlincc removed hold! On hold hold:testing! On hold for testing test_priority:high labels Sep 20, 2021

disable flaky test

46595d2

villebro merged commit 486e0d4 into apache:master Sep 22, 2021

michael-s-molina mentioned this pull request Sep 27, 2021

chore: Upgrades Storybook to version 6.3.8 to make it compatible with Webpack 5 #16857

Merged

9 tasks

This was referenced Sep 28, 2021

fix(build): enable hot reloading of linked packages #16867

Merged

fix(build): make npm linking work pt. 2 #16958

Merged

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Upgrade Webpack to v5 #16701

chore: Upgrade Webpack to v5 #16701

kgabryje commented Sep 14, 2021 •

edited

Loading

codecov bot commented Sep 14, 2021 •

edited

Loading

kgabryje commented Sep 14, 2021

github-actions bot commented Sep 14, 2021

etr2460 left a comment

etr2460 Sep 14, 2021

junlincc commented Sep 16, 2021

github-actions bot commented Sep 16, 2021

junlincc commented Sep 16, 2021 •

edited

Loading

villebro commented Sep 17, 2021

villebro left a comment

villebro Sep 17, 2021

kgabryje Sep 17, 2021

villebro Sep 17, 2021

villebro Sep 17, 2021

kgabryje Sep 17, 2021

jinghua-qa commented Sep 17, 2021

villebro commented Sep 17, 2021

github-actions bot commented Sep 17, 2021

villebro commented Sep 17, 2021

github-actions bot commented Sep 17, 2021

kgabryje commented Sep 19, 2021

github-actions bot commented Sep 22, 2021

chore: Upgrade Webpack to v5 #16701

chore: Upgrade Webpack to v5 #16701

Conversation

kgabryje commented Sep 14, 2021 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Sep 14, 2021 • edited Loading

Codecov Report

kgabryje commented Sep 14, 2021

github-actions bot commented Sep 14, 2021

etr2460 left a comment

Choose a reason for hiding this comment

etr2460 Sep 14, 2021

Choose a reason for hiding this comment

junlincc commented Sep 16, 2021

github-actions bot commented Sep 16, 2021

junlincc commented Sep 16, 2021 • edited Loading

villebro commented Sep 17, 2021

villebro left a comment

Choose a reason for hiding this comment

villebro Sep 17, 2021

Choose a reason for hiding this comment

kgabryje Sep 17, 2021

Choose a reason for hiding this comment

villebro Sep 17, 2021

Choose a reason for hiding this comment

villebro Sep 17, 2021

Choose a reason for hiding this comment

kgabryje Sep 17, 2021

Choose a reason for hiding this comment

jinghua-qa commented Sep 17, 2021

villebro commented Sep 17, 2021

github-actions bot commented Sep 17, 2021

villebro commented Sep 17, 2021

github-actions bot commented Sep 17, 2021

kgabryje commented Sep 19, 2021

github-actions bot commented Sep 22, 2021

kgabryje commented Sep 14, 2021 •

edited

Loading

codecov bot commented Sep 14, 2021 •

edited

Loading

junlincc commented Sep 16, 2021 •

edited

Loading