Skip to content

[Blazor] Switch to rollup for bundling #53095

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

Merged
merged 4 commits into from
Jan 3, 2024
Merged

Conversation

javiercn
Copy link
Member

@javiercn javiercn commented Jan 2, 2024

Looking at modernizing our JS infrastructure I've compared a few bundlers. Namely:

  • Webpack: What we've been using, has fallen out of favor and is lacking important features like producing ES6 bundlers (it's technically possible with plugins)
  • Vite: The community champion, its stable and builds on top of Rollup and Esbuild, but it doesn't work well for our library setup (requires more work) and requires type checking typescript separately.
  • Parcel: Builds on top of rollup and mostly offers a simpler configuration, our setup is complex enough that we don't benefit from this.
  • Esbuild: This is becoming a standard used by other tools, but its too experimental for now, produces bigger bundles than rollup and doesn't support type checking (needs to be done separately)
  • Rspack: Same as esbuild, aims to be backwards compatible with webpack config, but is even more experimental than esbuilds and also lacks type checking.

Rollup is the most mature bundler that supports our use cases (current and future) and there is a clear path in the future as vite is working on its own rust based version of rollup 'rolldown'.

This is the initial work to replace the old version of webpack with an up to date bundler implementation that will allow us to generate not only commonjs bundles but also esm bundles. The PR also removes many old and stale dependencies that we weren't using. The bundle sizes are reduces between 0-2% depending on the bundle, which is likely negligible, but it's less than nothing.

Contributes to #52819

@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Jan 2, 2024
@javiercn javiercn marked this pull request as ready for review January 3, 2024 15:16
@javiercn javiercn requested a review from a team as a code owner January 3, 2024 15:16
@SteveSandersonMS
Copy link
Member

This sounds great to me! Switching over to rollup totally makes sense as we modernize and consider more modular output. I also investigated current JS bundler options in December for a different project and also concluded that Rollup was the most practical choice for that, so can relate to all the reasoning you gave when comparing the options.

My only question is whether we know for sure that the updated minification mechanism will preserve the same APIs as we had before. We do have some E2E coverage to exercise some of the public APIs, but do you know if (in theory) the newer minification options might be mangling more of the pubternal JS API names than the older one did?

@javiercn
Copy link
Member Author

javiercn commented Jan 3, 2024

This sounds great to me! Switching over to rollup totally makes sense as we modernize and consider more modular output. I also investigated current JS bundler options in December for a different project and also concluded that Rollup was the most practical choice for that, so can relate to all the reasoning you gave when comparing the options.

My only question is whether we know for sure that the updated minification mechanism will preserve the same APIs as we had before. We do have some E2E coverage to exercise some of the public APIs, but do you know if (in theory) the newer minification options might be mangling more of the pubternal JS API names than the older one did?

I don't think this will be mangling more stuff, but I'll take a look from the JS side of things. I also expect all our API to be covered by E2E test to a large degree. Same for the 'pubternal' bits that are invoked from dotnet

@javiercn
Copy link
Member Author

javiercn commented Jan 3, 2024

@SteveSandersonMS API looks fine

image

@SteveSandersonMS
Copy link
Member

OK great. If we discover anything did get moved (e.g., I see it's configured with keep_classnames: false) we can always tweak the config later. But maybe it's already exactly correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants