Skip to content

build: configure static globals to control bundle/debug code #4273

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
Dec 14, 2021

Conversation

mitsuhiko
Copy link
Contributor

@mitsuhiko mitsuhiko commented Dec 13, 2021

This adds experimental configuration to rollup and terser to make enable dead code elimination for browser bundles. In particular right now we want to make sure some code that is absolutely pointless in browser never makes it there without having to modify reusable code from the node version.

In addition this provides a flag we can then use to turn on/off functionality that's only relevant for debugging.

I was unable to make this work with a constant instead of a function call due to limitations in terser. However this overhead this adds to non terser builds should be small enough that I believe we should be merging this as such and then optimize later to get rid of this newfound overhead.

@AbhiPrasad AbhiPrasad added this to the Treeshaking / Bundle Size milestone Dec 13, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Dec 13, 2021

size-limit report

Path Base Size (626102f) Current Size Change
@sentry/browser - CDN Bundle (gzipped) 22.45 KB 21.67 KB -3.44% 🔽
@sentry/browser - Webpack 23.29 KB 23.35 KB +0.29% 🔺
@sentry/browser - Webpack - gzip = false 82.73 KB 82.8 KB +0.09% 🔺
@sentry/react - Webpack 23.32 KB 23.38 KB +0.29% 🔺
@sentry/nextjs Client - Webpack 48.07 KB 48.08 KB +0.03% 🔺
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 29.98 KB 29.99 KB +0.02% 🔺

@lobsterkatie
Copy link
Member

This Iooks good - I do think there are ways to optimize it in the future (particularly, it would be nice to find a way to do this which helps slim down not only CDN bundles but also the vendor bundles of folks using us through npm), but I think this is a great start, and should let us start doing the work of sorting code into browser-only, node-only, and shared.

I wonder if you might add an example to the description of how this would get used. Would we be putting conditionals around dynamic imports? Or is the idea just that any code that we call inside such a conditional would be dropped because terser would know it would never run?

@mitsuhiko
Copy link
Contributor Author

I'm quite convinced that longer term we want to use something like a global __DEV__ var instead of these functions but for that we need to figure out how to make type script happy. I noticed for instance that react-native has a __DEV__ global declaration, and so does apollo-client. But then using both results in an error (apollographql/apollo-client#9039).

@mitsuhiko mitsuhiko merged commit f042723 into master Dec 14, 2021
@mitsuhiko mitsuhiko deleted the feature/globals branch December 14, 2021 17:48
AbhiPrasad added a commit that referenced this pull request Jan 19, 2022
Temporarily stop removing debug functionality from the CDN bundle while
we work on a more fleshed out bundling solution.

We previously added static globals in
#4273 to leverage
dead code elimination to remove debug functionality from the browser
CDN. Currently though, we only publish a single CDN bundle, the one that
has the debug functionality stripped. Until we publish seperate CDN
bundles for debug and production, we will disable stripped debug
functionality from the production bundle.
AbhiPrasad added a commit that referenced this pull request Jan 19, 2022
Temporarily stop removing debug functionality from the CDN bundle while
we work on a more fleshed out bundling solution.

We previously added static globals in
#4273 to leverage
dead code elimination to remove debug functionality from the browser
CDN. Currently though, we only publish a single CDN bundle, the one that
has the debug functionality stripped. Until we publish seperate CDN
bundles for debug and production, we will disable stripped debug
functionality from the production bundle.
@benjamn
Copy link

benjamn commented Feb 2, 2022

apollographql/apollo-client#9386 should fix the original declaration conflict

@AbhiPrasad
Copy link
Member

Perhaps we should switch to using DEV then

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.

4 participants