fet(build): Create debug versions of minified bundles #4699
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
size-limit report
|
|
Woops 🙈 |
1 similar comment
|
Woops 🙈 |
lobsterkatie
force-pushed
the
kmclb-create-debug-bundles
branch
from
ed654f3 to
57ec132
Compare
Lms24
approved these changes
Mar 10, 2022
There was a problem hiding this comment.
This looks good to me overall. Code-wise I think it makes sense. I left some comments on naming but given my (probably limited) understanding of our naming conventions, take them with a grain of salt.
EDIT: Whoops, the comments did not make it, re-added them below
Lms24
reviewed
Mar 10, 2022
Lms24
reviewed
Mar 10, 2022
…iants, s/bundleVariants/variantSpecificOptionsVariants
lobsterkatie
force-pushed
the
kmclb-create-debug-bundles
branch
from
57ec132 to
2eb6467
Compare
lobsterkatie
added a commit
that referenced
this pull request
Mar 15, 2022
Given that now most of our testing runs in parallel in CI, the biggest bottleneck has become the build step. Within that step, the single slowest thing we do is build integration bundles, simply because of the number of bundles which need to be created. (This is especially true now that we're creating three versions of each bundle rather than two[1].) To speed things up a bit, this parallelizes the building of those bundles. Though it ends up not being as dramatic a time savings as one might hope (because the typescript plugin's caching mechanism doesn't play nicely with concurrent builds[2]), it nonetheless drops the total build time from roughly two minutes down to 70-80 seconds in my local testing. [1] #4699 [2] ezolenko/rollup-plugin-typescript2#15
lobsterkatie
added a commit
to getsentry/sentry-docs
that referenced
this pull request
Mar 22, 2022
A number of PRs (see below) have recently added to the bundles available for use on the CDN. This documents the new options, and does some general wordsmithing on the page. Bundle-adding PRs: getsentry/sentry-javascript#4674 getsentry/sentry-javascript#4699 getsentry/sentry-javascript#4718
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds to our base CDN rollup config the ability to create bundles which are minified but nonetheless contain debug logging.
Notable changes:
Setting the constant (
__SENTRY_DEBUG__) which controls whether or not to include logging is now done by a separate rollup plugin rather than by terser (which was setting the now-obsolete__SENTRY_NO_DEBUG__flag). This allows rollup to do the treeshaking, which is good because it lets us create a non-minified no-debug bundle. Though such a bundle doesn't make a lot of sense to publish (why strip the logging if you’re not also going to minify the code?), it is very helpful for us, because it allows us to see what's being treeshaken in the context of human-readable code.The scope of the helper function
makeMinificationVariantshas broadened to include the creation of all bundle variants*. As a result, it no longer takes an array of configs (which it might have gotten had there ever been amakeJSVersionVariantsormakeDebugVariantswhose results could have been passed tomakeMinificationVariants) but instead takes a single base config which it then spins out into all necessary flavors. This broadening of scope is also now reflected in variable names and the function name itself.* This is not quite true, in that it's still not handling the ES5/ES6 question. I chose not to incorporate that because as soon as we go ES6-only in v7, it will no longer be an issue for any package except perhaps the browser package.
Ref: https://getsentry.atlassian.net/browse/WEB-546