Add options and hooks to control module side effects (#2844)

* Implement basic support for returning the `pure` flag from `resolveId`

* Handle conflicts between different resolutions by defaulting to impure

* Add no-inferrable-types rule

* Handle conflicts between resolutions without an opinion about pureness and others

* Add purity to getModuleInfo, change logic to only use the first resolution
to determine purity

* Implement pureInternalModules and use as default

* Split test, regenerate iterator on demand

* Switch form pure to moduleSideEffects

* Replace pureInternalModules with hasModuleSideEffects and refine logic

* Implement load and transform hook handling

* Test all versions of the moduleSideEffects option

* Explain deprecation alternatives in JSDoc

* Document new options

* Try to fix Windows tests

* Rename test folder, mark modules as executed in LocalVariable

* Refine and simplify interaction of plugins with the user option. Now plugins
always override the user option.

* Add an option for this.resolve to skip the plugin calling it

* Add "isEntry" to "getModuleInfo"

* Provide "isEntry" information already in the load hook.

docs/05-plugins.md

@@ -140,10 +140,14 @@ Kind: `async, parallel`
 Cf. [`output.intro/output.outro`](guide/en#output-intro-output-outro).
 
 #### `load`
-Type: `(id: string) => string | null | { code: string, map?: string | SourceMap }`<br>
+Type: `(id: string) => string | null | { code: string, map?: string | SourceMap, ast? : ESTree.Program, moduleSideEffects?: boolean | null }`<br>
 Kind: `async, first`
 
-Defines a custom loader. Returning `null` defers to other `load` functions (and eventually the default behavior of loading from the file system).
+Defines a custom loader. Returning `null` defers to other `load` functions (and eventually the default behavior of loading from the file system). To prevent additional parsing overhead in case e.g. this hook already used `this.parse` to generate an AST for some reason, this hook can optionally return a `{ code, ast }` object. The `ast` must be a standard ESTree AST with `start` and `end` properties for each node.
+
+If `false` is returned for `moduleSideEffects` and no other module imports anything from this module, then this module will not be included in the bundle without checking for actual side-effects inside the module. If `true` is returned, Rollup will use its default algorithm to include all statements in the module that have side-effects (such as modifying a global or exported variable). If `null` is returned or the flag is omitted, then `moduleSideEffects` will be determined by the first `resolveId` hook that resolved this module, the `treeshake.moduleSideEffects` option, or eventually default to `true`. The `transform` hook can override this.
+
+You can use [`this.getModuleInfo`](guide/en#this-getmoduleinfo-moduleid-string-moduleinfo) to find out the previous value of `moduleSideEffects` inside this hook.
 
 #### `options`
 Type: `(options: InputOptions) => InputOptions | null`<br>
@@ -224,10 +228,10 @@ resolveFileUrl({fileName}) {
 ```
 
 #### `resolveId`
-Type: `(source: string, importer: string) => string | false | null | {id: string, external?: boolean}`<br>
+Type: `(source: string, importer: string) => string | false | null | {id: string, external?: boolean, moduleSideEffects?: boolean | null}`<br>
 Kind: `async, first`
 
-Defines a custom resolver. A resolver can be useful for e.g. locating third-party dependencies. Returning `null` defers to other `resolveId` functions (and eventually the default resolution behavior); returning `false` signals that `source` should be treated as an external module and not included in the bundle.
+Defines a custom resolver. A resolver can be useful for e.g. locating third-party dependencies. Returning `null` defers to other `resolveId` functions and eventually the default resolution behavior; returning `false` signals that `source` should be treated as an external module and not included in the bundle.
 
 If you return an object, then it is possible to resolve an import to a different id while excluding it from the bundle at the same time. This allows you to replace dependencies with external dependencies without the need for the user to mark them as "external" manually via the `external` option:
 
@@ -240,6 +244,8 @@ resolveId(source) {
 }
 ```
 
+If `false` is returned for `moduleSideEffects` in the first hook that resolves a module id and no other module imports anything from this module, then this module will not be included without checking for actual side-effects inside the module. If `true` is returned, Rollup will use its default algorithm to include all statements in the module that have side-effects (such as modifying a global or exported variable). If `null` is returned or the flag is omitted, then `moduleSideEffects` will be determined by the `treeshake.moduleSideEffects` option or default to `true`. The `load` and `transform` hooks can override this.
+
 #### `resolveImportMeta`
 Type: `(property: string | null, {chunkId: string, moduleId: string, format: string}) => string | null`<br>
 Kind: `sync, first`
@@ -263,11 +269,16 @@ resolveImportMeta(property, {moduleId}) {
 Note that since this hook has access to the filename of the current chunk, its return value will not be considered when generating the hash of this chunk.
 
 #### `transform`
-Type: `(code: string, id: string) => string | { code: string, map?: string | SourceMap, ast? : ESTree.Program } | null`
-<br>
+Type: `(code: string, id: string) => string | null | { code: string, map?: string | SourceMap, ast? : ESTree.Program, moduleSideEffects?: boolean | null }`<br>
 Kind: `async, sequential`
 
-Can be used to transform individual modules. Note that in watch mode, the result of this hook is cached when rebuilding and the hook is only triggered again for a module `id` if either the `code` of the module has changed or a file has changed that was added via `this.addWatchFile` the last time the hook was triggered for this module.
+Can be used to transform individual modules. To prevent additional parsing overhead in case e.g. this hook already used `this.parse` to generate an AST for some reason, this hook can optionally return a `{ code, ast }` object. The `ast` must be a standard ESTree AST with `start` and `end` properties for each node.
+
+Note that in watch mode, the result of this hook is cached when rebuilding and the hook is only triggered again for a module `id` if either the `code` of the module has changed or a file has changed that was added via `this.addWatchFile` the last time the hook was triggered for this module.
+
+If `false` is returned for `moduleSideEffects` and no other module imports anything from this module, then this module will not be included without checking for actual side-effects inside the module. If `true` is returned, Rollup will use its default algorithm to include all statements in the module that have side-effects (such as modifying a global or exported variable). If `null` is returned or the flag is omitted, then `moduleSideEffects` will be determined by the first `resolveId` hook that resolved this module, the `treeshake.moduleSideEffects` option, or eventually default to `true`.
+
+You can use [`this.getModuleInfo`](guide/en#this-getmoduleinfo-moduleid-string-moduleinfo) to find out the previous value of `moduleSideEffects` inside this hook.
 
 #### `watchChange`
 Type: `(id: string) => void`<br>
@@ -346,11 +357,13 @@ Get the file name of an emitted chunk. The file name will be relative to `output
 
 Returns additional information about the module in question in the form
 
-```js
+```
 {
-  id, // the id of the module, for convenience
-  isExternal, // for external modules that are not included in the graph
-  importedIds // the module ids imported by this module
+  id: string, // the id of the module, for convenience
+  isEntry: boolean, // is this a user- or plugin-defined entry point
+  isExternal: boolean, // for external modules that are not included in the graph
+  importedIds: string[], // the module ids imported by this module
+  hasModuleSideEffects: boolean // are imports of this module included if nothing is imported from it
 }
 ```
 
@@ -374,9 +387,11 @@ or converted into an Array via `Array.from(this.moduleIds)`.
 
 Use Rollup's internal acorn instance to parse code to an AST.
 
-#### `this.resolve(source: string, importer: string) => Promise<{id: string, external: boolean} | null>`
+#### `this.resolve(source: string, importer: string, options?: {skipSelf: boolean}) => Promise<{id: string, external: boolean} | null>`
 Resolve imports to module ids (i.e. file names) using the same plugins that Rollup uses, and determine if an import should be external. If `null` is returned, the import could not be resolved by Rollup or any plugin but was not explicitly marked as external by the user.
 
+If you pass `skipSelf: true`, then the `resolveId` hook of the plugin from which `this.resolve` is called will be skipped when resolving.
+
 #### `this.setAssetSource(assetReferenceId: string, source: string | Buffer) => void`
 
 Set the deferred source of an asset.
@@ -502,10 +517,6 @@ export const size = 6;
 
 If you build this code, both the main chunk and the worklet will share the code from `config.js` via a shared chunk. This enables us to make use of the browser cache to reduce transmitted data and speed up loading the worklet.
 
-### Advanced Loaders
-
-The `load` hook can optionally return a `{ code, ast }` object. The `ast` must be a standard ESTree AST with `start` and `end` properties for each node.
-
 ### Transformers
 
 Transformer plugins (i.e. those that return a `transform` function for e.g. transpiling non-JS files) should support `options.include` and `options.exclude`, both of which can be a minimatch pattern or an array of minimatch patterns. If `options.include` is omitted or of zero length, files should be included by default; otherwise they should only be included if the ID matches one of the patterns.

docs/999-big-list-of-options.md

@@ -729,32 +729,13 @@ Default: `false`
 If this option is provided, bundling will not fail if bindings are imported from a file that does not define these bindings. Instead, new variables will be created for these bindings with the value `undefined`.
 
 #### treeshake
-Type: `boolean | { propertyReadSideEffects?: boolean, annotations?: boolean, pureExternalModules?: boolean }`<br>
+Type: `boolean | { annotations?: boolean, moduleSideEffects?: ModuleSideEffectsOption, propertyReadSideEffects?: boolean }`<br>
 CLI: `--treeshake`/`--no-treeshake`<br>
 Default: `true`
 
 Whether or not to apply tree-shaking and to fine-tune the tree-shaking process. Setting this option to `false` will produce bigger bundles but may improve build performance. If you discover a bug caused by the tree-shaking algorithm, please file an issue!
 Setting this option to an object implies tree-shaking is enabled and grants the following additional options:
 
-**treeshake.propertyReadSideEffects**
-Type: `boolean`<br>
-CLI: `--treeshake.propertyReadSideEffects`/`--no-treeshake.propertyReadSideEffects`<br>
-Default: `true`
-
-If `false`, assume reading a property of an object never has side-effects. Depending on your code, disabling this option can significantly reduce bundle size but can potentially break functionality if you rely on getters or errors from illegal property access.
-
-```javascript
-// Will be removed if treeshake.propertyReadSideEffects === false
-const foo = {
-  get bar() {
-    console.log('effect');
-    return 'bar';
-  }
-}
-const result = foo.bar;
-const illegalAccess = foo.quux.tooDeep;
-```
-
 **treeshake.annotations**<br>
 Type: `boolean`<br>
 CLI: `--treeshake.annotations`/`--no-treeshake.annotations`<br>
@@ -774,12 +755,12 @@ class Impure {
 /*@__PURE__*/new Impure();
 ```
 
-**treeshake.pureExternalModules**<br>
-Type: `boolean`<br>
-CLI: `--treeshake.pureExternalModules`/`--no-treeshake.pureExternalModules`<br>
-Default: `false`
+**treeshake.moduleSideEffects**<br>
+Type: `boolean | "no-external" | string[] | (id: string, external: boolean) => boolean`<br>
+CLI: `--treeshake.moduleSideEffects`/`--no-treeshake.moduleSideEffects`<br>
+Default: `true`
 
-If `true`, assume external dependencies from which nothing is imported do not have other side-effects like mutating global variables or logging.
+If `false`, assume modules and external dependencies from which nothing is imported do not have other side-effects like mutating global variables or logging without checking. For external dependencies, this will suppress empty imports:
 
 ```javascript
 // input file
@@ -789,17 +770,61 @@ console.log(42);
 ```
 
 ```javascript
-// output with treeshake.pureExternalModules === false
+// output with treeshake.moduleSideEffects === true
 import 'external-a';
 import 'external-b';
 console.log(42);
 ```
 
 ```javascript
-// output with treeshake.pureExternalModules === true
+// output with treeshake.moduleSideEffects === false
+console.log(42);
+```
+
+For non-external modules, `false` will not include any statements from a module unless at least one import from this module is included:
+
+```javascript
+// input file a.js
+import {unused} from './b.js';
+console.log(42);
+
+// input file b.js
+console.log('side-effect');
+```
+
+```javascript
+// output with treeshake.moduleSideEffects === true
+console.log('side-effect');
+
 console.log(42);
 ```
 
+```javascript
+// output with treeshake.moduleSideEffects === false
+console.log(42);
+```
+
+You can also supply a list of modules with side-effects or a function to determine it for each module individually. The value `"no-external"` will only remove external imports if possible and is equivalent to the function `(id, external) => !external`;
+
+**treeshake.propertyReadSideEffects**
+Type: `boolean`<br>
+CLI: `--treeshake.propertyReadSideEffects`/`--no-treeshake.propertyReadSideEffects`<br>
+Default: `true`
+
+If `false`, assume reading a property of an object never has side-effects. Depending on your code, disabling this option can significantly reduce bundle size but can potentially break functionality if you rely on getters or errors from illegal property access.
+
+```javascript
+// Will be removed if treeshake.propertyReadSideEffects === false
+const foo = {
+  get bar() {
+    console.log('effect');
+    return 'bar';
+  }
+}
+const result = foo.bar;
+const illegalAccess = foo.quux.tooDeep;
+```
+
 ### Experimental options
 
 These options reflect new features that have not yet been fully finalized. Availability, behaviour and usage may therefore be subject to change between minor versions.
@@ -903,3 +928,35 @@ export default {
   }
 };
 ```
+
+### Deprecated options
+
+☢️ These options have been deprecated and may be removed in a future Rollup version.
+
+#### treeshake.pureExternalModules
+Type: `boolean | string[] | (id: string) => boolean | null`<br>
+CLI: `--treeshake.pureExternalModules`/`--no-treeshake.pureExternalModules`<br>
+Default: `false`
+
+If `true`, assume external dependencies from which nothing is imported do not have other side-effects like mutating global variables or logging.
+
+```javascript
+// input file
+import {unused} from 'external-a';
+import 'external-b';
+console.log(42);
+```
+
+```javascript
+// output with treeshake.pureExternalModules === false
+import 'external-a';
+import 'external-b';
+console.log(42);
+```
+
+```javascript
+// output with treeshake.pureExternalModules === true
+console.log(42);
+```
+
+You can also supply a list of external ids to be considered pure or a function that is called whenever an external import could be removed.

src/Chunk.ts

@@ -114,10 +114,10 @@ export function isChunkRendered(chunk: Chunk): boolean {
 export default class Chunk {
 	entryModules: Module[] = [];
 	execIndex: number;
-	exportMode: string = 'named';
+	exportMode = 'named';
 	facadeModule: Module | null = null;
 	graph: Graph;
-	hasDynamicImport: boolean = false;
+	hasDynamicImport = false;
 	id: string = undefined;
 	indentString: string = undefined;
 	isEmpty: boolean;
@@ -135,7 +135,7 @@ export default class Chunk {
 	private exportNames: { [name: string]: Variable } = Object.create(null);
 	private exports = new Set<Variable>();
 	private imports = new Set<Variable>();
-	private needsExportsShim: boolean = false;
+	private needsExportsShim = false;
 	private renderedDeclarations: {
 		dependencies: ChunkDependencies;
 		exports: ChunkExports;
@@ -753,7 +753,7 @@ export default class Chunk {
 			if (depModule instanceof Module) {
 				dependency = depModule.chunk;
 			} else {
-				if (!depModule.used && this.graph.isPureExternalModule(depModule.id)) {
+				if (!(depModule.used || depModule.moduleSideEffects)) {
 					continue;
 				}
 				dependency = depModule;

src/ExternalModule.ts

@@ -10,24 +10,25 @@ export default class ExternalModule {
 	execIndex: number;
 	exportedVariables: Map<ExternalVariable, string>;
 	exportsNames = false;
-	exportsNamespace: boolean = false;
+	exportsNamespace = false;
 	id: string;
-	isEntryPoint = false;
 	isExternal = true;
-	mostCommonSuggestion: number = 0;
+	moduleSideEffects: boolean;
+	mostCommonSuggestion = 0;
 	nameSuggestions: { [name: string]: number };
-	reexported: boolean = false;
+	reexported = false;
 	renderPath: string = undefined;
 	renormalizeRenderPath = false;
 	used = false;
 	variableName: string;
 
 	private graph: Graph;
 
-	constructor({ graph, id }: { graph: Graph; id: string }) {
+	constructor(graph: Graph, id: string, moduleSideEffects: boolean) {
 		this.graph = graph;
 		this.id = id;
 		this.execIndex = Infinity;
+		this.moduleSideEffects = moduleSideEffects;
 
 		const parts = id.split(/[\\/]/);
 		this.variableName = makeLegal(parts.pop());

src/Graph.ts

@@ -67,10 +67,9 @@ export default class Graph {
 	curChunkIndex = 0;
 	deoptimizationTracker: EntityPathTracker;
 	getModuleContext: (id: string) => string;
-	isPureExternalModule: (id: string) => boolean;
 	moduleById = new Map<string, Module | ExternalModule>();
 	moduleLoader: ModuleLoader;
-	needsTreeshakingPass: boolean = false;
+	needsTreeshakingPass = false;
 	phase: BuildPhase = BuildPhase.LOAD_AND_PARSE;
 	pluginDriver: PluginDriver;
 	preserveModules: boolean;
@@ -114,23 +113,17 @@ export default class Graph {
 			this.treeshakingOptions = options.treeshake
 				? {
 						annotations: (<TreeshakingOptions>options.treeshake).annotations !== false,
+						moduleSideEffects: (<TreeshakingOptions>options.treeshake).moduleSideEffects,
 						propertyReadSideEffects:
 							(<TreeshakingOptions>options.treeshake).propertyReadSideEffects !== false,
 						pureExternalModules: (<TreeshakingOptions>options.treeshake).pureExternalModules
 				  }
-				: { propertyReadSideEffects: true, annotations: true, pureExternalModules: false };
-			if (this.treeshakingOptions.pureExternalModules === true) {
-				this.isPureExternalModule = () => true;
-			} else if (typeof this.treeshakingOptions.pureExternalModules === 'function') {
-				this.isPureExternalModule = this.treeshakingOptions.pureExternalModules;
-			} else if (Array.isArray(this.treeshakingOptions.pureExternalModules)) {
-				const pureExternalModules = new Set(this.treeshakingOptions.pureExternalModules);
-				this.isPureExternalModule = id => pureExternalModules.has(id);
-			} else {
-				this.isPureExternalModule = () => false;
-			}
-		} else {
-			this.isPureExternalModule = () => false;
+				: {
+						annotations: true,
+						moduleSideEffects: true,
+						propertyReadSideEffects: true,
+						pureExternalModules: false
+				  };
 		}
 
 		this.contextParse = (code: string, options: acorn.Options = {}) =>
@@ -193,7 +186,9 @@ export default class Graph {
 			this.moduleById,
 			this.pluginDriver,
 			options.external,
-			typeof options.manualChunks === 'function' && options.manualChunks
+			typeof options.manualChunks === 'function' && options.manualChunks,
+			this.treeshake ? this.treeshakingOptions.moduleSideEffects : null,
+			this.treeshake ? this.treeshakingOptions.pureExternalModules : false
 		);
 	}