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
Add options and hooks to control module side effects #2844
Merged
Merged
Conversation
This file contains 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
…ution to determine purity
…plugins always override the user option.
lukastaegert
force-pushed
the
module-side-effects
branch
from
May 13, 2019 05:25
2625f5c
to
1ef16d8
Compare
This was referenced May 14, 2019
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 PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Resolves #2593
Description
This will add the necessary means to explicitly specify module side-effects to support features such as Webpack's package side-effects, cf. here: https://webpack.js.org/guides/tree-shaking/#mark-the-file-as-side-effect-free
Basic idea
A module now has an internal
moduleSideEffects
flag that can be eithertrue
orfalse
. The default istrue
and represents the previous behaviour where each statement in each module was checked for side-effects and was included in the bundle if Rollup determined there could possibly be side-effects.If the value is set to
false
via some of the means described below, then code of a module is only included in the bundle if at least one of its exports is (not only imported but actually) used by another module that is already included in the bundle. If that is not the case, then the statements of a module will not even be checked for side-effects but all code of the module will be omitted from the bundle.Entry modules are always included in the bundle.
Why would we even want this instead of further improving the side-effect detection (rest assured, we ARE still improving this one as well!) considering we already have
treeshake.pureExternalModules
to handle the case of external modules? Sometimes, it is impossible to determine something is side-effect-free no matter how much we improve the algorithm. This is usually the case when missing outside information is involved. Consider this example:Even if
pureExternalModules
was true, Rollup cannot remove this code becausepureFunction
is called as part of the initialization ofa.js
; and even thoughexternal
might be pure, we do not know from looking at the code ifpureFunction
is actually pure (or even a function).If
moduleSideEffects
are set tofalse
fora.js
, then the import ofa.js
would be removed fromb.js
without actually looking intoa.js
.Internally, this uses the same mechanism that is used for tree-shaking dynamic imports where the
import()
has been removed because it is in a dead branch.How can this feature be used?
Plugin API
The
resolveId
,load
andtransform
have all been extended so that a plugin can optionally mark a module as side-effect-free/side-effect-ful. The logic here is that if an object is returned for any of those hooks, one can optionally add amoduleSideEffects: boolean | null
flag. If the value is a boolean, this will override any value set for this flag via a previous hook or thetreeshake.moduleSideEffects
option (see below), otherwise the previous value will be retained.User-facing option:
treeshake.moduleSideEffects
There is also a new user-facing option to control this.
treeshake.moduleSideEffects
can be a boolean, an array of ids specifying the modules with side-effects (all other modules will be assumed to be side-effect-free), a function(id: string, external: boolean) => boolean
to determine it for each module individually, and the value"no-external"
that represents the behaviour of the soon-to-be-deprecatedtreeshake.pureExternalModules
.I decided to go for the more Webpack-like
moduleSideEffects
way of expressing it over the alternative approach to specify "pure" modules because I assume if users use this option, they will probably want to rather specify the few modules with side-effects instead of listing everything else. And it is more consistent with the rest of the JS ecosystem.Additional feature: `this.resolve(id, importer, {skipSelf: true})
Plugins can now request to skip their own
resolveId
hooks when using thethis.resolve
context function to find out how Rollup would resolve an id. This allows us to remove a lot of custom logic fromrollup-plugin-commonjs
that would fail if plugins resolved ids to objects, see rollup/rollup-plugin-commonjs#387Additional feature: Add
isEntry
flag tothis.getModuleInfo
Plugins can now identify entry points via
this.getModuleInfo(id)
. This will also work correctly for entry points injected by plugins. Again this helped to improve and simplify some logic in rollup-plugin-commonjs.Please also have a look at the included documentation changes for more details.
I will only merge this once we have an accompanying PR for rollup-plugin-node-resolve.