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

build(ui): upgrade to Webpack v5 + upgrade loaders + plugins #11628

Merged
merged 3 commits into from
Aug 23, 2023

Conversation

agilgur5
Copy link

@agilgur5 agilgur5 commented Aug 20, 2023

Motivation

  • Upgrade the UI build to use latest versions of many dev deps
    • Webpack v4 hasn't been updated in 2.5 years.
    • Webpack v5 stable was released 3 years ago, and RC / beta / alpha are even older
    • This unblocks other dep upgrades as well

Modifications

Webpack Core

Plugins

  • upgrade HtmlWebpackPlugin and CopyWebpackPlugin to latest major:
    • html-webpack-plugin to v5's main breaking changes are to drop support for Webpack v4 and Node <= 10
    • copy-webpack-plugin to v11 has some options changes as well as Webpack and Node support drops:
      • v6 changes to use patterns: [...]
      • v7 drops support for Webpack v4
      • v8 does not impact our usage
      • v9-v11 drop support for older Node, now requiring Node >= 14.15.0
  • no changes needed to DefinePlugin
  • MonacoEditorWebpackPlugin is being upgraded separately in chore(deps): upgrade monaco-editor to 0.30 #11593

Loaders

  • upgrade style-loader to v1 and sass-loader to latest major:
    • style-loader's main breaking changes are dropping support for Webpack v4 and old Node versions:
      • v1 drops support for Webpack < v4, Node < 8.9.0, and otherwise does not impact our usage
      • v2 drops support for Node < 10.13.0 and otherwise does not impact our usage
      • v3 drops support for Webpack v4, Node < 12.13.0, and otherwise does not impact our usage
    • sass-loader's main breaking changes are dropping support for Webpack v4 and old Node versions:
      • v11 drops support for Webpack v4
      • v12-v13 drop support for older Node, now requiring Node >= 14.15.0
  • upgrade raw-loader to latest major
    • raw-loader's main breaking changes are dropping support for Webpack <v4 and old Node versions:
      • v1 drops support for Webpack < v4 and Node < 6.9
      • v2 does not seem to impact our usage
      • v3 drops support for Node < 8.9.0
      • v4 drops support for Node < 10.13.0
    • I was not able to swap raw-loader for built-in Webpack v5 asset loading as it is an in-between loader
      • it would have to be replaced by css-loader, effectively, but doing that causes Webpack to fail to find assets from argo-ui etc
        • using resolve-url-loader or other trial-and-error attempts did not fix that either; not really sure why it's failing
  • did not touch raw-loader or file-loader as both are deprecated in Webpack v5
    • can replace these with built-in alternatives in a future PR
    • no deprecation warnings from these right now
    • EDIT: removed file-loader and replaced with built-in asset loading (asset/resource)
    • EDIT: see above for raw-loader
  • did not touch source-map-loader, react-hot-loader, babel-loader, or ts-loader as I'd like to replace these with esbuild-loader
    • as it would be much faster, still serves our minimal config needs, and matches Argo CD config
    • react-hot-loader is also deprecated
    • no deprecation warnings from these right now

Misc

Verification

Build passes. No new install warnings.
Spot check on render.

Notes for Reviewers

Future Work

As mentioned in the above Loaders section, to replace several loaders with built-in Webpack v5 assets and esbuild-loader. EDIT: asset changes is now complete, or to the extent it could be (see above re: raw-loader).

- upgrade `webpack` and `webpack-cli` to latest major v5
  - (`webpack-dev-server` does not have a v5, is already at latest major)
  - fix config errors and deprecation warnings:
    - `[hash]` -> `[contenthash]` (https://webpack.js.org/migrate/5/#clean-up-configuration)
      - `contenthash` is just a better hash, as it is calculated per entrypoint instead for the entire bundle
        - that being said, we only have one entrypoint, so not super relevant. but `hash` was also deprecated
      - also same as [Argo CD config](https://github.com/argoproj/argo-cd/blob/6ca2e90b5094eba3878746fee5f2c357bfbc6cb8/ui/src/app/webpack.config.js#L20)
    - `node.fs: "empty"` -> `resolve.fallback.fs: false` (https://webpack.js.org/migrate/5/#clean-up-configuration)
      - also same as [Argo CD config](https://github.com/argoproj/argo-cd/blob/6ca2e90b5094eba3878746fee5f2c357bfbc6cb8/ui/src/app/webpack.config.js#L28)
    - `loaders` -> `use` (https://webpack.js.org/migrate/5/#update-outdated-options)
      - `!` syntax for chaining loaders has been fully removed as well. replaced by using arrays

- upgrade `HtmlWebpackPlugin` and `CopyWebpackPlugin` to latest major:
  - `html-webpack-plugin` to v5's main [breaking changes](https://github.com/jantimon/html-webpack-plugin/blob/v5.5.3/CHANGELOG.md#500-2021-02-03) are to drop support for Webpack v4 and Node <= 10
  - `copy-webpack-plugin` to v11 has some options changes as well as Webpack and Node support drops:
    - [v6](https://github.com/webpack-contrib/copy-webpack-plugin/blob/v11.0.0/CHANGELOG.md#600-2020-05-15) changes to use `patterns: [...]`
      - same syntax as [Argo CD config](https://github.com/argoproj/argo-cd/blob/6ca2e90b5094eba3878746fee5f2c357bfbc6cb8/ui/src/app/webpack.config.js#L72)
    - [v7](https://github.com/webpack-contrib/copy-webpack-plugin/blob/v11.0.0/CHANGELOG.md#700-2020-12-10) drops support for Webpack v4
    - [v8](https://github.com/webpack-contrib/copy-webpack-plugin/blob/v11.0.0/CHANGELOG.md#800-2021-03-04) does not impact our usage
    - v9-[v11](https://github.com/webpack-contrib/copy-webpack-plugin/blob/v11.0.0/CHANGELOG.md#1100-2022-05-17) drop support for older Node, now requiring Node >= 14.15.0
- no changes needed to `DefinePlugin`
- `MonacoEditorWebpackPlugin` is being upgraded in a separate PR/commit

- upgrade `style-loader` and `sass-loader` to latest major:
  - `style-loader`'s main breaking changes are dropping support for Webpack v4 and old Node versions:
    - [v1](https://github.com/webpack-contrib/style-loader/blob/v3.3.3/CHANGELOG.md#100-2019-08-06) drops support for Webpack < v4, Node < 8.9.0, and otherwise does not impact our usage
    - [v2](https://github.com/webpack-contrib/style-loader/blob/v3.3.3/CHANGELOG.md#200-2020-10-09) drops support for Node < 10.13.0 and otherwise does not impact our usage
    - [v3](https://github.com/webpack-contrib/style-loader/blob/v3.3.3/CHANGELOG.md#300-2021-06-24) drops support for Webpack v4, Node < 12.13.0, and otherwise does not impact our usage
  - `sass-loader`'s main breaking changes are dropping support for Webpack v4 and old Node versions:
    - [v11](https://github.com/webpack-contrib/sass-loader/blob/v13.3.2/CHANGELOG.md#1100-2021-02-05) drops support for Webpack v4
    - v12-[v13](https://github.com/webpack-contrib/sass-loader/blob/v13.3.2/CHANGELOG.md#1300-2022-05-18) drop support for older Node, now requiring Node >= 14.15.0
- did not touch `raw-loader` or `file-loader` as both are [deprecated in Webpack v5](https://webpack.js.org/guides/asset-modules/)
  - can replace these with native alternatives in a future PR
  - no deprecation warnings from these right now
- did not touch `source-map-loader`, `react-hot-loader`, `babel-loader`, or `ts-loader` as I'd like to replace these with `esbuild-loader`
  - as it would be much faster, still serves our minimal config needs, and matches [Argo CD config](https://github.com/argoproj/argo-cd/blob/6ca2e90b5094eba3878746fee5f2c357bfbc6cb8/ui/src/app/webpack.config.js#L37)
  - `react-hot-loader` is also [deprecated](https://github.com/gaearon/react-hot-loader#moving-towards-next-step)
  - no deprecation warnings from these right now

- also change some single quotes to double quotes in the config for consistency (`'` -> `"`)

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
@agilgur5 agilgur5 added type/dependencies PRs and issues specific to updating dependencies area/ui javascript Pull requests that update Javascript dependencies labels Aug 20, 2023
- this is no longer necessary with Webpack v5

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
@isubasinghe
Copy link
Member

Neat! Thanks for this PR. Out of curiosity how long of a task do you think it would be to replace webpack with faster tooling (vite, turbopack, esbuild) etc ?

Copy link
Member

@isubasinghe isubasinghe left a comment

Choose a reason for hiding this comment

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

LGTM

@agilgur5
Copy link
Author

agilgur5 commented Aug 20, 2023

Out of curiosity how long of a task do you think it would be to replace webpack with faster tooling (vite, turbopack, esbuild) etc ?

Per the PR description, I am looking to replace babel-loader, ts-loader, etc with esbuild-loader, so going step by step here. That will be a follow-up PR.

Personally, I'm not a huge fan of Vite as it's a wrapper tool (which are not fun to maintain, as I can say first-hand as the longest solo maintainer of TSDX) and can give some headaches for TS users (that I have to respond to with some frequency in rollup-plugin-typescript2). I know Rollup quite well (as a result of being a maintainer of the above two libraries), so I personally don't have a need for a wrapper either.
Haven't tried turbopack as of yet. Last I checked it was pretty alpha and tightly integrated into Next.js (which Argo doesn't use)

@isubasinghe
Copy link
Member

@agilgur5 cool thanks, makes sense as to why you are going with esbuild in that case, I was just curious about the other options, mostly because I am fairly uninformed about that area now.

I haven't done much frontend work in the last 2 years, meaning I'm incredibly outdated with my knowledge here.

@agilgur5
Copy link
Author

agilgur5 commented Aug 20, 2023

I haven't done much frontend work in the last 2 years

My day job the past few years has been running MLOps, k8s SRE, and DevSecOps teams, so technically same 😅 Some of those teams did work on Node & front-end though. I still follow the React ecosystem, but not nearly as closely as I did in the past (I ran one of the first production React sites back in early 2014).

meaning I'm incredibly outdated with my knowledge here.

Build systems specifically though I do follow more closely, mainly by virtue of being a TS tooling maintainer. I also just look things up / refresh myself when relevant as well 🙂

@isubasinghe
Copy link
Member

@agilgur5 ah right that makes sense, I kinda ran away to academia after getting bored in industry (the Australian software eng market is boring, mostly pushing JSON around), I now work in the intersection of compiler dev/OS dev and formal verification. We are attempting to build a verified OS on top of the verified seL4 kernel, it isn't much money but a bit more rewarding to me because it is challenging.

Build systems specifically though I do follow more closely, mainly by virtue of being a TS tooling maintainer. I also just look things up / refresh myself when relevant as well 🙂

Ah yeah, if you are responsible for TS tooling, it is understandable that

- `raw-loader`'s main breaking changes are dropping support for Webpack <v4 and old Node versions:
  - [v1](https://github.com/webpack-contrib/raw-loader/blob/v4.0.2/CHANGELOG.md#100-2018-12-10) drops support for Webpack < v4 and Node < 6.9
  - [v2](https://github.com/webpack-contrib/raw-loader/blob/v4.0.2/CHANGELOG.md#200-2019-03-18) does not seem to impact our usage
  - [v3](https://github.com/webpack-contrib/raw-loader/blob/v4.0.2/CHANGELOG.md#300-2019-06-05) drops support for Node < 8.9.0
  - [v4](https://github.com/webpack-contrib/raw-loader/blob/v4.0.2/CHANGELOG.md#400-2019-11-25) drops support for Node < 10.13.0
- I was not able to swap `raw-loader` for built-in Webpack v5 asset loading as it is an in-between loader
  - it would have to be replaced by `css-loader`, effectively, but doing that causes Webpack to fail to find assets from `argo-ui` etc
    - using `resolve-url-loader` or other trial-and-error attempts did not fix that either; not really sure why it's failing
- swap `file-loader` for built-in Webpack v5 `asset/resouce` loading

- downgrade `style-loader` to v1
  - [v2](https://github.com/webpack-contrib/style-loader/blob/v3.3.3/CHANGELOG.md#200-2020-10-09)'s breaking change in webpack-contrib/style-loader@7a0ce4c appears to break our usage
    - I believe this is because the chained `raw-loader` returns a String and not an Array. Updating `raw-loader` did not fix this (and its latest version is older than `style-loader` v2 as well)
    - this didn't cause a build error, but it caused styles to fail to load on render instead

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
@agilgur5
Copy link
Author

agilgur5 commented Aug 21, 2023

@isubasinghe cool! I did some formal methods work back in my college days (and a bit of post-college lab work), was definitely a bit tedious (to say the least) to get everything verified 😅 But it is one reason why I like higher-level languages and their strong guarantees (e.g. OCaml; see also ReasonML in JS ecosystem). We can definitely chat more offline -- feel free to DM me on CNCF Slack or LinkedIn 🙂

@agilgur5
Copy link
Author

agilgur5 commented Aug 21, 2023

Pushed another commit to upgrade all asset loaders. Summarized in the commit message and added to the PR description.

Basically: updated raw-loader, removed and replaced file-loader with Webpack v5 built-in asset/resource (which I was originally going to save for a follow-up PR), and had to downgrade style-loader to v1 (which is still newer than it was before this PR) due to a breaking change that broke CSS rendering (but did not give a build error... so was a bit hard to track down).

UI CI build still passing. CI failure seems unrelated (in Go code), so probably a test flake? Will need a retry in CI

@terrytangyuan terrytangyuan enabled auto-merge (squash) August 23, 2023 01:09
@terrytangyuan terrytangyuan merged commit aac4f89 into argoproj:master Aug 23, 2023
@agilgur5 agilgur5 deleted the build-upgrade-webpack branch August 23, 2023 01:34
@agilgur5 agilgur5 added the area/build Build or GithubAction/CI issues label Aug 27, 2023
@agilgur5
Copy link
Author

agilgur5 commented Jan 14, 2024

  • did not touch source-map-loader, react-hot-loader, babel-loader, or ts-loader as I'd like to replace these with esbuild-loader
    • as it would be much faster, still serves our minimal config needs, and matches Argo CD config
    • react-hot-loader is also deprecated
    • no deprecation warnings from these right now

Removed react-hot-loader and replaced the rest with esbuild-loader in #12516

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build Build or GithubAction/CI issues area/ui javascript Pull requests that update Javascript dependencies type/dependencies PRs and issues specific to updating dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants