fix(build): Prevent unused utils code in integration bundles #4547
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.
Even without a plugin like
terser
, Rollup by default will treeshake code it sees isn't being used. That said, by default it will err on the side of caution, to make sure it doesn't break anyone's code.One of the conservative choices it makes out of the box is to assume that any module with side effects ought to be retained, even if none of the code it contains is used. Some of the things it considers side effects make sense (
console.log()
, for example), but others are much less obvious (and, frankly, a little hard to understand), among them accessingprocess
and usingSomeClass.prototype.someMethod.call(someInstance, ...args)
rather thansomeInstance.someMethod(...args)
. (Disclaimer: These were discovered through repeated... and repeated... and repeated trial and error. The truth lies somewhere in rollup'sast
code, but in my testing they consistently were the two things which prevented treeshaking.) As it happens, we have the proverbial perfect storm in ourutils
package, in the form ofisNodeEnv()
, which has not one but both of those things.In any case, that's a lot of preamble to say that in
@sentry/utils
, the index file imports fromtime.ts
,time.ts
includes an IIFE which callsgetGlobalObject()
, andgetGlobalObject()
in turn callsisNodeEnv()
. Thus, importing anything at all from@sentry/utils
has meant that the entirety oftime.ts
,global.js
, andnode.js
had to come along for the ride in any bundle we create, regardless of their use (or lack thereof). In particular, even though none of our integrations use any of the code intime.ts
, it was being included all but two integration bundles, because they all use something else from@sentry/utils
.Fortunately, rollup provides an option,
treeshake.moduleSideEffects : false
, to only consider actual code use when deciding which modules to include in a bundle, regardless of whatever side effects that module may or may not generate. This PR uses the more generaltresshake: "smallest"
(which includes settingmoduleSideEffects
tofalse
) in order to eliminate the incorrect inclusion oftime.ts
in the integration bundles.Before:
After:
UPDATE: This turns out to explain the
process
access being considered a side effect. It's also overridden bytreeshake: "smallest"
.