Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade Webpack to the v5 #16113

Merged
merged 25 commits into from Jul 19, 2021
Merged

Upgrade Webpack to the v5 #16113

merged 25 commits into from Jul 19, 2021

Conversation

nemanjaglumac
Copy link
Member

@nemanjaglumac nemanjaglumac commented May 17, 2021

What does this PR try to accomplish?

  • Upgrade Webpack to the latest version (5)

How to follow the changes in this PR?

  • My suggestion is that you digest this commit by commit, and do it in the same order that commits have been made
  • I've included A LOT of logs and comments, trying to document every single step
    • Looking all of them at the same time will be impossible to follow
    • Every next commit is the logical next step that resulted from the previous one (hence the suggestion to follow them in the same order)

Comments (steps)

  1. 3504153#r633726539
  2. bcfe606#r633727551
  3. 273e943#r633728108
  4. 6009c5d#r633729719
  5. 1746fd2#r633730820
  6. eacd778#r633731338
  7. e5c1f83#r633731960
  8. bf74488#r633732359
  9. ce89e3f#r633732795
  10. 1ca8280#r633733392
  11. 778ed18#r633734344
  12. 9281059#r633734878
  13. 0a4e9c3#r633735428
  14. 847955b#r633737220 (this is the first build that succeeds but renders the blank page)
  15. https://github.com/metabase/metabase/pull/16113/files/9fdc4a169c29684c83056702799a1e0a954dfb06#r633737766

TODO

  • Fix Uberjar build on CircleCI (local build and GitHub Actions work fine)
  • Fix incorrect text rendering (in some cases, e.g. tables, text is using the bold variant)
  • Fix a few broken tests in smoke tests

@@ -148,7 +148,7 @@
"style-loader": "^0.19.0",
"uglifyjs-webpack-plugin": "^1.0.0",
"unused-files-webpack-plugin": "^3.0.0",
"webpack": "^3.8.1",
"webpack": "^5.37.0",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upgrade Webpack to the latest version

yarn upgrade webpack@latest

warning " > @cypress/webpack-preprocessor@4.1.5" has incorrect peer dependency "webpack@^4.18.1".
warning " > @testing-library/cypress@5.3.1" has incorrect peer dependency "cypress@^2.1.0 || ^3.0.0 || ^4.0.0".
warning " > babel-loader@7.1.5" has incorrect peer dependency "webpack@2 || 3 || 4".
warning " > extract-text-webpack-plugin@3.0.2" has incorrect peer dependency "webpack@^3.1.0".
warning " > html-webpack-plugin@2.30.1" has incorrect peer dependency "webpack@1 || ^2 || ^2.1.0-beta || ^2.2.0-rc || ^3".
warning " > postcss-cssnext@2.11.0" has unmet peer dependency "caniuse-db@^1.0.30000652".
warning " > uglifyjs-webpack-plugin@1.3.0" has incorrect peer dependency "webpack@^2.0.0 || ^3.0.0 || ^4.0.0".
warning " > webpack-dev-server@2.11.5" has incorrect peer dependency "webpack@^2.2.0 || ^3.0.0".
warning "webpack-dev-server > webpack-dev-middleware@1.12.2" has incorrect peer dependency "webpack@^1.0.0 || ^2.0.0 || ^3.0.0".

Running yarn-dev-ee after this produces the following error:

[frontend] internal/modules/cjs/loader.js:883
[frontend]   throw err;
[frontend]   ^
[frontend] 
[frontend] Error: Cannot find module 'webpack/bin/config-yargs'
[frontend] Require stack:
[frontend] - /Users/offbynone/code/work/metabase/node_modules/webpack-dev-server/bin/webpack-dev-server.js
[frontend]     at Function.Module._resolveFilename (internal/modules/cjs/loader.js:880:15)
[frontend]     at Function.Module._load (internal/modules/cjs/loader.js:725:27)
[frontend]     at Module.require (internal/modules/cjs/loader.js:952:19)
[frontend]     at require (internal/modules/cjs/helpers.js:88:18)
[frontend]     at Object.<anonymous> (/Users/offbynone/code/work/metabase/node_modules/webpack-dev-server/bin/webpack-dev-server.js:54:1)
[frontend]     at Module._compile (internal/modules/cjs/loader.js:1063:30)
[frontend]     at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
[frontend]     at Module.load (internal/modules/cjs/loader.js:928:32)
[frontend]     at Function.Module._load (internal/modules/cjs/loader.js:769:14)
[frontend]     at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12) {
[frontend]   code: 'MODULE_NOT_FOUND',
[frontend]   requireStack: [
[frontend]     '/Users/offbynone/code/work/metabase/node_modules/webpack-dev-server/bin/webpack-dev-server.js'
[frontend]   ]
[frontend] }
error Command failed with exit code 1.

Searching online for a solution:

If you're using webpack-cli 4 or webpack 5, change webpack-dev-server to webpack serve.

The next logical step: upgrade webpack-dev-server

@@ -149,7 +149,7 @@
"uglifyjs-webpack-plugin": "^1.0.0",
"unused-files-webpack-plugin": "^3.0.0",
"webpack": "^5.37.0",
"webpack-dev-server": "^2.9.1",
"webpack-dev-server": "^3.11.2",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upgrade webpack-dev-server to the latest version

yarn upgrade webpack-dev-server@latest

warning webpack-dev-server > chokidar@2.1.8: Chokidar 2 will break on node v14+. Upgrade to chokidar 3 with 15x less dependencies.
[3/5] 🚚  Fetching packages...
[4/5] 🔗  Linking dependencies...
warning " > @cypress/webpack-preprocessor@4.1.5" has incorrect peer dependency "webpack@^4.18.1".
warning " > @testing-library/cypress@5.3.1" has incorrect peer dependency "cypress@^2.1.0 || ^3.0.0 || ^4.0.0".
warning " > babel-loader@7.1.5" has incorrect peer dependency "webpack@2 || 3 || 4".
warning " > extract-text-webpack-plugin@3.0.2" has incorrect peer dependency "webpack@^3.1.0".
warning " > html-webpack-plugin@2.30.1" has incorrect peer dependency "webpack@1 || ^2 || ^2.1.0-beta || ^2.2.0-rc || ^3".
warning " > postcss-cssnext@2.11.0" has unmet peer dependency "caniuse-db@^1.0.30000652".
warning " > uglifyjs-webpack-plugin@1.3.0" has incorrect peer dependency "webpack@^2.0.0 || ^3.0.0 || ^4.0.0".

Running yarn-dev-ee after this produces the following error:

[frontend] CLI for webpack must be installed.
[frontend]   webpack-cli (https://github.com/webpack/webpack-cli)

@@ -149,6 +149,7 @@
"uglifyjs-webpack-plugin": "^1.0.0",
"unused-files-webpack-plugin": "^3.0.0",
"webpack": "^5.37.0",
"webpack-cli": "^4.7.0",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Install webpack-cli

yarn add -D webpack-cli

warning " > @cypress/webpack-preprocessor@4.1.5" has incorrect peer dependency "webpack@^4.18.1".
warning " > @testing-library/cypress@5.3.1" has incorrect peer dependency "cypress@^2.1.0 || ^3.0.0 || ^4.0.0".
warning " > babel-loader@7.1.5" has incorrect peer dependency "webpack@2 || 3 || 4".
warning " > extract-text-webpack-plugin@3.0.2" has incorrect peer dependency "webpack@^3.1.0".
warning " > html-webpack-plugin@2.30.1" has incorrect peer dependency "webpack@1 || ^2 || ^2.1.0-beta || ^2.2.0-rc || ^3".
warning " > postcss-cssnext@2.11.0" has unmet peer dependency "caniuse-db@^1.0.30000652".
warning " > uglifyjs-webpack-plugin@1.3.0" has incorrect peer dependency "webpack@^2.0.0 || ^3.0.0 || ^4.0.0".

Running yarn-dev-ee after this produces the following error:

[cljs] .[webpack-cli] Failed to load '/Users/offbynone/code/work/metabase/webpack.config.js' config
[cljs] .[webpack-cli] TypeError: webpack.optimize.CommonsChunkPlugin is not a constructor
[frontend]     at Object.<anonymous> (/Users/offbynone/code/work/metabase/webpack.config.js:129:5)
[frontend]     at Module._compile (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/node_modules/v8-compile-cache/v8-compile-cache.js:192:30)
[frontend]     at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
[frontend]     at Module.load (internal/modules/cjs/loader.js:928:32)
[frontend]     at Function.Module._load (internal/modules/cjs/loader.js:769:14)
[frontend]     at Module.require (internal/modules/cjs/loader.js:952:19)
[frontend]     at require (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/node_modules/v8-compile-cache/v8-compile-cache.js:159:20)
[frontend]     at loadConfig (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/lib/webpack-cli.js:1327:31)
[frontend]     at WebpackCLI.resolveConfig (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/lib/webpack-cli.js:1459:44)
[frontend]     at WebpackCLI.createCompiler (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/lib/webpack-cli.js:1837:33)
[frontend]     at Command.<anonymous> (/Users/offbynone/code/work/metabase/node_modules/@webpack-cli/serve/lib/index.js:81:40)
[frontend]     at Command.listener [as _actionHandler] (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/node_modules/commander/index.js:922:31)
[frontend]     at Command._parseCommand (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/node_modules/commander/index.js:1503:14)
[frontend]     at Command._dispatchSubcommand (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/node_modules/commander/index.js:1443:18)
[frontend]     at Command._parseCommand (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/node_modules/commander/index.js:1460:12)
[frontend]     at Command.parse (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/node_modules/commander/index.js:1292:10)
error Command failed with exit code 2.

Related reading material:

Warning
The CommonsChunkPlugin has been removed in webpack v4 legato. To learn how chunks are treated in the latest version, check out the SplitChunksPlugin.

Comment on lines +128 to +139
optimization: {
splitChunks: {
chunks: "all",
},
},

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace CommonsChunkPlugin with splitCunks

[cljs] shadow-cljs - server starting ...[webpack-cli] Failed to load '/Users/offbynone/code/work/metabase/webpack.config.js' config
[frontend] [webpack-cli] TypeError: webpack.NamedModulesPlugin is not a constructor
[frontend]     at Object.<anonymous> (/Users/offbynone/code/work/metabase/webpack.config.js:265:5)
[frontend]     at Module._compile (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/node_modules/v8-compile-cache/v8-compile-cache.js:192:30)
[frontend]     at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
[frontend]     at Module.load (internal/modules/cjs/loader.js:928:32)
[frontend]     at Function.Module._load (internal/modules/cjs/loader.js:769:14)
[frontend]     at Module.require (internal/modules/cjs/loader.js:952:19)
[frontend]     at require (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/node_modules/v8-compile-cache/v8-compile-cache.js:159:20)
[frontend]     at loadConfig (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/lib/webpack-cli.js:1327:31)
[frontend]     at WebpackCLI.resolveConfig (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/lib/webpack-cli.js:1459:44)
[frontend]     at WebpackCLI.createCompiler (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/lib/webpack-cli.js:1837:33)
[frontend]     at Command.<anonymous> (/Users/offbynone/code/work/metabase/node_modules/@webpack-cli/serve/lib/index.js:81:40)
[frontend]     at Command.listener [as _actionHandler] (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/node_modules/commander/index.js:922:31)
[frontend]     at Command._parseCommand (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/node_modules/commander/index.js:1503:14)
[frontend]     at Command._dispatchSubcommand (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/node_modules/commander/index.js:1443:18)
[frontend]     at Command._parseCommand (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/node_modules/commander/index.js:1460:12)
[frontend]     at Command.parse (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/node_modules/commander/index.js:1292:10)
error Command failed with exit code 2.

Searching online for a solution:

This plugin was removed, you even don’t need this anymore, we use better logic for long term caching out of box

@@ -262,7 +262,6 @@ if (NODE_ENV === "hot") {

config.plugins.unshift(
new webpack.NoEmitOnErrorsPlugin(),
new webpack.NamedModulesPlugin(),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove NamedModulesPlugin (1746fd2)

[frontend] [webpack-cli] TypeError: compiler.plugin is not a function
[frontend]     at UnusedFilesWebpackPlugin.apply (/Users/offbynone/code/work/metabase/node_modules/unused-files-webpack-plugin/lib/index__8.0.0__.js:105:14)
[frontend]     at createCompiler (/Users/offbynone/code/work/metabase/node_modules/webpack/lib/webpack.js:74:12)
[frontend]     at create (/Users/offbynone/code/work/metabase/node_modules/webpack/lib/webpack.js:123:16)
[frontend]     at webpack (/Users/offbynone/code/work/metabase/node_modules/webpack/lib/webpack.js:147:32)
[frontend]     at WebpackCLI.f [as webpack] (/Users/offbynone/code/work/metabase/node_modules/webpack/lib/index.js:54:15)
[frontend]     at WebpackCLI.createCompiler (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/lib/webpack-cli.js:1845:29)
[frontend]     at Command.<anonymous> (/Users/offbynone/code/work/metabase/node_modules/@webpack-cli/serve/lib/index.js:81:30)
[frontend]     at async Promise.all (index 1)
[frontend]     at Command.<anonymous> (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/lib/webpack-cli.js:1289:13)
error Command failed with exit code 2.

UnusedFilesWebpackPlugin is still not compatible with the Webpack 5. Not sure will they ever update it. It seems like it's currently stuck.

As a temporary solution, I'll remove it.
Possible alternative?:

@@ -147,7 +147,6 @@
"shadow-cljs": "2.11.20",
"style-loader": "^0.19.0",
"uglifyjs-webpack-plugin": "^1.0.0",
"unused-files-webpack-plugin": "^3.0.0",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove UnusedFilesWebpackPlugin

warning " > @cypress/webpack-preprocessor@4.1.5" has incorrect peer dependency "webpack@^4.18.1".
warning " > @testing-library/cypress@5.3.1" has incorrect peer dependency "cypress@^2.1.0 || ^3.0.0 || ^4.0.0".
warning " > babel-loader@7.1.5" has incorrect peer dependency "webpack@2 || 3 || 4".
warning " > extract-text-webpack-plugin@3.0.2" has incorrect peer dependency "webpack@^3.1.0".
warning " > html-webpack-plugin@2.30.1" has incorrect peer dependency "webpack@1 || ^2 || ^2.1.0-beta || ^2.2.0-rc || ^3".
warning " > postcss-cssnext@2.11.0" has unmet peer dependency "caniuse-db@^1.0.30000652".
warning " > uglifyjs-webpack-plugin@1.3.0" has incorrect peer dependency "webpack@^2.0.0 || ^3.0.0 || ^4.0.0".

Running yarn-dev-ee after this produces the following error:

[frontend] [webpack-cli] TypeError: compiler.plugin is not a function
[frontend]     at ExtractTextPlugin.apply (/Users/offbynone/code/work/metabase/node_modules/extract-text-webpack-plugin/dist/index.js:147:16)
[frontend]     at createCompiler (/Users/offbynone/code/work/metabase/node_modules/webpack/lib/webpack.js:74:12)
[frontend]     at create (/Users/offbynone/code/work/metabase/node_modules/webpack/lib/webpack.js:123:16)
[frontend]     at webpack (/Users/offbynone/code/work/metabase/node_modules/webpack/lib/webpack.js:147:32)
[frontend]     at WebpackCLI.f [as webpack] (/Users/offbynone/code/work/metabase/node_modules/webpack/lib/index.js:54:15)
[frontend]     at WebpackCLI.createCompiler (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/lib/webpack-cli.js:1845:29)
[frontend]     at Command.<anonymous> (/Users/offbynone/code/work/metabase/node_modules/@webpack-cli/serve/lib/index.js:81:30)
[frontend]     at async Promise.all (index 1)
[frontend]     at Command.<anonymous> (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/lib/webpack-cli.js:1289:13)
error Command failed with exit code 2.

Related reading:

⚠️ Since webpack v4 the extract-text-webpack-plugin should not be used for css. Use mini-css-extract-plugin instead.

@@ -7,7 +7,7 @@ require("regenerator-runtime/runtime");

const webpack = require("webpack");

const ExtractTextPlugin = require("extract-text-webpack-plugin");
const MiniCssExtractPlugin = require("mini-css-extract-plugin");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace extract-text-webpack-plugin with mini-css-extract-plugin

yarn add mini-css-extract-plugin -D

warning " > @cypress/webpack-preprocessor@4.1.5" has incorrect peer dependency "webpack@^4.18.1".
warning " > @testing-library/cypress@5.3.1" has incorrect peer dependency "cypress@^2.1.0 || ^3.0.0 || ^4.0.0".
warning " > babel-loader@7.1.5" has incorrect peer dependency "webpack@2 || 3 || 4".
warning " > extract-text-webpack-plugin@3.0.2" has incorrect peer dependency "webpack@^3.1.0".
warning " > html-webpack-plugin@2.30.1" has incorrect peer dependency "webpack@1 || ^2 || ^2.1.0-beta || ^2.2.0-rc || ^3".
warning " > postcss-cssnext@2.11.0" has unmet peer dependency "caniuse-db@^1.0.30000652".
warning " > uglifyjs-webpack-plugin@1.3.0" has incorrect peer dependency "webpack@^2.0.0 || ^3.0.0 || ^4.0.0".

yarn remove extract-text-webpack-plugin

warning " > @cypress/webpack-preprocessor@4.1.5" has incorrect peer dependency "webpack@^4.18.1".
warning " > @testing-library/cypress@5.3.1" has incorrect peer dependency "cypress@^2.1.0 || ^3.0.0 || ^4.0.0".
warning " > babel-loader@7.1.5" has incorrect peer dependency "webpack@2 || 3 || 4".
warning " > html-webpack-plugin@2.30.1" has incorrect peer dependency "webpack@1 || ^2 || ^2.1.0-beta || ^2.2.0-rc || ^3".
warning " > postcss-cssnext@2.11.0" has unmet peer dependency "caniuse-db@^1.0.30000652".
warning " > uglifyjs-webpack-plugin@1.3.0" has incorrect peer dependency "webpack@^2.0.0 || ^3.0.0 || ^4.0.0".

Running yarn-dev-ee after this produces the following error:

[cljs] .[webpack-cli] TypeError: compiler.plugin is not a function
[frontend]     at HtmlWebpackPlugin.apply (/Users/offbynone/code/work/metabase/node_modules/html-webpack-plugin/index.js:45:12)
[frontend]     at createCompiler (/Users/offbynone/code/work/metabase/node_modules/webpack/lib/webpack.js:74:12)
[frontend]     at create (/Users/offbynone/code/work/metabase/node_modules/webpack/lib/webpack.js:123:16)
[frontend]     at webpack (/Users/offbynone/code/work/metabase/node_modules/webpack/lib/webpack.js:147:32)
[frontend]     at WebpackCLI.f [as webpack] (/Users/offbynone/code/work/metabase/node_modules/webpack/lib/index.js:54:15)
[frontend]     at WebpackCLI.createCompiler (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/lib/webpack-cli.js:1845:29)
[frontend]     at Command.<anonymous> (/Users/offbynone/code/work/metabase/node_modules/@webpack-cli/serve/lib/index.js:81:30)
[frontend]     at async Promise.all (index 1)
[frontend]     at Command.<anonymous> (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/lib/webpack-cli.js:1289:13)
error Command failed with exit code 2.

We're still at the v2.30.1 of HtmlWebpackPlugin, so let's first try to upgrade and see what happens.

@@ -128,7 +128,7 @@
"fs-promise": "^2.0.2",
"glob": "^7.1.1",
"html-webpack-harddisk-plugin": "^0.1.0",
"html-webpack-plugin": "^2.30.1",
"html-webpack-plugin": "^5.3.1",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upgrade html-webpack-plugin to the latest version (5.3.1)

warning " > @cypress/webpack-preprocessor@4.1.5" has incorrect peer dependency "webpack@^4.18.1".
warning " > @testing-library/cypress@5.3.1" has incorrect peer dependency "cypress@^2.1.0 || ^3.0.0 || ^4.0.0".
warning " > babel-loader@7.1.5" has incorrect peer dependency "webpack@2 || 3 || 4".
warning " > postcss-cssnext@2.11.0" has unmet peer dependency "caniuse-db@^1.0.30000652".
warning " > uglifyjs-webpack-plugin@1.3.0" has incorrect peer dependency "webpack@^2.0.0 || ^3.0.0 || ^4.0.0".

Running yarn-dev-ee after this produces the following error:

[cljs] .[webpack-cli] TypeError: compiler.plugin is not a function
[frontend]     at HtmlWebpackHarddiskPlugin.apply (/Users/offbynone/code/work/metabase/node_modules/html-webpack-harddisk-plugin/index.js:14:12)
[frontend]     at createCompiler (/Users/offbynone/code/work/metabase/node_modules/webpack/lib/webpack.js:74:12)
[frontend]     at create (/Users/offbynone/code/work/metabase/node_modules/webpack/lib/webpack.js:123:16)
[frontend]     at webpack (/Users/offbynone/code/work/metabase/node_modules/webpack/lib/webpack.js:147:32)
[frontend]     at WebpackCLI.f [as webpack] (/Users/offbynone/code/work/metabase/node_modules/webpack/lib/index.js:54:15)
[frontend]     at WebpackCLI.createCompiler (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/lib/webpack-cli.js:1845:29)
[frontend]     at Command.<anonymous> (/Users/offbynone/code/work/metabase/node_modules/@webpack-cli/serve/lib/index.js:81:30)
[frontend]     at async Promise.all (index 1)
[frontend]     at Command.<anonymous> (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/lib/webpack-cli.js:1289:13)
error Command failed with exit code 2.

The same thing. This plugin was quite outdated (v0.1.0). Let's try updating it to the latest version () to get the support for Webpack 5.

@@ -127,7 +127,7 @@
"file-loader": "^0.11.1",
"fs-promise": "^2.0.2",
"glob": "^7.1.1",
"html-webpack-harddisk-plugin": "^0.1.0",
"html-webpack-harddisk-plugin": "^2.0.0",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upgrade html-webpack-harddisk-plugin to the latest version (2.0.0)

warning " > @cypress/webpack-preprocessor@4.1.5" has incorrect peer dependency "webpack@^4.18.1".
warning " > @testing-library/cypress@5.3.1" has incorrect peer dependency "cypress@^2.1.0 || ^3.0.0 || ^4.0.0".
warning " > babel-loader@7.1.5" has incorrect peer dependency "webpack@2 || 3 || 4".
warning " > postcss-cssnext@2.11.0" has unmet peer dependency "caniuse-db@^1.0.30000652".
warning " > uglifyjs-webpack-plugin@1.3.0" has incorrect peer dependency "webpack@^2.0.0 || ^3.0.0 || ^4.0.0".

Running yarn-dev-ee after this produces the following error:

[cljs] ...[webpack-cli] TypeError: compiler.plugin is not a function
[frontend]     at BannerWebpackPlugin.apply (/Users/offbynone/code/work/metabase/node_modules/banner-webpack-plugin/index.js:16:14)
[frontend]     at createCompiler (/Users/offbynone/code/work/metabase/node_modules/webpack/lib/webpack.js:74:12)
[frontend]     at create (/Users/offbynone/code/work/metabase/node_modules/webpack/lib/webpack.js:123:16)
[frontend]     at webpack (/Users/offbynone/code/work/metabase/node_modules/webpack/lib/webpack.js:147:32)
[frontend]     at WebpackCLI.f [as webpack] (/Users/offbynone/code/work/metabase/node_modules/webpack/lib/index.js:54:15)
[frontend]     at WebpackCLI.createCompiler (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/lib/webpack-cli.js:1845:29)
[frontend]     at Command.<anonymous> (/Users/offbynone/code/work/metabase/node_modules/@webpack-cli/serve/lib/index.js:81:30)
[frontend]     at async Promise.all (index 1)
[frontend]     at Command.<anonymous> (/Users/offbynone/code/work/metabase/node_modules/webpack-cli/lib/webpack-cli.js:1289:13)
error Command failed with exit code 2.

BannerWebpackPlugin we're using was last updated on 25 Sep 2017. Webpack seems to have the their own banner plugin. Not quite sure why we chose to go with the 3rd party version. Probably because it allows to specify chunks individually.

Comment on lines +175 to +180
new webpack.BannerPlugin({
banner:
"/*\n* This file is subject to the terms and conditions defined in\n * file 'LICENSE.txt', which is part of this source code package.\n */\n",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove banner-webpack-plugin and use the default Webpack's banner plugin

warning " > @cypress/webpack-preprocessor@4.1.5" has incorrect peer dependency "webpack@^4.18.1".
warning " > @testing-library/cypress@5.3.1" has incorrect peer dependency "cypress@^2.1.0 || ^3.0.0 || ^4.0.0".
warning " > babel-loader@7.1.5" has incorrect peer dependency "webpack@2 || 3 || 4".
warning " > postcss-cssnext@2.11.0" has unmet peer dependency "caniuse-db@^1.0.30000652".
warning " > uglifyjs-webpack-plugin@1.3.0" has incorrect peer dependency "webpack@^2.0.0 || ^3.0.0 || ^4.0.0".

#TODO:

  • Find a way to have a custom banner for app embed
    "/*\n* This file is subject to the terms and conditions defined in\n * file 'LICENSE-EMBEDDING.txt', which is part of this source code package.\n */\n",

Running yarn-dev-ee after this finally starts really long process of building, but the first warning I see is:

[frontend] (node:12557) [DEP_WEBPACK_TEMPLATE_PATH_PLUGIN_REPLACE_PATH_VARIABLES_HASH] DeprecationWarning: [hash] is now [fullhash] (also consider using [chunkhash] or [contenthash], see documentation for details)

@@ -59,7 +59,7 @@ const config = (module.exports = {
output: {
path: BUILD_PATH + "/app/dist",
// NOTE: the filename on disk won't include "?[chunkhash]" but the URL in index.html generated by HtmlWebpackPlugin will:
filename: "[name].bundle.js?[hash]",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace deprecated [hash] in the config

[frontend] ℹ 「wds」: Project is running at http://0.0.0.0:8080/
[frontend] ℹ 「wds」: webpack output is served from http://localhost:8080/app/dist/
[frontend] ℹ 「wds」: Content not from webpack is served from frontend

Loading plugins succeeded

[backend] 2021-05-17 16:31:27,127 INFO metabase.core :: Metabase Initialization COMPLETE

Errors:

[frontend] ✖ 「wdm」: Entrypoint app-main = 999.hot.bundle.js?c0e03a66f81dc341c630 66.hot.bundle.js?2592bdeea40df0a3f32b 358.hot.bundle.js?6c5b3dd6e16a099eeddb 968.hot.bundle.js?65f1efd13fcfb47c152d app-main.hot.bundle.js?3292e6c8d8b2b4e1ea0f 5 auxiliary assets
[frontend] Entrypoint app-public = 999.hot.bundle.js?c0e03a66f81dc341c630 66.hot.bundle.js?2592bdeea40df0a3f32b 968.hot.bundle.js?65f1efd13fcfb47c152d 417.hot.bundle.js?8f5b08420e667863af0e app-public.hot.bundle.js?6419673902b0e68bb825 5 auxiliary assets
[frontend] Entrypoint app-embed = 999.hot.bundle.js?c0e03a66f81dc341c630 66.hot.bundle.js?2592bdeea40df0a3f32b 968.hot.bundle.js?65f1efd13fcfb47c152d 417.hot.bundle.js?8f5b08420e667863af0e app-embed.hot.bundle.js?c8a820f3f83504b744c2 5 auxiliary assets
[frontend] Entrypoint styles = 999.hot.bundle.js?c0e03a66f81dc341c630 styles.hot.bundle.js?0f3376f9117758e07c81 2 auxiliary assets

Warnings

[frontend] WARNING in DefinePlugin
[frontend] Conflicting values for 'process.env.NODE_ENV'
[frontend] WARNING in configuration
[frontend] The 'mode' option has not been set, webpack will fallback to 'production' for this value.
[frontend] Set 'mode' option to 'development' or 'production' to enable defaults for each environment.
[frontend] You can also set it to 'none' to disable any default behavior. Learn more: https://webpack.js.org/configuration/mode/
[frontend] 1 WARNING in child compilations (Use 'stats.children: true' resp. '--stats-children' for more details)
[frontend] 1 warning has detailed information that is not shown.
[frontend] Use 'stats.errorDetails: true' resp. '--stats-error-details' to show it.

Error:

[frontend] ERROR in ../../../node_modules/babel-plugin-transform-react-display-name/lib/index.js 95:12-27
[frontend] Module not found: Error: Can't resolve 'path' in '/Users/offbynone/code/work/metabase/node_modules/babel-plugin-transform-react-display-name/lib'
[frontend] BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
[frontend] This is no longer the case. Verify if you need this module and configure a polyfill for it.
[frontend] 
[frontend] If you want to include a polyfill, you need to:
[frontend]      - add a fallback 'resolve.fallback: { "path": require.resolve("path-browserify") }'
[frontend]      - install 'path-browserify'
[frontend] If you don't want to include a polyfill, you can use an empty module like this:
[frontend]      resolve.fallback: { "path": false }
[frontend]  @ ../../../node_modules/babel-preset-react/lib/index.js 17:44-96
[frontend]  @ ./internal/components/ScratchApp.jsx 40:24-53
[frontend]  @ ./internal/components/ sync (\w+)App.jsx$ ./ScratchApp.jsx
[frontend]  @ ./internal/routes.js 46:10-80
[frontend]  @ ./routes.jsx 465:24-67
[frontend]  @ ./app-main.js 7:14-40

@@ -44,6 +44,7 @@ const CSS_CONFIG = {
};

const config = (module.exports = {
mode: devMode ? "development" : "production",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add mode to the config

[frontend] ERROR in ../../../node_modules/leaflet/dist/images/layers-2x.png
[frontend] Module build failed (from ../../../node_modules/file-loader/index.js):
[frontend] TypeError: Cannot read property 'fileLoader' of undefined
[frontend]     at Object.module.exports (/Users/offbynone/code/work/metabase/node_modules/file-loader/index.js:14:28)
[frontend]  @ ../../../node_modules/css-loader/index.js??ruleSet[1].rules[4].use[1]!../../../node_modules/postcss-loader/lib/index.js!../../../node_modules/leaflet/dist/leaflet.css 7:8841-8874
[frontend]  @ ../../../node_modules/leaflet/dist/leaflet.css 4:14-128 18:2-22:4 19:20-134
[frontend]  @ ./visualizations/components/LeafletChoropleth.jsx 46:0-35
[frontend]  @ ./visualizations/components/ChoroplethMap.jsx 92:25-55
[frontend]  @ ./visualizations/visualizations/Map.jsx 85:21-59
[frontend]  @ ./visualizations/register.js 79:11-42
[frontend]  @ ./app.js 28:16-59
[frontend]  @ ./app-public.js 3:11-27

We're using file-loader version 0.11.1. The latest one is 6.2.0! Let's update it in the next step.

package.json Outdated
@@ -123,7 +123,7 @@
"eslint-plugin-import": "^2.22.0",
"eslint-plugin-react": "^7.22.0",
"eslint-plugin-react-hooks": "^4.2.0",
"file-loader": "^0.11.1",
"file-loader": "^6.2.0",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upgrade file-loader to the latest version 6.2.0

[frontend] ✖ 「wdm」: Entrypoint app-main = vendors-node_modules_css-loader_lib_css-base_js-node_modules_css-loader_lib_url_escape_js-nod-a5171a.hot.bundle.js?42dabef00344531a7f2f vendors-node_modules_ace-builds_src-min-noconflict_ace_js-node_modules_ace-builds_src-min-noc-2e5415.hot.bundle.js?a07e666296aee02b9af3 vendors-node_modules_babel_standalone_babel_js-node_modules_babel-preset-react_lib_index_js-n-e9bac3.hot.bundle.js?84b2e0e8efd2ee668d7f app_js-reducers-common_js-node_modules_moment-timezone_node_modules_moment_locale_sync_recurs-4e241d.hot.bundle.js?38d14bcb723ecb0a6ff6 app-main.hot.bundle.js?7c8bcf9cca422045e832 24 auxiliary assets
[frontend] Entrypoint app-public = vendors-node_modules_css-loader_lib_css-base_js-node_modules_css-loader_lib_url_escape_js-nod-a5171a.hot.bundle.js?42dabef00344531a7f2f vendors-node_modules_ace-builds_src-min-noconflict_ace_js-node_modules_ace-builds_src-min-noc-2e5415.hot.bundle.js?a07e666296aee02b9af3 app_js-reducers-common_js-node_modules_moment-timezone_node_modules_moment_locale_sync_recurs-4e241d.hot.bundle.js?38d14bcb723ecb0a6ff6 reducers-public_js-public_containers_PublicApp_jsx.hot.bundle.js?8e7a53ff75be8bff46e7 app-public.hot.bundle.js?b1df2b90da3c973642e3 24 auxiliary assets
[frontend] Entrypoint app-embed = vendors-node_modules_css-loader_lib_css-base_js-node_modules_css-loader_lib_url_escape_js-nod-a5171a.hot.bundle.js?42dabef00344531a7f2f vendors-node_modules_ace-builds_src-min-noconflict_ace_js-node_modules_ace-builds_src-min-noc-2e5415.hot.bundle.js?a07e666296aee02b9af3 app_js-reducers-common_js-node_modules_moment-timezone_node_modules_moment_locale_sync_recurs-4e241d.hot.bundle.js?38d14bcb723ecb0a6ff6 reducers-public_js-public_containers_PublicApp_jsx.hot.bundle.js?8e7a53ff75be8bff46e7 app-embed.hot.bundle.js?a3d022f0f923eb8196bc 24 auxiliary assets
[frontend] Entrypoint styles = vendors-node_modules_css-loader_lib_css-base_js-node_modules_css-loader_lib_url_escape_js-nod-a5171a.hot.bundle.js?42dabef00344531a7f2f styles.hot.bundle.js?3b11b47203fd50bcaf20 21 auxiliary assets
[frontend] 
[frontend] WARNING in DefinePlugin
[frontend] Conflicting values for 'process.env.NODE_ENV'
[frontend] 
[frontend] 1 warning has detailed information that is not shown.
[frontend] Use 'stats.errorDetails: true' resp. '--stats-error-details' to show it.
[frontend] 
[frontend] ERROR in ../../../node_modules/babel-plugin-transform-react-display-name/lib/index.js 95:12-27
[frontend] Module not found: Error: Can't resolve 'path' in '/Users/offbynone/code/work/metabase/node_modules/babel-plugin-transform-react-display-name/lib'
[frontend] 
[frontend] BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
[frontend] This is no longer the case. Verify if you need this module and configure a polyfill for it.
[frontend] 
[frontend] If you want to include a polyfill, you need to:
[frontend]      - add a fallback 'resolve.fallback: { "path": require.resolve("path-browserify") }'
[frontend]      - install 'path-browserify'
[frontend] If you don't want to include a polyfill, you can use an empty module like this:
[frontend]      resolve.fallback: { "path": false }
[frontend]  @ ../../../node_modules/babel-preset-react/lib/index.js 17:44-96
[frontend]  @ ./internal/components/ScratchApp.jsx 40:24-53
[frontend]  @ ./internal/components/ sync (\w+)App.jsx$ ./ScratchApp.jsx
[frontend]  @ ./internal/routes.js 46:10-80
[frontend]  @ ./routes.jsx 465:24-67
[frontend]  @ ./app-main.js 7:14-40
[frontend] 
[frontend] ERROR in ../../../node_modules/password-generator/lib/password-generator.js 99:18-47
[frontend] Module not found: Error: Can't resolve 'crypto' in '/Users/offbynone/code/work/metabase/node_modules/password-generator/lib'
[frontend] 
[frontend] BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
[frontend] This is no longer the case. Verify if you need this module and configure a polyfill for it.
[frontend] 
[frontend] If you want to include a polyfill, you need to:
[frontend]      - add a fallback 'resolve.fallback: { "crypto": require.resolve("crypto-browserify") }'
[frontend]      - install 'crypto-browserify'
[frontend] If you don't want to include a polyfill, you can use an empty module like this:
[frontend]      resolve.fallback: { "crypto": false }
[frontend]  @ ../../../node_modules/password-generator/index.js 1:0-52
[frontend]  @ ./lib/utils.js 20:25-54
[frontend]  @ ./lib/settings.js 29:13-42
[frontend]  @ ./app.js 48:16-48
[frontend]  @ ./app-main.js 5:11-34
[frontend] 
[frontend] ERROR in ../../../node_modules/vfile/core.js 3:11-26
[frontend] Module not found: Error: Can't resolve 'path' in '/Users/offbynone/code/work/metabase/node_modules/vfile'
[frontend] 
[frontend] BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
[frontend] This is no longer the case. Verify if you need this module and configure a polyfill for it.
[frontend] 
[frontend] If you want to include a polyfill, you need to:
[frontend]      - add a fallback 'resolve.fallback: { "path": require.resolve("path-browserify") }'
[frontend]      - install 'path-browserify'
[frontend] If you don't want to include a polyfill, you can use an empty module like this:
[frontend]      resolve.fallback: { "path": false }
[frontend]  @ ../../../node_modules/vfile/index.js 4:12-32
[frontend]  @ ../../../node_modules/unified/index.js 6:12-28
[frontend]  @ ../../../node_modules/react-markdown/lib/react-markdown.js 6:14-32
[frontend]  @ ./visualizations/visualizations/Text.jsx 38:21-46
[frontend]  @ ./visualizations/register.js 47:12-44
[frontend]  @ ./app.js 28:16-59
[frontend]  @ ./app-main.js 5:11-34
[frontend] 
[frontend] ERROR in ../../../node_modules/vfile/node_modules/replace-ext/index.js 3:11-26
[frontend] Module not found: Error: Can't resolve 'path' in '/Users/offbynone/code/work/metabase/node_modules/vfile/node_modules/replace-ext'
[frontend] 
[frontend] BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
[frontend] This is no longer the case. Verify if you need this module and configure a polyfill for it.
[frontend] 
[frontend] If you want to include a polyfill, you need to:
[frontend]      - add a fallback 'resolve.fallback: { "path": require.resolve("path-browserify") }'
[frontend]      - install 'path-browserify'
[frontend] If you don't want to include a polyfill, you can use an empty module like this:
[frontend]      resolve.fallback: { "path": false }
[frontend]  @ ../../../node_modules/vfile/core.js 4:14-36
[frontend]  @ ../../../node_modules/vfile/index.js 4:12-32
[frontend]  @ ../../../node_modules/unified/index.js 6:12-28
[frontend]  @ ../../../node_modules/react-markdown/lib/react-markdown.js 6:14-32
[frontend]  @ ./visualizations/visualizations/Text.jsx 38:21-46
[frontend]  @ ./visualizations/register.js 47:12-44
[frontend]  @ ./app.js 28:16-59
[frontend]  @ ./app-main.js 5:11-34
[frontend] 
[frontend] 4 errors have detailed information that is not shown.
[frontend] Use 'stats.errorDetails: true' resp. '--stats-error-details' to show it.
[frontend] 
[frontend] webpack 5.37.0 compiled with 4 errors and 1 warning in 1966 ms
[frontend] ℹ 「wdm」: Failed to compile.

Further reading:
https://webpack.js.org/blog/2020-10-10-webpack-5-release/#automatic-nodejs-polyfills-removed

MIGRATION:

  • Try to use frontend-compatible modules whenever possible.
  • It's possible to manually add a polyfill for a Node.js core module. An error message will give a hint on how to achieve that.
  • Package authors: Use the browser field in package.json to make a package frontend-compatible. Provide alternative implementations/dependencies for the browser.

Comment on lines 130 to 133
fallback: {
path: false,
crypto: false,
},
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't include polyfills for path and crypto

The title should've probably said: "Try not including polyfills for path and crypto. After this change, Webpack finally finished the whole build for the first time, but the output is simply a blank page. Many things could've gone wrong so far, but I think we will be forced to finally upgrade babel as well :/

[frontend] <s> [webpack.Progress] 100% 
[frontend] 
[frontend] ⚠ 「wdm」: Entrypoint app-main 25.3 MiB (23.1 MiB) = 6 assets 25 auxiliary assets
[frontend] Entrypoint app-public 18 MiB (17.2 MiB) = 6 assets 25 auxiliary assets
[frontend] Entrypoint app-embed 18 MiB (17.2 MiB) = 6 assets 25 auxiliary assets
[frontend] Entrypoint styles 556 KiB (1.22 MiB) = vendors-node_modules_css-loader_lib_css-base_js-node_modules_css-loader_lib_url_escape_js-nod-a5171a.hot.bundle.js?42dabef00344531a7f2f 368 KiB styles.hot.bundle.js?503d45e5f8d42a08fe8f 187 KiB styles.d67114e33a9c0b281197.hot-update.js 527 bytes 22 auxiliary assets
[frontend] 
[frontend] WARNING in DefinePlugin
[frontend] Conflicting values for 'process.env.NODE_ENV'
[frontend] 
[frontend] 1 warning has detailed information that is not shown.
[frontend] Use 'stats.errorDetails: true' resp. '--stats-error-details' to show it.
[frontend] 
[frontend] webpack 5.37.0 compiled with 1 warning in 2298 ms
[frontend] ℹ 「wdm」: Compiled with warnings.

@@ -145,7 +145,6 @@
"react-test-renderer": "~16.14.0",
"shadow-cljs": "2.11.20",
"style-loader": "^0.19.0",
"uglifyjs-webpack-plugin": "^1.0.0",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove uglifyjs-webpack-plugin

warning " > @cypress/webpack-preprocessor@4.1.5" has incorrect peer dependency "webpack@^4.18.1".
warning " > @testing-library/cypress@5.3.1" has incorrect peer dependency "cypress@^2.1.0 || ^3.0.0 || ^4.0.0".
warning " > babel-loader@7.1.5" has incorrect peer dependency "webpack@2 || 3 || 4".
warning " > postcss-cssnext@2.11.0" has unmet peer dependency "caniuse-db@^1.0.30000652".

Related reading material:

If you are using webpack v5 or above you do not need to install this plugin. Webpack v5 comes with the latest terser-webpack-plugin out of the box.

@nemanjaglumac nemanjaglumac changed the title Upgrade webpack Upgrade webpack [WIP] May 17, 2021
@paoliniluis
Copy link
Contributor

leaving something here that "may" be fixed with this upgrade as well #13219

@codecov
Copy link

codecov bot commented Jun 3, 2021

Codecov Report

Merging #16113 (593888c) into master (0195771) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 593888c differs from pull request most recent head db49839. Consider uploading reports for the commit db49839 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16113      +/-   ##
==========================================
- Coverage   84.96%   84.96%   -0.01%     
==========================================
  Files         416      416              
  Lines       33215    33279      +64     
  Branches     2382     2390       +8     
==========================================
+ Hits        28221    28275      +54     
- Misses       2612     2614       +2     
- Partials     2382     2390       +8     
Impacted Files Coverage Δ
...nd/src/metabase_enterprise/audit/pages/queries.clj 95.50% <100.00%> (-0.05%) ⬇️
src/metabase/pulse/render.clj 81.05% <0.00%> (-4.01%) ⬇️
src/metabase/task/sync_databases.clj 74.10% <0.00%> (-3.60%) ⬇️
src/metabase/automagic_dashboards/filters.clj 80.30% <0.00%> (-1.52%) ⬇️
src/metabase/pulse/render/body.clj 80.71% <0.00%> (-1.34%) ⬇️
src/metabase/api/search.clj 86.19% <0.00%> (-0.23%) ⬇️
src/metabase/api/database.clj 86.95% <0.00%> (-0.13%) ⬇️
src/metabase/util/quotation.clj 100.00% <0.00%> (ø)
src/metabase/api/collection.clj 94.14% <0.00%> (+0.10%) ⬆️
src/metabase/email/messages.clj 85.66% <0.00%> (+0.13%) ⬆️
... and 7 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 fb0eab4...db49839. Read the comment docs.

@nemanjaglumac
Copy link
Member Author

Once we make this stable enough and make sure it's running correctly, the next step could be simplifying and dividing the config into 3 separate files. The gist of it is explained in this article:
https://webpack.js.org/guides/production/

tl;dr

  1. webpack.common.js
  2. webpack.dev.js
  3. webpack.prod.js

Unify separate configs with webpack-merge.

@daltojohnso
Copy link
Contributor

0c5cbf1

Using "asset/resource" instead of file-loader seems to solve the issue with fonts not loading.

@ariya ariya marked this pull request as ready for review July 19, 2021 20:14
@nemanjaglumac nemanjaglumac requested a review from a team July 19, 2021 20:31
Copy link
Contributor

@gusaiani gusaiani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well in my manual tests.

Code looks good.

Thanks for giving this the priority and putting in the serious work.

@nemanjaglumac nemanjaglumac changed the title Upgrade webpack [WIP] Upgrade Webpack to the v5 Jul 19, 2021
@nemanjaglumac
Copy link
Member Author

@ariya I've removed WIP from the title. Please, you have the honor of merging this :)

@@ -179,31 +172,20 @@ const config = (module.exports = {
outputPath: __dirname + "/resources/frontend_client/app/dist",
}),
new webpack.DefinePlugin({
"process.env": { NODE_ENV: JSON.stringify(NODE_ENV) },
process: { env: { NODE_ENV: JSON.stringify(NODE_ENV) } },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm getting a warning about conflicting NODE_ENV values that is resolved by removing this line. I think webpack sets this on its own so no reason to add it via DefinePlugin

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found this

https://webpack.js.org/plugins/define-plugin/

Warning
When defining values for process prefer 'process.env.NODE_ENV': JSON.stringify('production') over process: { env: { NODE_ENV: JSON.stringify('production') } }. Using the latter will overwrite the process object which can break compatibility with some modules that expect other values on the process object to be defined.

@daltojohnso so you think we can just remove this all together?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oooor, let's simply use what they have on their webpage as an example?

'process.env.NODE_ENV': JSON.stringify(process.env.NODE_ENV),

agreed @ariya @daltojohnso ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interestingly, I still get the warning when using 'process.env.NODE_ENV'. trying to figure out what's causing the conflict

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sanity check for what what @daltojohnso said (that we don't have to add it manually). Created a file foo.js - has nothing to do with the webpack config.

// foo.js

process.env.NODE_ENV = "development";

const bar = JSON.stringify(process.env.NODE_ENV);

console.log(process.env.NODE_ENV); // development
console.log(process.env.NODE_ENV === "development"); // true
console.log(bar); // "development"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interestingly, I still get the warning when using 'process.env.NODE_ENV'. trying to figure out what's causing the conflict

Me too. Removing the line completely removed the error. This experiment with the foo.js shows that we should be good even without stringifying NODE_ENV value for the sake of strict equality checks.

@@ -179,31 +172,20 @@ const config = (module.exports = {
outputPath: __dirname + "/resources/frontend_client/app/dist",
}),
new webpack.DefinePlugin({
"process.env": { NODE_ENV: JSON.stringify(NODE_ENV) },
process: { env: { NODE_ENV: JSON.stringify(NODE_ENV) } },
INCLUDE_EE_PLUGINS: JSON.stringify(process.env.MB_EDITION === "ee"),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just searched through the entire code base and we don't use this anywhere. Probably safe to remove the whole line.

Suggested change
INCLUDE_EE_PLUGINS: JSON.stringify(process.env.MB_EDITION === "ee"),

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants