-
Notifications
You must be signed in to change notification settings - Fork 51
feat: Combine injection plugins #844
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
feat: Combine injection plugins #844
Conversation
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛Plugin Manager
Other
Documentation 📚
Build / dependencies / internal 🔧
Other
🤖 This preview updates automatically when you update the PR. |
Lms24
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice improvement, thanks Tim!
andreiborza
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
| datasource | package | from | to | | ---------- | ------------------- | ----- | ----- | | npm | @sentry/vite-plugin | 4.6.1 | 4.7.0 | ## [v4.7.0](https://github.com/getsentry/sentry-javascript-bundler-plugins/blob/HEAD/CHANGELOG.md#470) - docs: Add RELEASE.md to document release process ([#834](getsentry/sentry-javascript-bundler-plugins#834)) - feat: Combine injection plugins ([#844](getsentry/sentry-javascript-bundler-plugins#844)) - fix(plugin-manager): Enable "rejectOnError" in debug ([#837](getsentry/sentry-javascript-bundler-plugins#837)) - fix(plugin-manager): Respect `sourcemap.ignore` values for injecting debugIDs ([#836](getsentry/sentry-javascript-bundler-plugins#836)) - fix(vite): Skip HTML injection for MPA but keep it for SPA ([#843](getsentry/sentry-javascript-bundler-plugins#843)) <details> <summary> <strong>Internal Changes</strong> </summary> - chore: Use pull\_request\_target for changelog preview ([#842](getsentry/sentry-javascript-bundler-plugins#842)) - ci(release): Switch from action-prepare-release to Craft ([#831](getsentry/sentry-javascript-bundler-plugins#831)) - test: Ensure Debug IDs match ([#840](getsentry/sentry-javascript-bundler-plugins#840)) </details> ## [v4.6.2](https://github.com/getsentry/sentry-javascript-bundler-plugins/blob/HEAD/CHANGELOG.md#462) - fix(vite): Ensure sentryVitePlugin always returns an array of plugins ([#832](getsentry/sentry-javascript-bundler-plugins#832)) - fix(vite): Skip code injection for HTML facade chunks ([#830](getsentry/sentry-javascript-bundler-plugins#830)) - fix(rollup): Prevent double-injection of debug ID ([#827](getsentry/sentry-javascript-bundler-plugins#827)) - fix(esbuild): fix debug ID injection when moduleMetadata or applicationKey is set ([#828](getsentry/sentry-javascript-bundler-plugins#828))
| datasource | package | from | to | | ---------- | ------------------- | ----- | ----- | | npm | @sentry/vite-plugin | 4.6.1 | 4.7.0 | ## [v4.7.0](https://github.com/getsentry/sentry-javascript-bundler-plugins/blob/HEAD/CHANGELOG.md#470) - docs: Add RELEASE.md to document release process ([#834](getsentry/sentry-javascript-bundler-plugins#834)) - feat: Combine injection plugins ([#844](getsentry/sentry-javascript-bundler-plugins#844)) - fix(plugin-manager): Enable "rejectOnError" in debug ([#837](getsentry/sentry-javascript-bundler-plugins#837)) - fix(plugin-manager): Respect `sourcemap.ignore` values for injecting debugIDs ([#836](getsentry/sentry-javascript-bundler-plugins#836)) - fix(vite): Skip HTML injection for MPA but keep it for SPA ([#843](getsentry/sentry-javascript-bundler-plugins#843)) <details> <summary> <strong>Internal Changes</strong> </summary> - chore: Use pull\_request\_target for changelog preview ([#842](getsentry/sentry-javascript-bundler-plugins#842)) - ci(release): Switch from action-prepare-release to Craft ([#831](getsentry/sentry-javascript-bundler-plugins#831)) - test: Ensure Debug IDs match ([#840](getsentry/sentry-javascript-bundler-plugins#840)) </details> ## [v4.6.2](https://github.com/getsentry/sentry-javascript-bundler-plugins/blob/HEAD/CHANGELOG.md#462) - fix(vite): Ensure sentryVitePlugin always returns an array of plugins ([#832](getsentry/sentry-javascript-bundler-plugins#832)) - fix(vite): Skip code injection for HTML facade chunks ([#830](getsentry/sentry-javascript-bundler-plugins#830)) - fix(rollup): Prevent double-injection of debug ID ([#827](getsentry/sentry-javascript-bundler-plugins#827)) - fix(esbuild): fix debug ID injection when moduleMetadata or applicationKey is set ([#828](getsentry/sentry-javascript-bundler-plugins#828))
While benchmarking large projects with Rolldown, I found that
MagicString.generateMap()has a large overhead. There will be ways to improve this in the future, however this issue is exacerbated by the fact that we inject up to 3 separate times via 3 separate plugins. Each of these 3 injects into the top of the source and then has to rewrite the sourcemap!This PR combines the 3 injection plugins into a single plugin which means the chunks only need the sourcemaps generation once.
This PR retains esbuild with 3 separate plugins because I haven't worked out how to migrate that yet.
With an example app of 10k modules and 10MB output file: