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

ref(nextjs): Exclude cross-platform tracing code from bundles #3978

Merged
merged 5 commits into from
Sep 14, 2021

Conversation

lobsterkatie
Copy link
Member

The @sentry/tracing package contains shared code, code which only applies to browser, and code which only applies to node. At the moment, webpack is unable to treeshake the former out of the server build and the latter out of the client build. This PR does that treeshaking manually, taking advantage of webpack 5’s new ability to replace a module with an empty object by setting resolve.alias.<someModule> = false in the webpack config.

Notes:

  • All node tracing integrations have been moved into a node folder, so we don't need to worry that at some point a non-node integration might get added to integrations and be accidentally excluded.

  • Normally we only care about bundle size on the browser side, but since Vercel turns server routes into serverless functions, bundle size matters on the backend, too, so the change was made in both directions.

  • As a point of reference, on my test app it took the sentry portion of the client _app bundle down from ~32.3 kb to ~30.6 kb, which is a little over a 5% savings.

Before:

image

After:

image

@github-actions
Copy link
Contributor

github-actions bot commented Sep 14, 2021

size-limit report

Path Size
@sentry/browser - CDN Bundle (gzipped) 21.66 KB (0%)
@sentry/browser - Webpack 22.67 KB (0%)
@sentry/react - Webpack 22.71 KB (0%)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 29.11 KB (0%)

Copy link
Contributor

@iker-barriocanal iker-barriocanal left a comment

Choose a reason for hiding this comment

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

🚀
I'd also rename the tests. If the integration is in src/integrations/node/mongo.ts, the test should be in test/integrations/node/mongo.test.ts (instead of test/integrations/mongo.test.ts).

@lobsterkatie
Copy link
Member Author

🚀
I'd also rename the tests. If the integration is in src/integrations/node/mongo.ts, the test should be in test/integrations/node/mongo.test.ts (instead of test/integrations/mongo.test.ts).

Oh, good call. Done.

@lobsterkatie lobsterkatie force-pushed the kmclb-nextjs-exclude-cross-platform-tracing-code branch from 2e6b2ba to 8652e86 Compare September 14, 2021 16:59
@lobsterkatie lobsterkatie merged commit c8a941c into master Sep 14, 2021
@lobsterkatie lobsterkatie deleted the kmclb-nextjs-exclude-cross-platform-tracing-code branch September 14, 2021 18:42
lobsterkatie added a commit that referenced this pull request Dec 14, 2021
In #3978, code was introduced to prevent node tracing integrations (mongo, postgres, etc) from appearing in the browser bundle where they don't belong. However, as part of our larger push to make our code more treeshakable, we recently changed how we export those integrations[1], making the previous workaround unnecessary.

As a bonus, this should fix a rendering issue some users were having when using the `fallback` flag.

Tested locally and on vercel.

Fixes #4090.

[1] #4204
onurtemizkan pushed a commit that referenced this pull request Dec 19, 2021
In #3978, code was introduced to prevent node tracing integrations (mongo, postgres, etc) from appearing in the browser bundle where they don't belong. However, as part of our larger push to make our code more treeshakable, we recently changed how we export those integrations[1], making the previous workaround unnecessary.

As a bonus, this should fix a rendering issue some users were having when using the `fallback` flag.

Tested locally and on vercel.

Fixes #4090.

[1] #4204
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.

3 participants