getsentry / sentry-javascript-bundler-plugins

JavaScript Bundler Plugins for Sentry
https://sentry.io
BSD 3-Clause "New" or "Revised" License
131 stars 34 forks source link

Sentry Webpack plugin breaks webpack-bundle-analyzer #335

Open jameshoward opened 1 year ago

jameshoward commented 1 year ago

Environment

webpack@5.88.0 webpack-bundle-analyzer@4.9.0 @sentry/webpack-plugin@2.4.0

Steps to Reproduce

We are having issues using the Webpack plugin in an existing project, where it's changing our bundle analysis tooling.

Minimal reproduction: https://github.com/jameshoward/webpack-sentry-demo

Check out the project then yarn install and yarn run analyze.

Changing the value of disabled for the Sentry plugin in webpack.config.js changes the output from webpack-bundle-analyzer. Specifically, with it enabled the import from node_modules is not split out from src/.

After lots of digging it seems that the problem is the way in which webpack-bundle-analyzer looks for webpack modules contained within an IIFE at the top-level. With the addition of the Sentry code, the files change to look like:

!function(){ /* Sentry code */ }(), (()=>{ /* Webpack code */ })();

Because this is now a single expression, it doesn't match the lookup for a webpack module. If I manually replace the , in the middle with a semi-colon things start working again. Similarly, if I turn off minifcation it also works but that's not a viable solution. I couldn't find a way to make the Sentry code not affect the analyzer.

Arguably this is a problem that webpack-bundle-analyzer may need to fix or we may need to find another workaround.

Expected Result

Enabling the Sentry plugin would not affect the bundle analysis.

Screenshot 2023-06-28 at 15 50 24

Actual Result

Enabling the Sentry plugin causes the bundle analyzer to not correctly split out modules.

Screenshot 2023-06-28 at 15 50 07
lforst commented 1 year ago

Thanks for reporting this and providing repro!

Sadly, I don't know a solution to this yet as of now. We tried a million different things to inject necessary code into bundles and the current solution seems like the most resilient one yet - so I am a bit hesitant to change it again.

I also wonder if this is something webpack-bundle-analyzer would need to fix. I am not aware of any contractual boundaries that we're breaking with the plugin.

jameshoward commented 1 year ago

Thanks for the quick reply. I think for now our workaround is just to disable the plugin when we're running the bundle analysis script. The disable option of the plugin is very useful for that.

I'll raise it with webpack-bundle-analyzer in the meantime.

lforst commented 1 year ago

I'll raise it with webpack-bundle-analyzer in the meantime.

Great, thank you!

valscion commented 1 year ago

Hi! This is @valscion from webpack-bundle-analyzer.

The situation right now is that webpack-bundle-analyzer does not support the BannerPlugin. We have an open feature request here: https://github.com/webpack-contrib/webpack-bundle-analyzer/discussions/601

So there currently is no way to make both this Sentry webpack plugin and webpack-bundle-analyzer to play along as the Sentry plugin uses BannerPlugin internally:

https://github.com/getsentry/sentry-javascript-bundler-plugins/blob/36bf09880f983d562d9179cbbeac40f7083be0ff/packages/webpack-plugin/src/index.ts#L51-L60

jameshoward commented 1 year ago

Since the two don't really need to operate together, I'll stick with my workaround of disabling the Sentry plugin when running the analysis script.

"scripts": {
  "preanalyze": "DISABLE_SENTRY_PLUGIN=true webpack build --mode=production --json=stats.json",
  "analyze": "webpack-bundle-analyzer stats.json build/",
}
// webpack.config.js
…
  plugins: [
    sentryWebpackPlugin({
      org: '***',
      project: '***',
      authToken: process.env.SENTRY_AUTH_TOKEN,
      disable: process.env.DISABLE_SENTRY_PLUGIN,
    }),
    …
  ]

Thanks for responding so quickly. :)

lforst commented 1 year ago

Hi @valscion, thanks for the insight! We'll leave this issue open in case other people run into this.

For now, I still don't have a solution. We need to inject (different) code into each bundle as early as possible. The BannerPlugin does a lot of important stuff for us, e.g. preserving source maps. A BannerPlugin independent solution for us would probably still run into the same compat issues with webpack-bundle-analyzer.

valscion commented 1 year ago

A BannerPlugin independent solution for us would probably still run into the same compat issues with webpack-bundle-analyzer.

Yeah I'd expect that to be the case.

I don't unfortunately have any idea yet how we could support BannerPlugin even though I do think it would be valuable.

You are doing the correct thing with what you're doing here :+1:.