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

Update to Rspack #7459

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

pjonsson
Copy link
Contributor

What this PR does

Depends on: #7433 and #7439

This is not ESBuild (#6624), but this brings release build times down to 20s instead of the current 116s, and also improves the build times for development builds (it was the tsc runtime that was the limiting factor for development builds in late December, incremental mode in #7424 improved that). This PR does get rid of Babel though (#6068).

The http://rspack.dev web page answers
"Why Rspack?" as:

Rspack is a high performance JavaScript bundler written in Rust. It offers strong compatibility with the webpack ecosystem, allowing for seamless replacement of webpack, and provides lightning fast build speeds.

Timing comparison on current
main branch with an empty
wwwroot/build:

Webpack 5, time yarn gulp release:

Assets:
  TerriaJS-specs.js (14.7 MiB)
  d068998a8a3b0b0db0d2.DAC (1.98 MiB)
  873.TerriaJS-specs.js (259 KiB)
  253.TerriaJS-specs.js (740 KiB)
  assimpjs.wasm (3.98 MiB)

real 116.81
user 219.80
sys 19.37

Rspack, time yarn gulp release:

│ Assets:
│   TerriaJS-specs.js (11.068 MiB)
│   f1a214c2392fcf40.DAC (3.842 MiB)
│   790.TerriaJS-specs.js (260.316 KiB)
│   911.TerriaJS-specs.js (730.658 KiB)
│   assimpjs.wasm (3.982 MiB)

Done in 20.83s.
real 20.98
user 62.55
sys 8.47

The sizes of TerriaJS-specs.js for
different cut-off times:

2019-01-01: 11.154 MiB
2020-01-01: 11.108 MiB
2021-01-01: 11.042 MiB
2022-01-01: 10.933 MiB

Status

  • Removing the cesium debug statements also doesn't work, but without that all tests pass with this on top of geoJsonGeometryFrom: use number for coordinates #7433 and BoxDrawing: stop triggering Cesium debug asserts #7439 after removing the @ts-check in configureRspack.js.
  • My late December tests were with Rspack 1.1.x, and this PR is 1.2.3 which gives a new warning about disabled--hover is not recognized as a valid pseudo-class. in the generated css file.
  • I don't know how watch-mode is supposed to work so I haven't touched that, that needs to be fixed before this can be merged.

Test me

Same way as the webpack 5 PR was tested.

Checklist

  • There are unit tests to verify my changes are correct or unit tests aren't applicable (if so, write quick reason why unit tests don't exist)
  • I've updated relevant documentation in doc/.
  • I've updated CHANGES.md with what I changed.
  • I've provided instructions in the PR description on how to test this PR.

The http://rspack.dev web page answers
"Why Rspack?" as:

Rspack is a high performance JavaScript bundler written in Rust. It
offers strong compatibility with the webpack ecosystem, allowing for
seamless replacement of webpack, and provides lightning fast build
speeds.

Timing comparison on current
main branch with an empty
wwwroot/build:

Webpack 5, time yarn gulp release:

Assets:
  TerriaJS-specs.js (14.7 MiB)
  d068998a8a3b0b0db0d2.DAC (1.98 MiB)
  873.TerriaJS-specs.js (259 KiB)
  253.TerriaJS-specs.js (740 KiB)
  assimpjs.wasm (3.98 MiB)

real 116.81
user 219.80
sys 19.37

Rspack, time yarn gulp release:

│ Assets:
│   TerriaJS-specs.js (11.068 MiB)
│   f1a214c2392fcf40.DAC (3.842 MiB)
│   790.TerriaJS-specs.js (260.316 KiB)
│   911.TerriaJS-specs.js (730.658 KiB)
│   assimpjs.wasm (3.982 MiB)

Done in 20.83s.
real 20.98
user 62.55
sys 8.47

The sizes of TerriaJS-specs.js for
different cut-off times:

2019-01-01: 11.154 MiB
2020-01-01: 11.108 MiB
2021-01-01: 11.042 MiB
2022-01-01: 10.933 MiB
env: {
coreJs: "3",
mode: "usage",
targets: ["since 2020-01-01 and not dead"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know how old browsers that are currently supported, so this line needs to be adjusted.

@@ -62,11 +66,25 @@ module.exports = function (devMode) {
]
}
};
if (devMode) {
// Enable cache with devMode.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The cache is new in Rspack 1.2, I don't know if enabling it gives a performance improvement.

@@ -69,8 +69,8 @@ async function download(items: TableMixin.Instance[]) {
const synthesized = synthesizeNameAndValueArrays(items);

// Could implement this using TaskProcessor, but requires webpack magic.
const worker = await import("worker-loader!./downloadHrefWorker").then(
(labelLayoutWorker) => new labelLayoutWorker.default()
const worker = new Worker(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

My interpretation is that this was the intention when migrating to webpack 5, but something didn't work, so the dependency on worker-loader was kept and will be removed in a separate PR.

"webpack-cli": "^5.1.4",
"webpack-dev-server": "^5.1.0",
"worker-loader": "^3.0.8"
"webpack": "^5.96.1"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it was svg-sprite-loader that tried to do webpack 2 (3?) version detection, and crashed if webpack couldn't be found at all.

@pjonsson
Copy link
Contributor Author

Is this PR of interest so I should try to fix the remaining parts?

@na9da
Copy link
Collaborator

na9da commented Feb 16, 2025

We haven't discussed about switching to a different build tool in the short term, but thanks for testing this out @pjonsson.

rspack looks interesting because it is a drop in replacement for webpack and so there's hopefully minimal changes. Personally, my concerns would be the popularity of the tool in long run.

Our previous attempt to use esbuild was largely propelled by Cesium's switch - it was already in our ecosystem. I think with the upgrade to webpack5 and removal of webpack specific features from the code, we should be closer to making esbuild work if we choose to.

@pjonsson
Copy link
Contributor Author

rspack looks interesting because it is a drop in replacement for webpack and so there's hopefully minimal changes. Personally, my concerns would be the popularity of the tool in long run.

Our previous attempt to use esbuild was largely propelled by Cesium's switch - it was already in our ecosystem. I think with the upgrade to webpack5 and removal of webpack specific features from the code, we should be closer to making esbuild work if we choose to.

@na9da this PR runs the tests successfully, so I think it's already pretty close to being comparable to what Webpack provides. A reasonably large part of the diff is because I renamed things from Webpack to Rspack which is strictly speaking not required, so I think it's fair to call Rspack a drop in replacement. For someone who knows Webpack, I'm pretty sure this PR in its current state would have taken an afternoon to accomplish.

ESBuild looks even faster than Rspack though, so if there is a reasonably easy upgrade from here to ESBuild I would absolutely love that. The main reason I didn't go that route is because I don't know any of these tools, so a drop-in replacement seemed like the best thing I could accomplish on my own without spending weeks of effort. Is there a draft ESBuild PR that isn't 2+ years old somewhere?

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.

2 participants