getsentry / sentry-javascript-bundler-plugins

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

feat: Allow to configure `bundleSizeOptimizations` #428

Closed mydea closed 1 year ago

mydea commented 1 year ago

This introduces a new option for the plugins:

sentryPlugin({
   bundleSizeOptimizations: {
      excludeDebugStatements: true,
      excludePerformanceMonitoring: true,
      excludeReplayCanvas: true,
      excludeReplayIframe: true,
      excludeReplayShadowDom: true,
    },
});

When set, this will replace the relevant build time flags with a boolean, allow bundlers to treeshake code out and reduce the shipped bundle size.

Closes https://github.com/getsentry/sentry-javascript-bundler-plugins/issues/425

lforst commented 1 year ago

Do you think there's a way that we could filter out files before reading them? I'm thinking about looking at the file path and checking for a @sentry occurance. But not sure if I'm missing something tough why this wouldn't work.

Not a bad idea. we could do id.contains(os.sep + "node_modules" + os.sep + "@sentry" + os.sep). This has a few edge cases with resolution but in 99% of the cases it should work.

Counterargument, it would not be the same behavior for all bundlers and why should we make this unnecessarily fragile?

Lms24 commented 1 year ago

Counterargument, it would not be the same behavior for all bundlers and why should we make this unnecessarily fragile?

Also a good point. My concern is mostly that the plugin having to read every file might have a performance impact. But I'm totally fine if we just go with this first and optimize later if necessary.

mydea commented 1 year ago

Do you think there's a way that we could filter out files before reading them? I'm thinking about looking at the file path and checking for a @sentry occurance. But not sure if I'm missing something tough why this wouldn't work.

Yeah we can for sure revisit this, since this is opt in I think we can tweak this more later if needed!

mydea commented 1 year ago

OK so I fixed the tests and tried to address the feedback - is this ready to go? :)

I did not add any file filtering for now, I think we can still add this at a later stage if we find out this is too slow! This does not run at all if no bundleSizeOptimizations are configured so it should not affect any existing users.

lforst commented 1 year ago

This concern is critical and we need to address it before merging this.