From 5963243f8d2779353571271ed6308cfa84f96630 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Fri, 3 May 2019 16:54:17 +0200 Subject: [PATCH 01/19] Implement basic support for returning the `pure` flag from `resolveId` --- src/Chunk.ts | 3 +- src/ExternalModule.ts | 3 +- src/Module.ts | 15 ++--- src/ModuleLoader.ts | 19 +++++- src/rollup/types.d.ts | 7 ++- src/utils/executionOrder.ts | 6 -- src/utils/traverseStaticDependencies.ts | 15 +++-- .../samples/context-resolve/_config.js | 26 +++++--- .../module-side-effects/resolve-id/_config.js | 63 +++++++++++++++++++ .../module-side-effects/resolve-id/main.js | 14 +++++ test/incremental/index.js | 4 +- 11 files changed, 139 insertions(+), 36 deletions(-) create mode 100644 test/function/samples/module-side-effects/resolve-id/_config.js create mode 100644 test/function/samples/module-side-effects/resolve-id/main.js diff --git a/src/Chunk.ts b/src/Chunk.ts index f6df6ad2ba6..75085374ec4 100644 --- a/src/Chunk.ts +++ b/src/Chunk.ts @@ -753,7 +753,8 @@ export default class Chunk { if (depModule instanceof Module) { dependency = depModule.chunk; } else { - if (!depModule.used && this.graph.isPureExternalModule(depModule.id)) { + // TODO Lukas can we use a different flag that is only set depending on pureness? + if (!depModule.used && (depModule.pure || this.graph.isPureExternalModule(depModule.id))) { continue; } dependency = depModule; diff --git a/src/ExternalModule.ts b/src/ExternalModule.ts index 0b5dae5f44c..6df40a773c9 100644 --- a/src/ExternalModule.ts +++ b/src/ExternalModule.ts @@ -16,7 +16,8 @@ export default class ExternalModule { isExternal = true; mostCommonSuggestion: number = 0; nameSuggestions: { [name: string]: number }; - reexported: boolean = false; + pure = false; + reexported = false; renderPath: string = undefined; renormalizeRenderPath = false; used = false; diff --git a/src/Module.ts b/src/Module.ts index 0d415a4b4b5..d7663b02351 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -46,7 +46,7 @@ import relativeId from './utils/relativeId'; import { RenderOptions } from './utils/renderHelpers'; import { SOURCEMAPPING_URL_RE } from './utils/sourceMappingURL'; import { timeEnd, timeStart } from './utils/timers'; -import { visitStaticModuleDependencies } from './utils/traverseStaticDependencies'; +import { markModuleAndImpureDependenciesAsExecuted } from './utils/traverseStaticDependencies'; import { MISSING_EXPORT_SHIM_VARIABLE } from './utils/variableNames'; export interface CommentDescription { @@ -194,6 +194,7 @@ export default class Module { manualChunkAlias: string = null; originalCode: string; originalSourcemap: RawSourceMap | void; + pure = false; reexports: { [name: string]: ReexportDescription } = Object.create(null); resolvedIds: ResolvedIdMap; scope: ModuleScope; @@ -410,11 +411,7 @@ export default class Module { includeAllExports() { if (!this.isExecuted) { this.graph.needsTreeshakingPass = true; - visitStaticModuleDependencies(this, module => { - if (module instanceof ExternalModule || module.isExecuted) return true; - module.isExecuted = true; - return false; - }); + markModuleAndImpureDependenciesAsExecuted(this); } for (const exportName of this.getExports()) { @@ -768,11 +765,15 @@ export default class Module { } private includeVariable(variable: Variable) { + const variableModule = variable.module; if (!variable.included) { variable.include(); + if (variableModule instanceof Module && !variableModule.isExecuted) { + markModuleAndImpureDependenciesAsExecuted(variableModule); + } this.graph.needsTreeshakingPass = true; } - if (variable.module && variable.module !== this) { + if (variableModule && variableModule !== this) { this.imports.add(variable); } } diff --git a/src/ModuleLoader.ts b/src/ModuleLoader.ts index 140d83c0049..7d885b87883 100644 --- a/src/ModuleLoader.ts +++ b/src/ModuleLoader.ts @@ -327,9 +327,17 @@ export class ModuleLoader { if (externalModule instanceof ExternalModule === false) { error(errInternalIdCannotBeExternal(source, importer)); } + if (resolvedId.pure) { + externalModule.pure = true; + } return Promise.resolve(externalModule); } else { - return this.fetchModule(resolvedId.id, importer); + return this.fetchModule(resolvedId.id, importer).then(module => { + if (resolvedId.pure) { + module.pure = true; + } + return module; + }); } } @@ -343,7 +351,7 @@ export class ModuleLoader { error(errUnresolvedImport(source, importer)); } this.graph.warn(errUnresolvedImportTreatedAsExternal(source, importer)); - return { id: source, external: true }; + return { id: source, external: true, pure: false }; } return resolvedId; } @@ -384,12 +392,16 @@ export class ModuleLoader { ): ResolvedId | null { let id = ''; let external = false; + let pure = false; if (resolveIdResult) { if (typeof resolveIdResult === 'object') { id = resolveIdResult.id; if (resolveIdResult.external) { external = true; } + if (resolveIdResult.pure) { + pure = true; + } } else { id = resolveIdResult; if (this.isExternal(id, importer, true)) { @@ -406,7 +418,7 @@ export class ModuleLoader { } external = true; } - return { id, external }; + return { id, external, pure }; } private resolveAndFetchDependency( @@ -441,6 +453,7 @@ export class ModuleLoader { } return { external: false, + pure: false, ...resolution }; } diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index d2a83d7bff2..05aab097e51 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -141,13 +141,18 @@ export interface PluginContextMeta { export interface ResolvedId { external: boolean; id: string; + pure: boolean; } export interface ResolvedIdMap { [key: string]: ResolvedId; } -type PartialResolvedId = Partial & { id: string }; +interface PartialResolvedId { + external?: boolean; + id: string; + pure?: boolean | null; +} export type ResolveIdResult = string | false | void | PartialResolvedId; diff --git a/src/utils/executionOrder.ts b/src/utils/executionOrder.ts index 29f685abb78..c09f259472f 100644 --- a/src/utils/executionOrder.ts +++ b/src/utils/executionOrder.ts @@ -15,7 +15,6 @@ export function sortByExecutionOrder(units: OrderedExecutionUnit[]) { export function analyseModuleExecution(entryModules: Module[]) { let nextExecIndex = 0; - let inStaticGraph = true; const cyclePaths: string[][] = []; const analysedModules: { [id: string]: boolean } = {}; const orderedModules: Module[] = []; @@ -31,9 +30,6 @@ export function analyseModuleExecution(entryModules: Module[]) { return; } - if (inStaticGraph) { - module.isExecuted = true; - } for (const dependency of module.dependencies) { if (dependency.id in parents) { if (!analysedModules[dependency.id]) { @@ -63,8 +59,6 @@ export function analyseModuleExecution(entryModules: Module[]) { analyseModule(curEntry); } } - - inStaticGraph = false; for (const curEntry of dynamicImports) { if (!parents[curEntry.id]) { parents[curEntry.id] = null; diff --git a/src/utils/traverseStaticDependencies.ts b/src/utils/traverseStaticDependencies.ts index a2cb4b12ac3..8d9e4a7b7cd 100644 --- a/src/utils/traverseStaticDependencies.ts +++ b/src/utils/traverseStaticDependencies.ts @@ -2,16 +2,19 @@ import ExternalModule from '../ExternalModule'; import Module from '../Module'; import { NameCollection } from './reservedNames'; -export function visitStaticModuleDependencies( - baseModule: Module | ExternalModule, - areDependenciesSkipped: (module: Module | ExternalModule) => boolean -) { +export function markModuleAndImpureDependenciesAsExecuted(baseModule: Module) { + baseModule.isExecuted = true; const modules = [baseModule]; const visitedModules: NameCollection = {}; for (const module of modules) { - if (areDependenciesSkipped(module) || module instanceof ExternalModule) continue; for (const dependency of module.dependencies) { - if (!visitedModules[dependency.id]) { + if ( + !(dependency instanceof ExternalModule) && + !dependency.isExecuted && + !dependency.pure && + !visitedModules[dependency.id] + ) { + dependency.isExecuted = true; visitedModules[dependency.id] = true; modules.push(dependency); } diff --git a/test/function/samples/context-resolve/_config.js b/test/function/samples/context-resolve/_config.js index 8a6d1eff991..38ac7c3be67 100644 --- a/test/function/samples/context-resolve/_config.js +++ b/test/function/samples/context-resolve/_config.js @@ -4,7 +4,7 @@ const assert = require('assert'); const tests = [ { source: './existing', - expected: { id: path.resolve(__dirname, 'existing.js'), external: false } + expected: { id: path.resolve(__dirname, 'existing.js'), external: false, pure: false } }, { source: './missing-relative', @@ -16,35 +16,43 @@ const tests = [ }, { source: './marked-directly-external-relative', - expected: { id: path.resolve(__dirname, 'marked-directly-external-relative'), external: true } + expected: { + id: path.resolve(__dirname, 'marked-directly-external-relative'), + external: true, + pure: false + } }, { source: './marked-external-relative', - expected: { id: path.resolve(__dirname, 'marked-external-relative'), external: true } + expected: { + id: path.resolve(__dirname, 'marked-external-relative'), + external: true, + pure: false + } }, { source: 'marked-external-absolute', - expected: { id: 'marked-external-absolute', external: true } + expected: { id: 'marked-external-absolute', external: true, pure: false } }, { source: 'resolved-name', - expected: { id: 'resolved:resolved-name', external: false } + expected: { id: 'resolved:resolved-name', external: false, pure: false } }, { source: 'resolved-false', - expected: { id: 'resolved-false', external: true } + expected: { id: 'resolved-false', external: true, pure: false } }, { source: 'resolved-object', - expected: { id: 'resolved:resolved-object', external: false } + expected: { id: 'resolved:resolved-object', external: false, pure: false } }, { source: 'resolved-object-non-external', - expected: { id: 'resolved:resolved-object-non-external', external: false } + expected: { id: 'resolved:resolved-object-non-external', external: false, pure: false } }, { source: 'resolved-object-external', - expected: { id: 'resolved:resolved-object-external', external: true } + expected: { id: 'resolved:resolved-object-external', external: true, pure: false } } ]; diff --git a/test/function/samples/module-side-effects/resolve-id/_config.js b/test/function/samples/module-side-effects/resolve-id/_config.js new file mode 100644 index 00000000000..52135837656 --- /dev/null +++ b/test/function/samples/module-side-effects/resolve-id/_config.js @@ -0,0 +1,63 @@ +const assert = require('assert'); +const sideEffects = []; + +// TODO Lukas interaction with pureExternalModules, use as default? +// TODO Lukas how to handle if one says pure and one says impure +module.exports = { + description: 'does not include modules without used exports if moduleSideEffect is false', + context: { + sideEffects, + require(id) { + sideEffects.push(id); + return { value: id }; + } + }, + exports() { + assert.deepStrictEqual(sideEffects, [ + 'pure-false-external-true', + 'pure-true-external-true-used-import', + 'pure-false-external-true-unused-import', + 'pure-false-external-true-used-import', + 'pure-false-external-false', + 'pure-true-external-false-used-import', + 'pure-false-external-false-unused-import', + 'pure-false-external-false-used-import' + ]); + }, + options: { + plugins: { + name: 'test-plugin', + resolveId(id) { + if (!(id[0] === '/')) { + const [, pure, , external] = id.split('-'); + return { + id, + external: JSON.parse(external), + pure: JSON.parse(pure) + }; + } + }, + load(id) { + if (!(id[0] === '/')) { + return `export const value = '${id}'; sideEffects.push(value);`; + } + } + } + }, + warnings: [ + { + code: 'UNUSED_EXTERNAL_IMPORT', + message: + "'value' is imported from external module 'pure-true-external-true-unused-import' but never used", + names: ['value'], + source: 'pure-true-external-true-unused-import' + }, + { + code: 'UNUSED_EXTERNAL_IMPORT', + message: + "'value' is imported from external module 'pure-false-external-true-unused-import' but never used", + names: ['value'], + source: 'pure-false-external-true-unused-import' + } + ] +}; diff --git a/test/function/samples/module-side-effects/resolve-id/main.js b/test/function/samples/module-side-effects/resolve-id/main.js new file mode 100644 index 00000000000..2d1cd227643 --- /dev/null +++ b/test/function/samples/module-side-effects/resolve-id/main.js @@ -0,0 +1,14 @@ +import 'pure-true-external-false'; +import 'pure-true-external-true'; +import 'pure-false-external-false';// included +import 'pure-false-external-true';// included +import {value as unusedValue1} from 'pure-true-external-false-unused-import'; +import {value as usedValue1} from 'pure-true-external-false-used-import';// included +import {value as unusedValue2} from 'pure-true-external-true-unused-import'; +import {value as usedValue2} from 'pure-true-external-true-used-import';// included +import {value as unusedValue3} from 'pure-false-external-false-unused-import';// included +import {value as usedValue3} from 'pure-false-external-false-used-import';// included +import {value as unusedValue4} from 'pure-false-external-true-unused-import';// included +import {value as usedValue4} from 'pure-false-external-true-used-import';// included + +export const values = [usedValue1, usedValue2, usedValue3, usedValue4]; diff --git a/test/incremental/index.js b/test/incremental/index.js index 78158a89325..0afcbabeec7 100644 --- a/test/incremental/index.js +++ b/test/incremental/index.js @@ -240,8 +240,8 @@ describe('incremental', () => { assert.equal(bundle.cache.modules[1].id, 'entry'); assert.deepEqual(bundle.cache.modules[1].resolvedIds, { - foo: { id: 'foo', external: false }, - external: { id: 'external', external: true } + foo: { id: 'foo', external: false, pure: false }, + external: { id: 'external', external: true, pure: false } }); }); }); From e76e510ea5b2e948541fb46dde06a92fc6e1cb35 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Fri, 3 May 2019 17:51:17 +0200 Subject: [PATCH 02/19] Handle conflicts between different resolutions by defaulting to impure --- src/ExternalModule.ts | 2 +- src/Module.ts | 2 +- src/ModuleLoader.ts | 9 ++-- .../resolve-id-conflict/1-pure.js | 2 + .../resolve-id-conflict/2-pure.js | 2 + .../resolve-id-conflict/_config.js | 46 +++++++++++++++++++ .../resolve-id-conflict/main.js | 3 ++ .../resolve-id-conflict/sideeffects.js | 2 + .../module-side-effects/resolve-id/_config.js | 7 ++- 9 files changed, 63 insertions(+), 12 deletions(-) create mode 100644 test/function/samples/module-side-effects/resolve-id-conflict/1-pure.js create mode 100644 test/function/samples/module-side-effects/resolve-id-conflict/2-pure.js create mode 100644 test/function/samples/module-side-effects/resolve-id-conflict/_config.js create mode 100644 test/function/samples/module-side-effects/resolve-id-conflict/main.js create mode 100644 test/function/samples/module-side-effects/resolve-id-conflict/sideeffects.js diff --git a/src/ExternalModule.ts b/src/ExternalModule.ts index 6df40a773c9..68317603131 100644 --- a/src/ExternalModule.ts +++ b/src/ExternalModule.ts @@ -16,7 +16,7 @@ export default class ExternalModule { isExternal = true; mostCommonSuggestion: number = 0; nameSuggestions: { [name: string]: number }; - pure = false; + pure: boolean | null = null; reexported = false; renderPath: string = undefined; renormalizeRenderPath = false; diff --git a/src/Module.ts b/src/Module.ts index d7663b02351..8801a939e33 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -194,7 +194,7 @@ export default class Module { manualChunkAlias: string = null; originalCode: string; originalSourcemap: RawSourceMap | void; - pure = false; + pure: boolean | null = null; reexports: { [name: string]: ReexportDescription } = Object.create(null); resolvedIds: ResolvedIdMap; scope: ModuleScope; diff --git a/src/ModuleLoader.ts b/src/ModuleLoader.ts index 7d885b87883..d0234606b89 100644 --- a/src/ModuleLoader.ts +++ b/src/ModuleLoader.ts @@ -327,15 +327,12 @@ export class ModuleLoader { if (externalModule instanceof ExternalModule === false) { error(errInternalIdCannotBeExternal(source, importer)); } - if (resolvedId.pure) { - externalModule.pure = true; - } + externalModule.pure = + externalModule.pure === null ? resolvedId.pure : externalModule.pure && resolvedId.pure; return Promise.resolve(externalModule); } else { return this.fetchModule(resolvedId.id, importer).then(module => { - if (resolvedId.pure) { - module.pure = true; - } + module.pure = module.pure === null ? resolvedId.pure : module.pure && resolvedId.pure; return module; }); } diff --git a/test/function/samples/module-side-effects/resolve-id-conflict/1-pure.js b/test/function/samples/module-side-effects/resolve-id-conflict/1-pure.js new file mode 100644 index 00000000000..e648bc4dae6 --- /dev/null +++ b/test/function/samples/module-side-effects/resolve-id-conflict/1-pure.js @@ -0,0 +1,2 @@ +import 'external'; +import 'internal'; diff --git a/test/function/samples/module-side-effects/resolve-id-conflict/2-pure.js b/test/function/samples/module-side-effects/resolve-id-conflict/2-pure.js new file mode 100644 index 00000000000..e648bc4dae6 --- /dev/null +++ b/test/function/samples/module-side-effects/resolve-id-conflict/2-pure.js @@ -0,0 +1,2 @@ +import 'external'; +import 'internal'; diff --git a/test/function/samples/module-side-effects/resolve-id-conflict/_config.js b/test/function/samples/module-side-effects/resolve-id-conflict/_config.js new file mode 100644 index 00000000000..c46850fde20 --- /dev/null +++ b/test/function/samples/module-side-effects/resolve-id-conflict/_config.js @@ -0,0 +1,46 @@ +const assert = require('assert'); +const sideEffects = []; + +// TODO Lukas interaction with pureExternalModules, use as default? +// TODO Lukas resolvers without an opinion should not override pureness +module.exports = { + solo: true, + description: 'handles conflicts between different resolveId results', + context: { + sideEffects, + require(id) { + sideEffects.push(id); + return { value: id }; + } + }, + exports() { + assert.deepStrictEqual(sideEffects, ['external', 'internal']); + }, + options: { + plugins: { + name: 'test-plugin', + resolveId(id, importer) { + const pure = importer && importer.endsWith('pure.js'); + switch (id) { + case 'internal': + return { + external: false, + id, + pure + }; + case 'external': + return { + external: true, + id, + pure + }; + } + }, + load(id) { + if (id === 'internal') { + return `export const value = '${id}'; sideEffects.push(value);`; + } + } + } + } +}; diff --git a/test/function/samples/module-side-effects/resolve-id-conflict/main.js b/test/function/samples/module-side-effects/resolve-id-conflict/main.js new file mode 100644 index 00000000000..3bcc2e3b5bc --- /dev/null +++ b/test/function/samples/module-side-effects/resolve-id-conflict/main.js @@ -0,0 +1,3 @@ +import './1-pure.js'; +import './sideeffects.js'; +import './2-pure.js'; diff --git a/test/function/samples/module-side-effects/resolve-id-conflict/sideeffects.js b/test/function/samples/module-side-effects/resolve-id-conflict/sideeffects.js new file mode 100644 index 00000000000..e648bc4dae6 --- /dev/null +++ b/test/function/samples/module-side-effects/resolve-id-conflict/sideeffects.js @@ -0,0 +1,2 @@ +import 'external'; +import 'internal'; diff --git a/test/function/samples/module-side-effects/resolve-id/_config.js b/test/function/samples/module-side-effects/resolve-id/_config.js index 52135837656..0d80a363a94 100644 --- a/test/function/samples/module-side-effects/resolve-id/_config.js +++ b/test/function/samples/module-side-effects/resolve-id/_config.js @@ -1,9 +1,8 @@ const assert = require('assert'); const sideEffects = []; -// TODO Lukas interaction with pureExternalModules, use as default? -// TODO Lukas how to handle if one says pure and one says impure module.exports = { + solo: true, description: 'does not include modules without used exports if moduleSideEffect is false', context: { sideEffects, @@ -28,7 +27,7 @@ module.exports = { plugins: { name: 'test-plugin', resolveId(id) { - if (!(id[0] === '/')) { + if (id[0] !== '/') { const [, pure, , external] = id.split('-'); return { id, @@ -38,7 +37,7 @@ module.exports = { } }, load(id) { - if (!(id[0] === '/')) { + if (id[0] !== '/') { return `export const value = '${id}'; sideEffects.push(value);`; } } From 322435cfd9b0b3f073f1984774f43c21199d4377 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sat, 4 May 2019 07:29:42 +0200 Subject: [PATCH 03/19] Add no-inferrable-types rule --- src/Chunk.ts | 6 +++--- src/ExternalModule.ts | 4 ++-- src/Graph.ts | 2 +- src/Module.ts | 10 +++++----- src/ast/scopes/BlockScope.ts | 2 +- src/ast/scopes/CatchScope.ts | 2 +- src/ast/values.ts | 4 ++-- src/ast/variables/NamespaceVariable.ts | 4 ++-- src/ast/variables/Variable.ts | 8 ++++---- src/utils/defaultPlugin.ts | 2 +- src/utils/renderHelpers.ts | 8 ++------ src/utils/timers.ts | 4 ++-- src/watch/index.ts | 4 ++-- tslint.json | 1 + 14 files changed, 29 insertions(+), 32 deletions(-) diff --git a/src/Chunk.ts b/src/Chunk.ts index 75085374ec4..38e004ef1e2 100644 --- a/src/Chunk.ts +++ b/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(); private imports = new Set(); - private needsExportsShim: boolean = false; + private needsExportsShim = false; private renderedDeclarations: { dependencies: ChunkDependencies; exports: ChunkExports; diff --git a/src/ExternalModule.ts b/src/ExternalModule.ts index 68317603131..88514b5bb66 100644 --- a/src/ExternalModule.ts +++ b/src/ExternalModule.ts @@ -10,11 +10,11 @@ export default class ExternalModule { execIndex: number; exportedVariables: Map; exportsNames = false; - exportsNamespace: boolean = false; + exportsNamespace = false; id: string; isEntryPoint = false; isExternal = true; - mostCommonSuggestion: number = 0; + mostCommonSuggestion = 0; nameSuggestions: { [name: string]: number }; pure: boolean | null = null; reexported = false; diff --git a/src/Graph.ts b/src/Graph.ts index dd40cb55caa..1e237dbf407 100644 --- a/src/Graph.ts +++ b/src/Graph.ts @@ -70,7 +70,7 @@ export default class Graph { isPureExternalModule: (id: string) => boolean; moduleById = new Map(); moduleLoader: ModuleLoader; - needsTreeshakingPass: boolean = false; + needsTreeshakingPass = false; phase: BuildPhase = BuildPhase.LOAD_AND_PARSE; pluginDriver: PluginDriver; preserveModules: boolean; diff --git a/src/Module.ts b/src/Module.ts index 8801a939e33..eab56b830db 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -176,7 +176,7 @@ export default class Module { }[] = []; entryPointsHash: Uint8Array = new Uint8Array(10); excludeFromSourcemap: boolean; - execIndex: number = Infinity; + execIndex = Infinity; exportAllModules: (Module | ExternalModule)[] = null; exportAllSources: string[] = []; exports: { [name: string]: ExportDescription } = Object.create(null); @@ -187,10 +187,10 @@ export default class Module { importDescriptions: { [name: string]: ImportDescription } = Object.create(null); importMetas: MetaProperty[] = []; imports = new Set(); - isEntryPoint: boolean = false; - isExecuted: boolean = false; + isEntryPoint = false; + isExecuted = false; isExternal: false; - isUserDefinedEntryPoint: boolean = false; + isUserDefinedEntryPoint = false; manualChunkAlias: string = null; originalCode: string; originalSourcemap: RawSourceMap | void; @@ -201,7 +201,7 @@ export default class Module { sourcemapChain: RawSourceMap[]; sources: string[] = []; transformAssets: Asset[]; - usesTopLevelAwait: boolean = false; + usesTopLevelAwait = false; private ast: Program; private astContext: AstContext; diff --git a/src/ast/scopes/BlockScope.ts b/src/ast/scopes/BlockScope.ts index 5464683bda9..b50141b778b 100644 --- a/src/ast/scopes/BlockScope.ts +++ b/src/ast/scopes/BlockScope.ts @@ -10,7 +10,7 @@ export default class BlockScope extends ChildScope { identifier: Identifier, context: AstContext, init: ExpressionEntity | null = null, - isHoisted: boolean = false + isHoisted = false ) { if (isHoisted) { return this.parent.addDeclaration( diff --git a/src/ast/scopes/CatchScope.ts b/src/ast/scopes/CatchScope.ts index 913c4dc9e31..bafa90a1e06 100644 --- a/src/ast/scopes/CatchScope.ts +++ b/src/ast/scopes/CatchScope.ts @@ -9,7 +9,7 @@ export default class CatchScope extends ParameterScope { identifier: Identifier, context: AstContext, init: ExpressionEntity | null = null, - isHoisted: boolean = false + isHoisted = false ) { if (isHoisted) { return this.parent.addDeclaration(identifier, context, init, true) as LocalVariable; diff --git a/src/ast/values.ts b/src/ast/values.ts index 4e68c6c9eed..329a60dcae0 100644 --- a/src/ast/values.ts +++ b/src/ast/values.ts @@ -79,7 +79,7 @@ const callsArgReturnsUnknown: RawMemberDescription = { }; export class UnknownArrayExpression implements ExpressionEntity { - included: boolean = false; + included = false; deoptimizePath() {} @@ -275,7 +275,7 @@ const returnsString: RawMemberDescription = { }; export class UnknownObjectExpression implements ExpressionEntity { - included: boolean = false; + included = false; deoptimizePath() {} diff --git a/src/ast/variables/NamespaceVariable.ts b/src/ast/variables/NamespaceVariable.ts index b25570560ea..3f18b668c29 100644 --- a/src/ast/variables/NamespaceVariable.ts +++ b/src/ast/variables/NamespaceVariable.ts @@ -11,8 +11,8 @@ export default class NamespaceVariable extends Variable { memberVariables: { [name: string]: Variable } = Object.create(null); module: Module; - private containsExternalNamespace: boolean = false; - private referencedEarly: boolean = false; + private containsExternalNamespace = false; + private referencedEarly = false; private references: Identifier[] = []; constructor(context: AstContext) { diff --git a/src/ast/variables/Variable.ts b/src/ast/variables/Variable.ts index 272e53368b4..c7b2b5d21ec 100644 --- a/src/ast/variables/Variable.ts +++ b/src/ast/variables/Variable.ts @@ -10,15 +10,15 @@ import { LiteralValueOrUnknown, ObjectPath, UNKNOWN_EXPRESSION, UNKNOWN_VALUE } export default class Variable implements ExpressionEntity { exportName: string | null = null; - included: boolean = false; + included = false; isDefault?: boolean; isExternal?: boolean; - isId: boolean = false; + isId = false; isNamespace?: boolean; - isReassigned: boolean = false; + isReassigned = false; module: Module | ExternalModule | null; name: string; - reexported: boolean = false; + reexported = false; renderBaseName: string | null = null; renderName: string | null = null; safeExportName: string | null = null; diff --git a/src/utils/defaultPlugin.ts b/src/utils/defaultPlugin.ts index f447699ff2e..9aafad010ad 100644 --- a/src/utils/defaultPlugin.ts +++ b/src/utils/defaultPlugin.ts @@ -73,7 +73,7 @@ function createResolveId(preserveSymlinks: boolean) { }; } -const getResolveUrl = (path: string, URL: string = 'URL') => `new ${URL}(${path}).href`; +const getResolveUrl = (path: string, URL = 'URL') => `new ${URL}(${path}).href`; const getUrlFromDocument = (chunkId: string) => `(document.currentScript && document.currentScript.src || new URL('${chunkId}', document.baseURI).href)`; diff --git a/src/utils/renderHelpers.ts b/src/utils/renderHelpers.ts index 00ce060c94b..2bd6a63ebf9 100644 --- a/src/utils/renderHelpers.ts +++ b/src/utils/renderHelpers.ts @@ -23,11 +23,7 @@ export interface NodeRenderOptions { export const NO_SEMICOLON: NodeRenderOptions = { isNoStatement: true }; -export function findFirstOccurrenceOutsideComment( - code: string, - searchString: string, - start: number = 0 -) { +export function findFirstOccurrenceOutsideComment(code: string, searchString: string, start = 0) { let searchPos, charCodeAfterSlash; searchPos = code.indexOf(searchString, start); while (true) { @@ -50,7 +46,7 @@ export function findFirstOccurrenceOutsideComment( } } -export function findFirstLineBreakOutsideComment(code: string, start: number = 0) { +export function findFirstLineBreakOutsideComment(code: string, start = 0) { let lineBreakPos, charCodeAfterSlash; lineBreakPos = code.indexOf('\n', start); while (true) { diff --git a/src/utils/timers.ts b/src/utils/timers.ts index 884b62dd484..f974273a7a3 100644 --- a/src/utils/timers.ts +++ b/src/utils/timers.ts @@ -48,7 +48,7 @@ function getPersistedLabel(label: string, level: number) { } } -function timeStartImpl(label: string, level: number = 3) { +function timeStartImpl(label: string, level = 3) { label = getPersistedLabel(label, level); if (!timers.hasOwnProperty(label)) { timers[label] = { @@ -64,7 +64,7 @@ function timeStartImpl(label: string, level: number = 3) { timers[label].startMemory = currentMemory; } -function timeEndImpl(label: string, level: number = 3) { +function timeEndImpl(label: string, level = 3) { label = getPersistedLabel(label, level); if (timers.hasOwnProperty(label)) { const currentMemory = getMemory(); diff --git a/src/watch/index.ts b/src/watch/index.ts index 519ad166a2e..fc809b0a567 100644 --- a/src/watch/index.ts +++ b/src/watch/index.ts @@ -23,9 +23,9 @@ export class Watcher { private buildTimeout: NodeJS.Timer; private invalidatedIds: Set = new Set(); - private rerun: boolean = false; + private rerun = false; private running: boolean; - private succeeded: boolean = false; + private succeeded = false; private tasks: Task[]; constructor(configs: RollupWatchOptions[]) { diff --git a/tslint.json b/tslint.json index a7c778c62f2..c0c1012ee19 100644 --- a/tslint.json +++ b/tslint.json @@ -10,6 +10,7 @@ "alphabetize": true } ], + "no-inferrable-types": true, "no-string-literal": true, "no-unnecessary-type-assertion": true, "object-literal-shorthand": true, From 787a98b7a1f1f9f9ba736b5fd409357965bd83e3 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sun, 5 May 2019 09:05:50 +0200 Subject: [PATCH 04/19] Handle conflicts between resolutions without an opinion about pureness and others --- src/ExternalModule.ts | 8 +++ src/Module.ts | 8 +++ src/ModuleLoader.ts | 11 ++-- src/rollup/types.d.ts | 8 +-- .../samples/context-resolve/_config.js | 18 +++---- .../resolve-id-conflict-impure}/1-pure.js | 0 .../resolve-id-conflict-impure}/2-pure.js | 0 .../resolve-id-conflict-impure}/_config.js | 5 +- .../resolve-id-conflict-impure}/main.js | 2 +- .../sideeffects.js | 0 .../resolve-id-conflict-pure/1-unknown.js | 2 + .../resolve-id-conflict-pure/2-unknown.js | 2 + .../resolve-id-conflict-pure/_config.js | 52 +++++++++++++++++++ .../resolve-id-conflict-pure/main.js | 3 ++ .../resolve-id-conflict-pure/pure.js | 2 + .../resolve-id/_config.js | 1 - .../resolve-id/main.js | 0 test/incremental/index.js | 4 +- 18 files changed, 97 insertions(+), 29 deletions(-) rename test/function/samples/{module-side-effects/resolve-id-conflict => pure-modules/resolve-id-conflict-impure}/1-pure.js (100%) rename test/function/samples/{module-side-effects/resolve-id-conflict => pure-modules/resolve-id-conflict-impure}/2-pure.js (100%) rename test/function/samples/{module-side-effects/resolve-id-conflict => pure-modules/resolve-id-conflict-impure}/_config.js (83%) rename test/function/samples/{module-side-effects/resolve-id-conflict => pure-modules/resolve-id-conflict-impure}/main.js (100%) rename test/function/samples/{module-side-effects/resolve-id-conflict => pure-modules/resolve-id-conflict-impure}/sideeffects.js (100%) create mode 100644 test/function/samples/pure-modules/resolve-id-conflict-pure/1-unknown.js create mode 100644 test/function/samples/pure-modules/resolve-id-conflict-pure/2-unknown.js create mode 100644 test/function/samples/pure-modules/resolve-id-conflict-pure/_config.js create mode 100644 test/function/samples/pure-modules/resolve-id-conflict-pure/main.js create mode 100644 test/function/samples/pure-modules/resolve-id-conflict-pure/pure.js rename test/function/samples/{module-side-effects => pure-modules}/resolve-id/_config.js (99%) rename test/function/samples/{module-side-effects => pure-modules}/resolve-id/main.js (100%) diff --git a/src/ExternalModule.ts b/src/ExternalModule.ts index 88514b5bb66..3e3828d56b6 100644 --- a/src/ExternalModule.ts +++ b/src/ExternalModule.ts @@ -50,6 +50,14 @@ export default class ExternalModule { return declaration; } + markAsPure(pure: boolean | null) { + if (pure === false) { + this.pure = false; + } else if (pure === true && this.pure === null) { + this.pure = true; + } + } + setRenderPath(options: OutputOptions, inputBase: string) { this.renderPath = ''; if (options.paths) { diff --git a/src/Module.ts b/src/Module.ts index eab56b830db..ec0cbc74aaa 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -469,6 +469,14 @@ export default class Module { }); } + markAsPure(pure: boolean | null) { + if (pure === false) { + this.pure = false; + } else if (pure === true && this.pure === null) { + this.pure = true; + } + } + render(options: RenderOptions): MagicString { const magicString = this.magicString.clone(); this.ast.render(magicString, options); diff --git a/src/ModuleLoader.ts b/src/ModuleLoader.ts index d0234606b89..bd9aea305da 100644 --- a/src/ModuleLoader.ts +++ b/src/ModuleLoader.ts @@ -327,12 +327,11 @@ export class ModuleLoader { if (externalModule instanceof ExternalModule === false) { error(errInternalIdCannotBeExternal(source, importer)); } - externalModule.pure = - externalModule.pure === null ? resolvedId.pure : externalModule.pure && resolvedId.pure; + externalModule.markAsPure(resolvedId.pure); return Promise.resolve(externalModule); } else { return this.fetchModule(resolvedId.id, importer).then(module => { - module.pure = module.pure === null ? resolvedId.pure : module.pure && resolvedId.pure; + module.markAsPure(resolvedId.pure); return module; }); } @@ -389,15 +388,15 @@ export class ModuleLoader { ): ResolvedId | null { let id = ''; let external = false; - let pure = false; + let pure = null; if (resolveIdResult) { if (typeof resolveIdResult === 'object') { id = resolveIdResult.id; if (resolveIdResult.external) { external = true; } - if (resolveIdResult.pure) { - pure = true; + if (typeof resolveIdResult.pure === 'boolean') { + pure = resolveIdResult.pure; } } else { id = resolveIdResult; diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index 05aab097e51..b057e098a07 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -141,18 +141,14 @@ export interface PluginContextMeta { export interface ResolvedId { external: boolean; id: string; - pure: boolean; + pure: boolean | null; } export interface ResolvedIdMap { [key: string]: ResolvedId; } -interface PartialResolvedId { - external?: boolean; - id: string; - pure?: boolean | null; -} +type PartialResolvedId = Partial & { id: string }; export type ResolveIdResult = string | false | void | PartialResolvedId; diff --git a/test/function/samples/context-resolve/_config.js b/test/function/samples/context-resolve/_config.js index 38ac7c3be67..db885de7e09 100644 --- a/test/function/samples/context-resolve/_config.js +++ b/test/function/samples/context-resolve/_config.js @@ -4,7 +4,7 @@ const assert = require('assert'); const tests = [ { source: './existing', - expected: { id: path.resolve(__dirname, 'existing.js'), external: false, pure: false } + expected: { id: path.resolve(__dirname, 'existing.js'), external: false, pure: null } }, { source: './missing-relative', @@ -19,7 +19,7 @@ const tests = [ expected: { id: path.resolve(__dirname, 'marked-directly-external-relative'), external: true, - pure: false + pure: null } }, { @@ -27,32 +27,32 @@ const tests = [ expected: { id: path.resolve(__dirname, 'marked-external-relative'), external: true, - pure: false + pure: null } }, { source: 'marked-external-absolute', - expected: { id: 'marked-external-absolute', external: true, pure: false } + expected: { id: 'marked-external-absolute', external: true, pure: null } }, { source: 'resolved-name', - expected: { id: 'resolved:resolved-name', external: false, pure: false } + expected: { id: 'resolved:resolved-name', external: false, pure: null } }, { source: 'resolved-false', - expected: { id: 'resolved-false', external: true, pure: false } + expected: { id: 'resolved-false', external: true, pure: null } }, { source: 'resolved-object', - expected: { id: 'resolved:resolved-object', external: false, pure: false } + expected: { id: 'resolved:resolved-object', external: false, pure: null } }, { source: 'resolved-object-non-external', - expected: { id: 'resolved:resolved-object-non-external', external: false, pure: false } + expected: { id: 'resolved:resolved-object-non-external', external: false, pure: null } }, { source: 'resolved-object-external', - expected: { id: 'resolved:resolved-object-external', external: true, pure: false } + expected: { id: 'resolved:resolved-object-external', external: true, pure: null } } ]; diff --git a/test/function/samples/module-side-effects/resolve-id-conflict/1-pure.js b/test/function/samples/pure-modules/resolve-id-conflict-impure/1-pure.js similarity index 100% rename from test/function/samples/module-side-effects/resolve-id-conflict/1-pure.js rename to test/function/samples/pure-modules/resolve-id-conflict-impure/1-pure.js diff --git a/test/function/samples/module-side-effects/resolve-id-conflict/2-pure.js b/test/function/samples/pure-modules/resolve-id-conflict-impure/2-pure.js similarity index 100% rename from test/function/samples/module-side-effects/resolve-id-conflict/2-pure.js rename to test/function/samples/pure-modules/resolve-id-conflict-impure/2-pure.js diff --git a/test/function/samples/module-side-effects/resolve-id-conflict/_config.js b/test/function/samples/pure-modules/resolve-id-conflict-impure/_config.js similarity index 83% rename from test/function/samples/module-side-effects/resolve-id-conflict/_config.js rename to test/function/samples/pure-modules/resolve-id-conflict-impure/_config.js index c46850fde20..a088de18b82 100644 --- a/test/function/samples/module-side-effects/resolve-id-conflict/_config.js +++ b/test/function/samples/pure-modules/resolve-id-conflict-impure/_config.js @@ -1,11 +1,8 @@ const assert = require('assert'); const sideEffects = []; -// TODO Lukas interaction with pureExternalModules, use as default? -// TODO Lukas resolvers without an opinion should not override pureness module.exports = { - solo: true, - description: 'handles conflicts between different resolveId results', + description: 'handles conflicts between different resolveId results when one result is impure', context: { sideEffects, require(id) { diff --git a/test/function/samples/module-side-effects/resolve-id-conflict/main.js b/test/function/samples/pure-modules/resolve-id-conflict-impure/main.js similarity index 100% rename from test/function/samples/module-side-effects/resolve-id-conflict/main.js rename to test/function/samples/pure-modules/resolve-id-conflict-impure/main.js index 3bcc2e3b5bc..9174e3356c9 100644 --- a/test/function/samples/module-side-effects/resolve-id-conflict/main.js +++ b/test/function/samples/pure-modules/resolve-id-conflict-impure/main.js @@ -1,3 +1,3 @@ import './1-pure.js'; -import './sideeffects.js'; import './2-pure.js'; +import './sideeffects.js'; diff --git a/test/function/samples/module-side-effects/resolve-id-conflict/sideeffects.js b/test/function/samples/pure-modules/resolve-id-conflict-impure/sideeffects.js similarity index 100% rename from test/function/samples/module-side-effects/resolve-id-conflict/sideeffects.js rename to test/function/samples/pure-modules/resolve-id-conflict-impure/sideeffects.js diff --git a/test/function/samples/pure-modules/resolve-id-conflict-pure/1-unknown.js b/test/function/samples/pure-modules/resolve-id-conflict-pure/1-unknown.js new file mode 100644 index 00000000000..e648bc4dae6 --- /dev/null +++ b/test/function/samples/pure-modules/resolve-id-conflict-pure/1-unknown.js @@ -0,0 +1,2 @@ +import 'external'; +import 'internal'; diff --git a/test/function/samples/pure-modules/resolve-id-conflict-pure/2-unknown.js b/test/function/samples/pure-modules/resolve-id-conflict-pure/2-unknown.js new file mode 100644 index 00000000000..e648bc4dae6 --- /dev/null +++ b/test/function/samples/pure-modules/resolve-id-conflict-pure/2-unknown.js @@ -0,0 +1,2 @@ +import 'external'; +import 'internal'; diff --git a/test/function/samples/pure-modules/resolve-id-conflict-pure/_config.js b/test/function/samples/pure-modules/resolve-id-conflict-pure/_config.js new file mode 100644 index 00000000000..fc21fcf0bfa --- /dev/null +++ b/test/function/samples/pure-modules/resolve-id-conflict-pure/_config.js @@ -0,0 +1,52 @@ +const assert = require('assert'); +const sideEffects = []; + +// TODO Lukas interaction with pureExternalModules, use as default? +// TODO Lukas handle other hooks +module.exports = { + description: + 'handles conflicts between different resolveId results when all results are unknown or pure', + context: { + sideEffects, + require(id) { + sideEffects.push(id); + return { value: id }; + } + }, + exports() { + assert.deepStrictEqual(sideEffects, []); + }, + options: { + plugins: { + name: 'test-plugin', + resolveId(id, importer) { + const pure = importer && importer.endsWith('pure.js') ? true : null; + switch (id) { + case 'internal': + return { + external: false, + id, + pure + }; + case 'external': + return { + external: true, + id, + pure + }; + } + }, + load(id) { + if (id === 'internal') { + return `export const value = '${id}'; sideEffects.push(value);`; + } + } + } + }, + warnings: [ + { + code: 'EMPTY_BUNDLE', + message: 'Generated an empty bundle' + } + ] +}; diff --git a/test/function/samples/pure-modules/resolve-id-conflict-pure/main.js b/test/function/samples/pure-modules/resolve-id-conflict-pure/main.js new file mode 100644 index 00000000000..179ba71f5c3 --- /dev/null +++ b/test/function/samples/pure-modules/resolve-id-conflict-pure/main.js @@ -0,0 +1,3 @@ +import './1-unknown.js'; +import './2-unknown.js'; +import './pure.js'; diff --git a/test/function/samples/pure-modules/resolve-id-conflict-pure/pure.js b/test/function/samples/pure-modules/resolve-id-conflict-pure/pure.js new file mode 100644 index 00000000000..e648bc4dae6 --- /dev/null +++ b/test/function/samples/pure-modules/resolve-id-conflict-pure/pure.js @@ -0,0 +1,2 @@ +import 'external'; +import 'internal'; diff --git a/test/function/samples/module-side-effects/resolve-id/_config.js b/test/function/samples/pure-modules/resolve-id/_config.js similarity index 99% rename from test/function/samples/module-side-effects/resolve-id/_config.js rename to test/function/samples/pure-modules/resolve-id/_config.js index 0d80a363a94..085a3225bf7 100644 --- a/test/function/samples/module-side-effects/resolve-id/_config.js +++ b/test/function/samples/pure-modules/resolve-id/_config.js @@ -2,7 +2,6 @@ const assert = require('assert'); const sideEffects = []; module.exports = { - solo: true, description: 'does not include modules without used exports if moduleSideEffect is false', context: { sideEffects, diff --git a/test/function/samples/module-side-effects/resolve-id/main.js b/test/function/samples/pure-modules/resolve-id/main.js similarity index 100% rename from test/function/samples/module-side-effects/resolve-id/main.js rename to test/function/samples/pure-modules/resolve-id/main.js diff --git a/test/incremental/index.js b/test/incremental/index.js index 0afcbabeec7..ba37ec88eba 100644 --- a/test/incremental/index.js +++ b/test/incremental/index.js @@ -240,8 +240,8 @@ describe('incremental', () => { assert.equal(bundle.cache.modules[1].id, 'entry'); assert.deepEqual(bundle.cache.modules[1].resolvedIds, { - foo: { id: 'foo', external: false, pure: false }, - external: { id: 'external', external: true, pure: false } + foo: { id: 'foo', external: false, pure: null }, + external: { id: 'external', external: true, pure: null } }); }); }); From 0286cfbcf4db883914c3b974c943b0ba850baadd Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sun, 5 May 2019 12:21:19 +0200 Subject: [PATCH 05/19] Add purity to getModuleInfo, change logic to only use the first resolution to determine purity --- src/ExternalModule.ts | 14 +---- src/Module.ts | 13 +--- src/ModuleLoader.ts | 22 ++++--- src/rollup/types.d.ts | 9 ++- src/utils/pluginDriver.ts | 3 +- .../samples/context-resolve/_config.js | 18 +++--- .../plugin-module-information/_config.js | 12 ++-- .../resolve-id-conflict-impure/1-pure.js | 2 - .../resolve-id-conflict-impure/2-pure.js | 2 - .../resolve-id-conflict-impure/_config.js | 43 -------------- .../resolve-id-conflict-impure/main.js | 3 - .../resolve-id-conflict-impure/sideeffects.js | 2 - .../resolve-id-conflict-pure/1-unknown.js | 2 - .../resolve-id-conflict-pure/2-unknown.js | 2 - .../resolve-id-conflict-pure/_config.js | 52 ---------------- .../resolve-id-conflict-pure/main.js | 3 - .../resolve-id-conflict-pure/pure.js | 2 - .../pure-modules/resolve-id/_config.js | 59 ++++++++++++++++--- .../samples/pure-modules/resolve-id/main.js | 35 +++++++---- test/incremental/index.js | 4 +- 20 files changed, 119 insertions(+), 183 deletions(-) delete mode 100644 test/function/samples/pure-modules/resolve-id-conflict-impure/1-pure.js delete mode 100644 test/function/samples/pure-modules/resolve-id-conflict-impure/2-pure.js delete mode 100644 test/function/samples/pure-modules/resolve-id-conflict-impure/_config.js delete mode 100644 test/function/samples/pure-modules/resolve-id-conflict-impure/main.js delete mode 100644 test/function/samples/pure-modules/resolve-id-conflict-impure/sideeffects.js delete mode 100644 test/function/samples/pure-modules/resolve-id-conflict-pure/1-unknown.js delete mode 100644 test/function/samples/pure-modules/resolve-id-conflict-pure/2-unknown.js delete mode 100644 test/function/samples/pure-modules/resolve-id-conflict-pure/_config.js delete mode 100644 test/function/samples/pure-modules/resolve-id-conflict-pure/main.js delete mode 100644 test/function/samples/pure-modules/resolve-id-conflict-pure/pure.js diff --git a/src/ExternalModule.ts b/src/ExternalModule.ts index 3e3828d56b6..c787c050348 100644 --- a/src/ExternalModule.ts +++ b/src/ExternalModule.ts @@ -12,11 +12,10 @@ export default class ExternalModule { exportsNames = false; exportsNamespace = false; id: string; - isEntryPoint = false; isExternal = true; mostCommonSuggestion = 0; nameSuggestions: { [name: string]: number }; - pure: boolean | null = null; + pure: boolean; reexported = false; renderPath: string = undefined; renormalizeRenderPath = false; @@ -25,10 +24,11 @@ export default class ExternalModule { private graph: Graph; - constructor({ graph, id }: { graph: Graph; id: string }) { + constructor(graph: Graph, id: string, pure: boolean) { this.graph = graph; this.id = id; this.execIndex = Infinity; + this.pure = pure; const parts = id.split(/[\\/]/); this.variableName = makeLegal(parts.pop()); @@ -50,14 +50,6 @@ export default class ExternalModule { return declaration; } - markAsPure(pure: boolean | null) { - if (pure === false) { - this.pure = false; - } else if (pure === true && this.pure === null) { - this.pure = true; - } - } - setRenderPath(options: OutputOptions, inputBase: string) { this.renderPath = ''; if (options.paths) { diff --git a/src/Module.ts b/src/Module.ts index ec0cbc74aaa..347714e7147 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -194,7 +194,7 @@ export default class Module { manualChunkAlias: string = null; originalCode: string; originalSourcemap: RawSourceMap | void; - pure: boolean | null = null; + pure: boolean; reexports: { [name: string]: ReexportDescription } = Object.create(null); resolvedIds: ResolvedIdMap; scope: ModuleScope; @@ -212,11 +212,12 @@ export default class Module { private namespaceVariable: NamespaceVariable = undefined; private transformDependencies: string[]; - constructor(graph: Graph, id: string) { + constructor(graph: Graph, id: string, pure: boolean) { this.id = id; this.graph = graph; this.excludeFromSourcemap = /\0/.test(id); this.context = graph.getModuleContext(id); + this.pure = pure; } basename() { @@ -469,14 +470,6 @@ export default class Module { }); } - markAsPure(pure: boolean | null) { - if (pure === false) { - this.pure = false; - } else if (pure === true && this.pure === null) { - this.pure = true; - } - } - render(options: RenderOptions): MagicString { const magicString = this.magicString.clone(); this.ast.render(magicString, options); diff --git a/src/ModuleLoader.ts b/src/ModuleLoader.ts index bd9aea305da..8421aea5a5b 100644 --- a/src/ModuleLoader.ts +++ b/src/ModuleLoader.ts @@ -224,14 +224,14 @@ export class ModuleLoader { ).then(() => fetchDynamicImportsPromise); } - private fetchModule(id: string, importer: string): Promise { + private fetchModule(id: string, importer: string, pure: boolean): Promise { const existingModule = this.modulesById.get(id); if (existingModule) { if (existingModule.isExternal) throw new Error(`Cannot fetch external module ${id}`); return Promise.resolve(existingModule); } - const module: Module = new Module(this.graph, id); + const module: Module = new Module(this.graph, id, pure); this.modulesById.set(id, module); const manualChunkAlias = this.getManualChunk(id); if (typeof manualChunkAlias === 'string') { @@ -319,7 +319,7 @@ export class ModuleLoader { if (!this.modulesById.has(resolvedId.id)) { this.modulesById.set( resolvedId.id, - new ExternalModule({ graph: this.graph, id: resolvedId.id }) + new ExternalModule(this.graph, resolvedId.id, resolvedId.pure) ); } @@ -327,13 +327,9 @@ export class ModuleLoader { if (externalModule instanceof ExternalModule === false) { error(errInternalIdCannotBeExternal(source, importer)); } - externalModule.markAsPure(resolvedId.pure); return Promise.resolve(externalModule); } else { - return this.fetchModule(resolvedId.id, importer).then(module => { - module.markAsPure(resolvedId.pure); - return module; - }); + return this.fetchModule(resolvedId.id, importer, resolvedId.pure); } } @@ -366,9 +362,11 @@ export class ModuleLoader { resolveIdResult && typeof resolveIdResult === 'object' ? resolveIdResult.id : resolveIdResult; + const pure = + (resolveIdResult && typeof resolveIdResult === 'object' && resolveIdResult.pure) || false; if (typeof id === 'string') { - return this.fetchModule(id, undefined).then(module => { + return this.fetchModule(id, undefined, pure).then(module => { if (alias !== null) { if (module.chunkAlias !== null && module.chunkAlias !== alias) { error(errCannotAssignModuleToChunk(module.id, alias, module.chunkAlias)); @@ -388,15 +386,15 @@ export class ModuleLoader { ): ResolvedId | null { let id = ''; let external = false; - let pure = null; + let pure = false; if (resolveIdResult) { if (typeof resolveIdResult === 'object') { id = resolveIdResult.id; if (resolveIdResult.external) { external = true; } - if (typeof resolveIdResult.pure === 'boolean') { - pure = resolveIdResult.pure; + if (resolveIdResult.pure) { + pure = true; } } else { id = resolveIdResult; diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index b057e098a07..7e39741cf67 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -120,6 +120,7 @@ export interface PluginContext extends MinimalPluginContext { id: string; importedIds: string[]; isExternal: boolean; + isPure: boolean; }; /** @deprecated */ isExternal: IsExternal; @@ -141,14 +142,18 @@ export interface PluginContextMeta { export interface ResolvedId { external: boolean; id: string; - pure: boolean | null; + pure: boolean; } export interface ResolvedIdMap { [key: string]: ResolvedId; } -type PartialResolvedId = Partial & { id: string }; +interface PartialResolvedId { + external?: boolean; + id: string; + pure?: boolean | null; +} export type ResolveIdResult = string | false | void | PartialResolvedId; diff --git a/src/utils/pluginDriver.ts b/src/utils/pluginDriver.ts index cea0d193bbe..204de313634 100644 --- a/src/utils/pluginDriver.ts +++ b/src/utils/pluginDriver.ts @@ -178,7 +178,8 @@ export function createPluginDriver( importedIds: foundModule.isExternal ? [] : (foundModule as Module).sources.map(id => (foundModule as Module).resolvedIds[id].id), - isExternal: !!foundModule.isExternal + isExternal: !!foundModule.isExternal, + isPure: foundModule.pure }; }, meta: { diff --git a/test/function/samples/context-resolve/_config.js b/test/function/samples/context-resolve/_config.js index db885de7e09..38ac7c3be67 100644 --- a/test/function/samples/context-resolve/_config.js +++ b/test/function/samples/context-resolve/_config.js @@ -4,7 +4,7 @@ const assert = require('assert'); const tests = [ { source: './existing', - expected: { id: path.resolve(__dirname, 'existing.js'), external: false, pure: null } + expected: { id: path.resolve(__dirname, 'existing.js'), external: false, pure: false } }, { source: './missing-relative', @@ -19,7 +19,7 @@ const tests = [ expected: { id: path.resolve(__dirname, 'marked-directly-external-relative'), external: true, - pure: null + pure: false } }, { @@ -27,32 +27,32 @@ const tests = [ expected: { id: path.resolve(__dirname, 'marked-external-relative'), external: true, - pure: null + pure: false } }, { source: 'marked-external-absolute', - expected: { id: 'marked-external-absolute', external: true, pure: null } + expected: { id: 'marked-external-absolute', external: true, pure: false } }, { source: 'resolved-name', - expected: { id: 'resolved:resolved-name', external: false, pure: null } + expected: { id: 'resolved:resolved-name', external: false, pure: false } }, { source: 'resolved-false', - expected: { id: 'resolved-false', external: true, pure: null } + expected: { id: 'resolved-false', external: true, pure: false } }, { source: 'resolved-object', - expected: { id: 'resolved:resolved-object', external: false, pure: null } + expected: { id: 'resolved:resolved-object', external: false, pure: false } }, { source: 'resolved-object-non-external', - expected: { id: 'resolved:resolved-object-non-external', external: false, pure: null } + expected: { id: 'resolved:resolved-object-non-external', external: false, pure: false } }, { source: 'resolved-object-external', - expected: { id: 'resolved:resolved-object-external', external: true, pure: null } + expected: { id: 'resolved:resolved-object-external', external: true, pure: false } } ]; diff --git a/test/function/samples/plugin-module-information/_config.js b/test/function/samples/plugin-module-information/_config.js index 10b7d66b691..a8e2188ee89 100644 --- a/test/function/samples/plugin-module-information/_config.js +++ b/test/function/samples/plugin-module-information/_config.js @@ -20,22 +20,26 @@ module.exports = { assert.deepEqual(this.getModuleInfo(ID_MAIN), { id: ID_MAIN, importedIds: [ID_FOO, ID_NESTED], - isExternal: false + isExternal: false, + isPure: false }); assert.deepEqual(this.getModuleInfo(ID_FOO), { id: ID_FOO, importedIds: [ID_PATH], - isExternal: false + isExternal: false, + isPure: false }); assert.deepEqual(this.getModuleInfo(ID_NESTED), { id: ID_NESTED, importedIds: [ID_FOO], - isExternal: false + isExternal: false, + isPure: false }); assert.deepEqual(this.getModuleInfo(ID_PATH), { id: ID_PATH, importedIds: [], - isExternal: true + isExternal: true, + isPure: false }); } } diff --git a/test/function/samples/pure-modules/resolve-id-conflict-impure/1-pure.js b/test/function/samples/pure-modules/resolve-id-conflict-impure/1-pure.js deleted file mode 100644 index e648bc4dae6..00000000000 --- a/test/function/samples/pure-modules/resolve-id-conflict-impure/1-pure.js +++ /dev/null @@ -1,2 +0,0 @@ -import 'external'; -import 'internal'; diff --git a/test/function/samples/pure-modules/resolve-id-conflict-impure/2-pure.js b/test/function/samples/pure-modules/resolve-id-conflict-impure/2-pure.js deleted file mode 100644 index e648bc4dae6..00000000000 --- a/test/function/samples/pure-modules/resolve-id-conflict-impure/2-pure.js +++ /dev/null @@ -1,2 +0,0 @@ -import 'external'; -import 'internal'; diff --git a/test/function/samples/pure-modules/resolve-id-conflict-impure/_config.js b/test/function/samples/pure-modules/resolve-id-conflict-impure/_config.js deleted file mode 100644 index a088de18b82..00000000000 --- a/test/function/samples/pure-modules/resolve-id-conflict-impure/_config.js +++ /dev/null @@ -1,43 +0,0 @@ -const assert = require('assert'); -const sideEffects = []; - -module.exports = { - description: 'handles conflicts between different resolveId results when one result is impure', - context: { - sideEffects, - require(id) { - sideEffects.push(id); - return { value: id }; - } - }, - exports() { - assert.deepStrictEqual(sideEffects, ['external', 'internal']); - }, - options: { - plugins: { - name: 'test-plugin', - resolveId(id, importer) { - const pure = importer && importer.endsWith('pure.js'); - switch (id) { - case 'internal': - return { - external: false, - id, - pure - }; - case 'external': - return { - external: true, - id, - pure - }; - } - }, - load(id) { - if (id === 'internal') { - return `export const value = '${id}'; sideEffects.push(value);`; - } - } - } - } -}; diff --git a/test/function/samples/pure-modules/resolve-id-conflict-impure/main.js b/test/function/samples/pure-modules/resolve-id-conflict-impure/main.js deleted file mode 100644 index 9174e3356c9..00000000000 --- a/test/function/samples/pure-modules/resolve-id-conflict-impure/main.js +++ /dev/null @@ -1,3 +0,0 @@ -import './1-pure.js'; -import './2-pure.js'; -import './sideeffects.js'; diff --git a/test/function/samples/pure-modules/resolve-id-conflict-impure/sideeffects.js b/test/function/samples/pure-modules/resolve-id-conflict-impure/sideeffects.js deleted file mode 100644 index e648bc4dae6..00000000000 --- a/test/function/samples/pure-modules/resolve-id-conflict-impure/sideeffects.js +++ /dev/null @@ -1,2 +0,0 @@ -import 'external'; -import 'internal'; diff --git a/test/function/samples/pure-modules/resolve-id-conflict-pure/1-unknown.js b/test/function/samples/pure-modules/resolve-id-conflict-pure/1-unknown.js deleted file mode 100644 index e648bc4dae6..00000000000 --- a/test/function/samples/pure-modules/resolve-id-conflict-pure/1-unknown.js +++ /dev/null @@ -1,2 +0,0 @@ -import 'external'; -import 'internal'; diff --git a/test/function/samples/pure-modules/resolve-id-conflict-pure/2-unknown.js b/test/function/samples/pure-modules/resolve-id-conflict-pure/2-unknown.js deleted file mode 100644 index e648bc4dae6..00000000000 --- a/test/function/samples/pure-modules/resolve-id-conflict-pure/2-unknown.js +++ /dev/null @@ -1,2 +0,0 @@ -import 'external'; -import 'internal'; diff --git a/test/function/samples/pure-modules/resolve-id-conflict-pure/_config.js b/test/function/samples/pure-modules/resolve-id-conflict-pure/_config.js deleted file mode 100644 index fc21fcf0bfa..00000000000 --- a/test/function/samples/pure-modules/resolve-id-conflict-pure/_config.js +++ /dev/null @@ -1,52 +0,0 @@ -const assert = require('assert'); -const sideEffects = []; - -// TODO Lukas interaction with pureExternalModules, use as default? -// TODO Lukas handle other hooks -module.exports = { - description: - 'handles conflicts between different resolveId results when all results are unknown or pure', - context: { - sideEffects, - require(id) { - sideEffects.push(id); - return { value: id }; - } - }, - exports() { - assert.deepStrictEqual(sideEffects, []); - }, - options: { - plugins: { - name: 'test-plugin', - resolveId(id, importer) { - const pure = importer && importer.endsWith('pure.js') ? true : null; - switch (id) { - case 'internal': - return { - external: false, - id, - pure - }; - case 'external': - return { - external: true, - id, - pure - }; - } - }, - load(id) { - if (id === 'internal') { - return `export const value = '${id}'; sideEffects.push(value);`; - } - } - } - }, - warnings: [ - { - code: 'EMPTY_BUNDLE', - message: 'Generated an empty bundle' - } - ] -}; diff --git a/test/function/samples/pure-modules/resolve-id-conflict-pure/main.js b/test/function/samples/pure-modules/resolve-id-conflict-pure/main.js deleted file mode 100644 index 179ba71f5c3..00000000000 --- a/test/function/samples/pure-modules/resolve-id-conflict-pure/main.js +++ /dev/null @@ -1,3 +0,0 @@ -import './1-unknown.js'; -import './2-unknown.js'; -import './pure.js'; diff --git a/test/function/samples/pure-modules/resolve-id-conflict-pure/pure.js b/test/function/samples/pure-modules/resolve-id-conflict-pure/pure.js deleted file mode 100644 index e648bc4dae6..00000000000 --- a/test/function/samples/pure-modules/resolve-id-conflict-pure/pure.js +++ /dev/null @@ -1,2 +0,0 @@ -import 'external'; -import 'internal'; diff --git a/test/function/samples/pure-modules/resolve-id/_config.js b/test/function/samples/pure-modules/resolve-id/_config.js index 085a3225bf7..3de71c7e584 100644 --- a/test/function/samples/pure-modules/resolve-id/_config.js +++ b/test/function/samples/pure-modules/resolve-id/_config.js @@ -1,6 +1,8 @@ const assert = require('assert'); const sideEffects = []; +// TODO Lukas interaction with pureExternalModules, use as default? +// TODO Lukas handle other hooks module.exports = { description: 'does not include modules without used exports if moduleSideEffect is false', context: { @@ -13,13 +15,19 @@ module.exports = { exports() { assert.deepStrictEqual(sideEffects, [ 'pure-false-external-true', - 'pure-true-external-true-used-import', 'pure-false-external-true-unused-import', 'pure-false-external-true-used-import', + 'pure-null-external-true', + 'pure-null-external-true-unused-import', + 'pure-null-external-true-used-import', + 'pure-true-external-true-used-import', 'pure-false-external-false', - 'pure-true-external-false-used-import', 'pure-false-external-false-unused-import', - 'pure-false-external-false-used-import' + 'pure-false-external-false-used-import', + 'pure-null-external-false', + 'pure-null-external-false-unused-import', + 'pure-null-external-false-used-import', + 'pure-true-external-false-used-import' ]); }, options: { @@ -37,8 +45,38 @@ module.exports = { }, load(id) { if (id[0] !== '/') { + const pure = JSON.parse(id.split('-')[1]); + assert.strictEqual(this.getModuleInfo(id).isPure, pure || false); return `export const value = '${id}'; sideEffects.push(value);`; } + }, + buildEnd() { + assert.deepStrictEqual( + Array.from(this.moduleIds) + .filter(id => id[0] !== '/') + .sort() + .map(id => ({ id, isPure: this.getModuleInfo(id).isPure })), + [ + { id: 'pure-false-external-false', isPure: false }, + { id: 'pure-false-external-false-unused-import', isPure: false }, + { id: 'pure-false-external-false-used-import', isPure: false }, + { id: 'pure-false-external-true', isPure: false }, + { id: 'pure-false-external-true-unused-import', isPure: false }, + { id: 'pure-false-external-true-used-import', isPure: false }, + { id: 'pure-null-external-false', isPure: false }, + { id: 'pure-null-external-false-unused-import', isPure: false }, + { id: 'pure-null-external-false-used-import', isPure: false }, + { id: 'pure-null-external-true', isPure: false }, + { id: 'pure-null-external-true-unused-import', isPure: false }, + { id: 'pure-null-external-true-used-import', isPure: false }, + { id: 'pure-true-external-false', isPure: true }, + { id: 'pure-true-external-false-unused-import', isPure: true }, + { id: 'pure-true-external-false-used-import', isPure: true }, + { id: 'pure-true-external-true', isPure: true }, + { id: 'pure-true-external-true-unused-import', isPure: true }, + { id: 'pure-true-external-true-used-import', isPure: true } + ] + ); } } }, @@ -46,16 +84,23 @@ module.exports = { { code: 'UNUSED_EXTERNAL_IMPORT', message: - "'value' is imported from external module 'pure-true-external-true-unused-import' but never used", + "'value' is imported from external module 'pure-false-external-true-unused-import' but never used", names: ['value'], - source: 'pure-true-external-true-unused-import' + source: 'pure-false-external-true-unused-import' }, { code: 'UNUSED_EXTERNAL_IMPORT', message: - "'value' is imported from external module 'pure-false-external-true-unused-import' but never used", + "'value' is imported from external module 'pure-null-external-true-unused-import' but never used", names: ['value'], - source: 'pure-false-external-true-unused-import' + source: 'pure-null-external-true-unused-import' + }, + { + code: 'UNUSED_EXTERNAL_IMPORT', + message: + "'value' is imported from external module 'pure-true-external-true-unused-import' but never used", + names: ['value'], + source: 'pure-true-external-true-unused-import' } ] }; diff --git a/test/function/samples/pure-modules/resolve-id/main.js b/test/function/samples/pure-modules/resolve-id/main.js index 2d1cd227643..31303129063 100644 --- a/test/function/samples/pure-modules/resolve-id/main.js +++ b/test/function/samples/pure-modules/resolve-id/main.js @@ -1,14 +1,25 @@ +import 'pure-false-external-false'; +import { value as unusedValue1 } from 'pure-false-external-false-unused-import'; +import { value as usedValue1 } from 'pure-false-external-false-used-import'; + +import 'pure-null-external-false'; +import { value as unusedValue2 } from 'pure-null-external-false-unused-import'; +import { value as usedValue2 } from 'pure-null-external-false-used-import'; + import 'pure-true-external-false'; +import { value as unusedValue3 } from 'pure-true-external-false-unused-import'; +import { value as usedValue3 } from 'pure-true-external-false-used-import'; + +import 'pure-false-external-true'; +import { value as unusedValue4 } from 'pure-false-external-true-unused-import'; +import { value as usedValue4 } from 'pure-false-external-true-used-import'; + +import 'pure-null-external-true'; +import { value as unusedValue5 } from 'pure-null-external-true-unused-import'; +import { value as usedValue5 } from 'pure-null-external-true-used-import'; + import 'pure-true-external-true'; -import 'pure-false-external-false';// included -import 'pure-false-external-true';// included -import {value as unusedValue1} from 'pure-true-external-false-unused-import'; -import {value as usedValue1} from 'pure-true-external-false-used-import';// included -import {value as unusedValue2} from 'pure-true-external-true-unused-import'; -import {value as usedValue2} from 'pure-true-external-true-used-import';// included -import {value as unusedValue3} from 'pure-false-external-false-unused-import';// included -import {value as usedValue3} from 'pure-false-external-false-used-import';// included -import {value as unusedValue4} from 'pure-false-external-true-unused-import';// included -import {value as usedValue4} from 'pure-false-external-true-used-import';// included - -export const values = [usedValue1, usedValue2, usedValue3, usedValue4]; +import { value as unusedValue6 } from 'pure-true-external-true-unused-import'; +import { value as usedValue6 } from 'pure-true-external-true-used-import'; + +export const values = [usedValue1, usedValue2, usedValue3, usedValue4, usedValue5, usedValue6]; diff --git a/test/incremental/index.js b/test/incremental/index.js index ba37ec88eba..0afcbabeec7 100644 --- a/test/incremental/index.js +++ b/test/incremental/index.js @@ -240,8 +240,8 @@ describe('incremental', () => { assert.equal(bundle.cache.modules[1].id, 'entry'); assert.deepEqual(bundle.cache.modules[1].resolvedIds, { - foo: { id: 'foo', external: false, pure: null }, - external: { id: 'external', external: true, pure: null } + foo: { id: 'foo', external: false, pure: false }, + external: { id: 'external', external: true, pure: false } }); }); }); From 36ee1b63b9db2e1aa870326c767c2cb410099a1f Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sun, 5 May 2019 14:03:05 +0200 Subject: [PATCH 06/19] Implement pureInternalModules and use as default --- src/Chunk.ts | 3 +- src/Graph.ts | 27 +- src/ModuleLoader.ts | 48 ++- src/rollup/types.d.ts | 11 +- src/utils/mergeOptions.ts | 1 - .../pure-modules/resolve-id/_config.js | 291 ++++++++++++++---- .../samples/pure-modules/resolve-id/main.js | 135 ++++++-- 7 files changed, 407 insertions(+), 109 deletions(-) diff --git a/src/Chunk.ts b/src/Chunk.ts index 38e004ef1e2..2e44c5ee294 100644 --- a/src/Chunk.ts +++ b/src/Chunk.ts @@ -753,8 +753,7 @@ export default class Chunk { if (depModule instanceof Module) { dependency = depModule.chunk; } else { - // TODO Lukas can we use a different flag that is only set depending on pureness? - if (!depModule.used && (depModule.pure || this.graph.isPureExternalModule(depModule.id))) { + if (!depModule.used && depModule.pure) { continue; } dependency = depModule; diff --git a/src/Graph.ts b/src/Graph.ts index 1e237dbf407..265dd0b438e 100644 --- a/src/Graph.ts +++ b/src/Graph.ts @@ -67,7 +67,6 @@ export default class Graph { curChunkIndex = 0; deoptimizationTracker: EntityPathTracker; getModuleContext: (id: string) => string; - isPureExternalModule: (id: string) => boolean; moduleById = new Map(); moduleLoader: ModuleLoader; needsTreeshakingPass = false; @@ -116,21 +115,15 @@ export default class Graph { annotations: (options.treeshake).annotations !== false, propertyReadSideEffects: (options.treeshake).propertyReadSideEffects !== false, - pureExternalModules: (options.treeshake).pureExternalModules + pureExternalModules: (options.treeshake).pureExternalModules, + pureInternalModules: (options.treeshake).pureInternalModules } - : { 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, + propertyReadSideEffects: true, + pureExternalModules: false, + pureInternalModules: 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.pureExternalModules : false, + this.treeshake ? this.treeshakingOptions.pureInternalModules : false ); } diff --git a/src/ModuleLoader.ts b/src/ModuleLoader.ts index 8421aea5a5b..f4bbd3173a4 100644 --- a/src/ModuleLoader.ts +++ b/src/ModuleLoader.ts @@ -6,7 +6,9 @@ import { ExternalOption, GetManualChunk, IsExternal, + IsPureModule, ModuleJSON, + PureModulesOption, ResolvedId, ResolveIdResult, SourceDescription @@ -44,6 +46,21 @@ function normalizeRelativeExternalId(importer: string, source: string) { return isRelative(source) ? resolve(importer, '..', source) : source; } +function getIdMatcher>( + option: boolean | string[] | ((id: string, ...args: T) => boolean | void) +): (id: string, ...args: T) => boolean { + if (option === true) { + return () => true; + } else if (typeof option === 'function') { + return (id, ...args) => (!id.startsWith('\0') && option(id, ...args)) || false; + } else if (option) { + const ids = new Set(Array.isArray(option) ? option : option ? [option] : []); + return id => ids.has(id); + } else { + return () => false; + } +} + export class ModuleLoader { readonly isExternal: IsExternal; private readonly entriesByReferenceId = new Map< @@ -53,6 +70,8 @@ export class ModuleLoader { private readonly entryModules: Module[] = []; private readonly getManualChunk: GetManualChunk; private readonly graph: Graph; + private readonly isPureExternalModule: IsPureModule; + private readonly isPureInternalModule: IsPureModule; private latestLoadModulesPromise: Promise = Promise.resolve(); private readonly manualChunkModules: Record = {}; private readonly modulesById: Map; @@ -63,18 +82,16 @@ export class ModuleLoader { modulesById: Map, pluginDriver: PluginDriver, external: ExternalOption, - getManualChunk: GetManualChunk | null + getManualChunk: GetManualChunk | null, + pureExternalModules: PureModulesOption, + pureInternalModules: PureModulesOption ) { this.graph = graph; this.modulesById = modulesById; this.pluginDriver = pluginDriver; - if (typeof external === 'function') { - this.isExternal = (id, parentId, isResolved) => - !id.startsWith('\0') && external(id, parentId, isResolved); - } else { - const ids = new Set(Array.isArray(external) ? external : external ? [external] : []); - this.isExternal = id => ids.has(id); - } + this.isExternal = getIdMatcher(external); + this.isPureExternalModule = getIdMatcher(pureExternalModules); + this.isPureInternalModule = getIdMatcher(pureInternalModules); this.getManualChunk = typeof getManualChunk === 'function' ? getManualChunk : () => null; } @@ -386,15 +403,15 @@ export class ModuleLoader { ): ResolvedId | null { let id = ''; let external = false; - let pure = false; + let pure = null; if (resolveIdResult) { if (typeof resolveIdResult === 'object') { id = resolveIdResult.id; if (resolveIdResult.external) { external = true; } - if (resolveIdResult.pure) { - pure = true; + if (typeof resolveIdResult.pure === 'boolean') { + pure = resolveIdResult.pure; } } else { id = resolveIdResult; @@ -412,7 +429,14 @@ export class ModuleLoader { } external = true; } - return { id, external, pure }; + return { + external, + id, + pure: + pure === null + ? (external ? this.isPureExternalModule : this.isPureInternalModule)(id) || false + : pure + }; } private resolveAndFetchDependency( diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index 7e39741cf67..c3f5f329e86 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -165,6 +165,8 @@ export type ResolveIdHook = ( export type IsExternal = (source: string, importer: string, isResolved: boolean) => boolean | void; +export type IsPureModule = (id: string) => boolean | void; + export type LoadHook = ( this: PluginContext, id: string @@ -317,12 +319,17 @@ export interface Plugin extends PluginHooks { export interface TreeshakingOptions { annotations?: boolean; propertyReadSideEffects?: boolean; - pureExternalModules?: boolean; + // TODO Lukas adjust docs + pureExternalModules?: PureModulesOption; + // TODO Lukas adjust docs + pureInternalModules?: PureModulesOption; } export type GetManualChunk = (id: string) => string | void; -export type ExternalOption = string[] | IsExternal; +// TODO Lukas test and document true +export type ExternalOption = boolean | string[] | IsExternal; +export type PureModulesOption = boolean | string[] | IsPureModule; export type GlobalsOption = { [name: string]: string } | ((name: string) => string); export type InputOption = string | string[] | { [entryAlias: string]: string }; export type ManualChunksOption = { [chunkAlias: string]: string[] } | GetManualChunk; diff --git a/src/utils/mergeOptions.ts b/src/utils/mergeOptions.ts index 63d2a2fcb14..37016d94b5f 100644 --- a/src/utils/mergeOptions.ts +++ b/src/utils/mergeOptions.ts @@ -56,7 +56,6 @@ const getOnWarn = ( ? warning => config.onwarn(warning, defaultOnWarnHandler) : defaultOnWarnHandler; -// TODO Lukas manual chunks should receive the same treatment const getExternal = (config: GenericConfigObject, command: GenericConfigObject) => { const configExternal = config.external; return typeof configExternal === 'function' diff --git a/test/function/samples/pure-modules/resolve-id/_config.js b/test/function/samples/pure-modules/resolve-id/_config.js index 3de71c7e584..459f9b02c48 100644 --- a/test/function/samples/pure-modules/resolve-id/_config.js +++ b/test/function/samples/pure-modules/resolve-id/_config.js @@ -1,8 +1,7 @@ const assert = require('assert'); const sideEffects = []; -// TODO Lukas interaction with pureExternalModules, use as default? -// TODO Lukas handle other hooks +// TODO Lukas implement load and transform hooks module.exports = { description: 'does not include modules without used exports if moduleSideEffect is false', context: { @@ -14,23 +13,65 @@ module.exports = { }, exports() { assert.deepStrictEqual(sideEffects, [ - 'pure-false-external-true', - 'pure-false-external-true-unused-import', - 'pure-false-external-true-used-import', - 'pure-null-external-true', - 'pure-null-external-true-unused-import', - 'pure-null-external-true-used-import', - 'pure-true-external-true-used-import', - 'pure-false-external-false', - 'pure-false-external-false-unused-import', - 'pure-false-external-false-used-import', - 'pure-null-external-false', - 'pure-null-external-false-unused-import', - 'pure-null-external-false-used-import', - 'pure-true-external-false-used-import' + 'pure-false-external-true-pureint-false-pureext-false', + 'pure-false-external-true-pureint-false-pureext-false-unused-import', + 'pure-false-external-true-pureint-false-pureext-false-used-import', + 'pure-null-external-true-pureint-false-pureext-false', + 'pure-null-external-true-pureint-false-pureext-false-unused-import', + 'pure-null-external-true-pureint-false-pureext-false-used-import', + 'pure-true-external-true-pureint-false-pureext-false-used-import', + 'pure-false-external-true-pureint-true-pureext-false', + 'pure-false-external-true-pureint-true-pureext-false-unused-import', + 'pure-false-external-true-pureint-true-pureext-false-used-import', + 'pure-null-external-true-pureint-true-pureext-false', + 'pure-null-external-true-pureint-true-pureext-false-unused-import', + 'pure-null-external-true-pureint-true-pureext-false-used-import', + 'pure-true-external-true-pureint-true-pureext-false-used-import', + 'pure-false-external-true-pureint-false-pureext-true', + 'pure-false-external-true-pureint-false-pureext-true-unused-import', + 'pure-false-external-true-pureint-false-pureext-true-used-import', + 'pure-null-external-true-pureint-false-pureext-true-used-import', + 'pure-true-external-true-pureint-false-pureext-true-used-import', + 'pure-false-external-true-pureint-true-pureext-true', + 'pure-false-external-true-pureint-true-pureext-true-unused-import', + 'pure-false-external-true-pureint-true-pureext-true-used-import', + 'pure-null-external-true-pureint-true-pureext-true-used-import', + 'pure-true-external-true-pureint-true-pureext-true-used-import', + 'pure-false-external-false-pureint-false-pureext-false', + 'pure-false-external-false-pureint-false-pureext-false-unused-import', + 'pure-false-external-false-pureint-false-pureext-false-used-import', + 'pure-null-external-false-pureint-false-pureext-false', + 'pure-null-external-false-pureint-false-pureext-false-unused-import', + 'pure-null-external-false-pureint-false-pureext-false-used-import', + 'pure-true-external-false-pureint-false-pureext-false-used-import', + 'pure-false-external-false-pureint-true-pureext-false', + 'pure-false-external-false-pureint-true-pureext-false-unused-import', + 'pure-false-external-false-pureint-true-pureext-false-used-import', + 'pure-null-external-false-pureint-true-pureext-false-used-import', + 'pure-true-external-false-pureint-true-pureext-false-used-import', + 'pure-false-external-false-pureint-false-pureext-true', + 'pure-false-external-false-pureint-false-pureext-true-unused-import', + 'pure-false-external-false-pureint-false-pureext-true-used-import', + 'pure-null-external-false-pureint-false-pureext-true', + 'pure-null-external-false-pureint-false-pureext-true-unused-import', + 'pure-null-external-false-pureint-false-pureext-true-used-import', + 'pure-true-external-false-pureint-false-pureext-true-used-import', + 'pure-false-external-false-pureint-true-pureext-true', + 'pure-false-external-false-pureint-true-pureext-true-unused-import', + 'pure-false-external-false-pureint-true-pureext-true-used-import', + 'pure-null-external-false-pureint-true-pureext-true-used-import', + 'pure-true-external-false-pureint-true-pureext-true-used-import' ]); }, options: { + treeshake: { + pureExternalModules(id) { + return JSON.parse(id.split('-')[7]); + }, + pureInternalModules(id) { + return JSON.parse(id.split('-')[5]); + } + }, plugins: { name: 'test-plugin', resolveId(id) { @@ -46,7 +87,8 @@ module.exports = { load(id) { if (id[0] !== '/') { const pure = JSON.parse(id.split('-')[1]); - assert.strictEqual(this.getModuleInfo(id).isPure, pure || false); + const pureInt = JSON.parse(id.split('-')[5]); + assert.strictEqual(this.getModuleInfo(id).isPure, pure === null ? pureInt : pure); return `export const value = '${id}'; sideEffects.push(value);`; } }, @@ -57,50 +99,185 @@ module.exports = { .sort() .map(id => ({ id, isPure: this.getModuleInfo(id).isPure })), [ - { id: 'pure-false-external-false', isPure: false }, - { id: 'pure-false-external-false-unused-import', isPure: false }, - { id: 'pure-false-external-false-used-import', isPure: false }, - { id: 'pure-false-external-true', isPure: false }, - { id: 'pure-false-external-true-unused-import', isPure: false }, - { id: 'pure-false-external-true-used-import', isPure: false }, - { id: 'pure-null-external-false', isPure: false }, - { id: 'pure-null-external-false-unused-import', isPure: false }, - { id: 'pure-null-external-false-used-import', isPure: false }, - { id: 'pure-null-external-true', isPure: false }, - { id: 'pure-null-external-true-unused-import', isPure: false }, - { id: 'pure-null-external-true-used-import', isPure: false }, - { id: 'pure-true-external-false', isPure: true }, - { id: 'pure-true-external-false-unused-import', isPure: true }, - { id: 'pure-true-external-false-used-import', isPure: true }, - { id: 'pure-true-external-true', isPure: true }, - { id: 'pure-true-external-true-unused-import', isPure: true }, - { id: 'pure-true-external-true-used-import', isPure: true } + { id: 'pure-false-external-false-pureint-false-pureext-false', isPure: false }, + { + id: 'pure-false-external-false-pureint-false-pureext-false-unused-import', + isPure: false + }, + { + id: 'pure-false-external-false-pureint-false-pureext-false-used-import', + isPure: false + }, + { id: 'pure-false-external-false-pureint-false-pureext-true', isPure: false }, + { + id: 'pure-false-external-false-pureint-false-pureext-true-unused-import', + isPure: false + }, + { + id: 'pure-false-external-false-pureint-false-pureext-true-used-import', + isPure: false + }, + { id: 'pure-false-external-false-pureint-true-pureext-false', isPure: false }, + { + id: 'pure-false-external-false-pureint-true-pureext-false-unused-import', + isPure: false + }, + { + id: 'pure-false-external-false-pureint-true-pureext-false-used-import', + isPure: false + }, + { id: 'pure-false-external-false-pureint-true-pureext-true', isPure: false }, + { + id: 'pure-false-external-false-pureint-true-pureext-true-unused-import', + isPure: false + }, + { + id: 'pure-false-external-false-pureint-true-pureext-true-used-import', + isPure: false + }, + { id: 'pure-false-external-true-pureint-false-pureext-false', isPure: false }, + { + id: 'pure-false-external-true-pureint-false-pureext-false-unused-import', + isPure: false + }, + { + id: 'pure-false-external-true-pureint-false-pureext-false-used-import', + isPure: false + }, + { id: 'pure-false-external-true-pureint-false-pureext-true', isPure: false }, + { + id: 'pure-false-external-true-pureint-false-pureext-true-unused-import', + isPure: false + }, + { + id: 'pure-false-external-true-pureint-false-pureext-true-used-import', + isPure: false + }, + { id: 'pure-false-external-true-pureint-true-pureext-false', isPure: false }, + { + id: 'pure-false-external-true-pureint-true-pureext-false-unused-import', + isPure: false + }, + { + id: 'pure-false-external-true-pureint-true-pureext-false-used-import', + isPure: false + }, + { id: 'pure-false-external-true-pureint-true-pureext-true', isPure: false }, + { + id: 'pure-false-external-true-pureint-true-pureext-true-unused-import', + isPure: false + }, + { id: 'pure-false-external-true-pureint-true-pureext-true-used-import', isPure: false }, + { id: 'pure-null-external-false-pureint-false-pureext-false', isPure: false }, + { + id: 'pure-null-external-false-pureint-false-pureext-false-unused-import', + isPure: false + }, + { + id: 'pure-null-external-false-pureint-false-pureext-false-used-import', + isPure: false + }, + { id: 'pure-null-external-false-pureint-false-pureext-true', isPure: false }, + { + id: 'pure-null-external-false-pureint-false-pureext-true-unused-import', + isPure: false + }, + { + id: 'pure-null-external-false-pureint-false-pureext-true-used-import', + isPure: false + }, + { id: 'pure-null-external-false-pureint-true-pureext-false', isPure: true }, + { + id: 'pure-null-external-false-pureint-true-pureext-false-unused-import', + isPure: true + }, + { id: 'pure-null-external-false-pureint-true-pureext-false-used-import', isPure: true }, + { id: 'pure-null-external-false-pureint-true-pureext-true', isPure: true }, + { + id: 'pure-null-external-false-pureint-true-pureext-true-unused-import', + isPure: true + }, + { id: 'pure-null-external-false-pureint-true-pureext-true-used-import', isPure: true }, + { id: 'pure-null-external-true-pureint-false-pureext-false', isPure: false }, + { + id: 'pure-null-external-true-pureint-false-pureext-false-unused-import', + isPure: false + }, + { + id: 'pure-null-external-true-pureint-false-pureext-false-used-import', + isPure: false + }, + { id: 'pure-null-external-true-pureint-false-pureext-true', isPure: true }, + { + id: 'pure-null-external-true-pureint-false-pureext-true-unused-import', + isPure: true + }, + { id: 'pure-null-external-true-pureint-false-pureext-true-used-import', isPure: true }, + { id: 'pure-null-external-true-pureint-true-pureext-false', isPure: false }, + { + id: 'pure-null-external-true-pureint-true-pureext-false-unused-import', + isPure: false + }, + { id: 'pure-null-external-true-pureint-true-pureext-false-used-import', isPure: false }, + { id: 'pure-null-external-true-pureint-true-pureext-true', isPure: true }, + { id: 'pure-null-external-true-pureint-true-pureext-true-unused-import', isPure: true }, + { id: 'pure-null-external-true-pureint-true-pureext-true-used-import', isPure: true }, + { id: 'pure-true-external-false-pureint-false-pureext-false', isPure: true }, + { + id: 'pure-true-external-false-pureint-false-pureext-false-unused-import', + isPure: true + }, + { + id: 'pure-true-external-false-pureint-false-pureext-false-used-import', + isPure: true + }, + { id: 'pure-true-external-false-pureint-false-pureext-true', isPure: true }, + { + id: 'pure-true-external-false-pureint-false-pureext-true-unused-import', + isPure: true + }, + { id: 'pure-true-external-false-pureint-false-pureext-true-used-import', isPure: true }, + { id: 'pure-true-external-false-pureint-true-pureext-false', isPure: true }, + { + id: 'pure-true-external-false-pureint-true-pureext-false-unused-import', + isPure: true + }, + { id: 'pure-true-external-false-pureint-true-pureext-false-used-import', isPure: true }, + { id: 'pure-true-external-false-pureint-true-pureext-true', isPure: true }, + { + id: 'pure-true-external-false-pureint-true-pureext-true-unused-import', + isPure: true + }, + { id: 'pure-true-external-false-pureint-true-pureext-true-used-import', isPure: true }, + { id: 'pure-true-external-true-pureint-false-pureext-false', isPure: true }, + { + id: 'pure-true-external-true-pureint-false-pureext-false-unused-import', + isPure: true + }, + { id: 'pure-true-external-true-pureint-false-pureext-false-used-import', isPure: true }, + { id: 'pure-true-external-true-pureint-false-pureext-true', isPure: true }, + { + id: 'pure-true-external-true-pureint-false-pureext-true-unused-import', + isPure: true + }, + { id: 'pure-true-external-true-pureint-false-pureext-true-used-import', isPure: true }, + { id: 'pure-true-external-true-pureint-true-pureext-false', isPure: true }, + { + id: 'pure-true-external-true-pureint-true-pureext-false-unused-import', + isPure: true + }, + { id: 'pure-true-external-true-pureint-true-pureext-false-used-import', isPure: true }, + { id: 'pure-true-external-true-pureint-true-pureext-true', isPure: true }, + { id: 'pure-true-external-true-pureint-true-pureext-true-unused-import', isPure: true }, + { id: 'pure-true-external-true-pureint-true-pureext-true-used-import', isPure: true } ] ); } } }, - warnings: [ - { - code: 'UNUSED_EXTERNAL_IMPORT', - message: - "'value' is imported from external module 'pure-false-external-true-unused-import' but never used", - names: ['value'], - source: 'pure-false-external-true-unused-import' - }, - { - code: 'UNUSED_EXTERNAL_IMPORT', - message: - "'value' is imported from external module 'pure-null-external-true-unused-import' but never used", - names: ['value'], - source: 'pure-null-external-true-unused-import' - }, - { - code: 'UNUSED_EXTERNAL_IMPORT', - message: - "'value' is imported from external module 'pure-true-external-true-unused-import' but never used", - names: ['value'], - source: 'pure-true-external-true-unused-import' + warnings(warnings) { + for (const warning of warnings) { + assert.strictEqual(warning.code, 'UNUSED_EXTERNAL_IMPORT'); } - ] + } }; diff --git a/test/function/samples/pure-modules/resolve-id/main.js b/test/function/samples/pure-modules/resolve-id/main.js index 31303129063..d67728d98a4 100644 --- a/test/function/samples/pure-modules/resolve-id/main.js +++ b/test/function/samples/pure-modules/resolve-id/main.js @@ -1,25 +1,122 @@ -import 'pure-false-external-false'; -import { value as unusedValue1 } from 'pure-false-external-false-unused-import'; -import { value as usedValue1 } from 'pure-false-external-false-used-import'; +import 'pure-false-external-false-pureint-false-pureext-false'; +import { value as unusedValue1 } from 'pure-false-external-false-pureint-false-pureext-false-unused-import'; +import { value as usedValue1 } from 'pure-false-external-false-pureint-false-pureext-false-used-import'; -import 'pure-null-external-false'; -import { value as unusedValue2 } from 'pure-null-external-false-unused-import'; -import { value as usedValue2 } from 'pure-null-external-false-used-import'; +import 'pure-null-external-false-pureint-false-pureext-false'; +import { value as unusedValue2 } from 'pure-null-external-false-pureint-false-pureext-false-unused-import'; +import { value as usedValue2 } from 'pure-null-external-false-pureint-false-pureext-false-used-import'; -import 'pure-true-external-false'; -import { value as unusedValue3 } from 'pure-true-external-false-unused-import'; -import { value as usedValue3 } from 'pure-true-external-false-used-import'; +import 'pure-true-external-false-pureint-false-pureext-false'; +import { value as unusedValue3 } from 'pure-true-external-false-pureint-false-pureext-false-unused-import'; +import { value as usedValue3 } from 'pure-true-external-false-pureint-false-pureext-false-used-import'; -import 'pure-false-external-true'; -import { value as unusedValue4 } from 'pure-false-external-true-unused-import'; -import { value as usedValue4 } from 'pure-false-external-true-used-import'; +import 'pure-false-external-true-pureint-false-pureext-false'; +import { value as unusedValue4 } from 'pure-false-external-true-pureint-false-pureext-false-unused-import'; +import { value as usedValue4 } from 'pure-false-external-true-pureint-false-pureext-false-used-import'; -import 'pure-null-external-true'; -import { value as unusedValue5 } from 'pure-null-external-true-unused-import'; -import { value as usedValue5 } from 'pure-null-external-true-used-import'; +import 'pure-null-external-true-pureint-false-pureext-false'; +import { value as unusedValue5 } from 'pure-null-external-true-pureint-false-pureext-false-unused-import'; +import { value as usedValue5 } from 'pure-null-external-true-pureint-false-pureext-false-used-import'; -import 'pure-true-external-true'; -import { value as unusedValue6 } from 'pure-true-external-true-unused-import'; -import { value as usedValue6 } from 'pure-true-external-true-used-import'; +import 'pure-true-external-true-pureint-false-pureext-false'; +import { value as unusedValue6 } from 'pure-true-external-true-pureint-false-pureext-false-unused-import'; +import { value as usedValue6 } from 'pure-true-external-true-pureint-false-pureext-false-used-import'; -export const values = [usedValue1, usedValue2, usedValue3, usedValue4, usedValue5, usedValue6]; +import 'pure-false-external-false-pureint-true-pureext-false'; +import { value as unusedValue7 } from 'pure-false-external-false-pureint-true-pureext-false-unused-import'; +import { value as usedValue7 } from 'pure-false-external-false-pureint-true-pureext-false-used-import'; + +import 'pure-null-external-false-pureint-true-pureext-false'; +import { value as unusedValue8 } from 'pure-null-external-false-pureint-true-pureext-false-unused-import'; +import { value as usedValue8 } from 'pure-null-external-false-pureint-true-pureext-false-used-import'; + +import 'pure-true-external-false-pureint-true-pureext-false'; +import { value as unusedValue9 } from 'pure-true-external-false-pureint-true-pureext-false-unused-import'; +import { value as usedValue9 } from 'pure-true-external-false-pureint-true-pureext-false-used-import'; + +import 'pure-false-external-true-pureint-true-pureext-false'; +import { value as unusedValue10 } from 'pure-false-external-true-pureint-true-pureext-false-unused-import'; +import { value as usedValue10 } from 'pure-false-external-true-pureint-true-pureext-false-used-import'; + +import 'pure-null-external-true-pureint-true-pureext-false'; +import { value as unusedValue11 } from 'pure-null-external-true-pureint-true-pureext-false-unused-import'; +import { value as usedValue11 } from 'pure-null-external-true-pureint-true-pureext-false-used-import'; + +import 'pure-true-external-true-pureint-true-pureext-false'; +import { value as unusedValue12 } from 'pure-true-external-true-pureint-true-pureext-false-unused-import'; +import { value as usedValue12 } from 'pure-true-external-true-pureint-true-pureext-false-used-import'; + +import 'pure-false-external-false-pureint-false-pureext-true'; +import { value as unusedValue13 } from 'pure-false-external-false-pureint-false-pureext-true-unused-import'; +import { value as usedValue13 } from 'pure-false-external-false-pureint-false-pureext-true-used-import'; + +import 'pure-null-external-false-pureint-false-pureext-true'; +import { value as unusedValue14 } from 'pure-null-external-false-pureint-false-pureext-true-unused-import'; +import { value as usedValue14 } from 'pure-null-external-false-pureint-false-pureext-true-used-import'; + +import 'pure-true-external-false-pureint-false-pureext-true'; +import { value as unusedValue15 } from 'pure-true-external-false-pureint-false-pureext-true-unused-import'; +import { value as usedValue15 } from 'pure-true-external-false-pureint-false-pureext-true-used-import'; + +import 'pure-false-external-true-pureint-false-pureext-true'; +import { value as unusedValue16 } from 'pure-false-external-true-pureint-false-pureext-true-unused-import'; +import { value as usedValue16 } from 'pure-false-external-true-pureint-false-pureext-true-used-import'; + +import 'pure-null-external-true-pureint-false-pureext-true'; +import { value as unusedValue17 } from 'pure-null-external-true-pureint-false-pureext-true-unused-import'; +import { value as usedValue17 } from 'pure-null-external-true-pureint-false-pureext-true-used-import'; + +import 'pure-true-external-true-pureint-false-pureext-true'; +import { value as unusedValue18 } from 'pure-true-external-true-pureint-false-pureext-true-unused-import'; +import { value as usedValue18 } from 'pure-true-external-true-pureint-false-pureext-true-used-import'; + +import 'pure-false-external-false-pureint-true-pureext-true'; +import { value as unusedValue19 } from 'pure-false-external-false-pureint-true-pureext-true-unused-import'; +import { value as usedValue19 } from 'pure-false-external-false-pureint-true-pureext-true-used-import'; + +import 'pure-null-external-false-pureint-true-pureext-true'; +import { value as unusedValue20 } from 'pure-null-external-false-pureint-true-pureext-true-unused-import'; +import { value as usedValue20 } from 'pure-null-external-false-pureint-true-pureext-true-used-import'; + +import 'pure-true-external-false-pureint-true-pureext-true'; +import { value as unusedValue21 } from 'pure-true-external-false-pureint-true-pureext-true-unused-import'; +import { value as usedValue21 } from 'pure-true-external-false-pureint-true-pureext-true-used-import'; + +import 'pure-false-external-true-pureint-true-pureext-true'; +import { value as unusedValue22 } from 'pure-false-external-true-pureint-true-pureext-true-unused-import'; +import { value as usedValue22 } from 'pure-false-external-true-pureint-true-pureext-true-used-import'; + +import 'pure-null-external-true-pureint-true-pureext-true'; +import { value as unusedValue23 } from 'pure-null-external-true-pureint-true-pureext-true-unused-import'; +import { value as usedValue23 } from 'pure-null-external-true-pureint-true-pureext-true-used-import'; + +import 'pure-true-external-true-pureint-true-pureext-true'; +import { value as unusedValue24 } from 'pure-true-external-true-pureint-true-pureext-true-unused-import'; +import { value as usedValue24 } from 'pure-true-external-true-pureint-true-pureext-true-used-import'; + +export const values = [ + usedValue1, + usedValue2, + usedValue3, + usedValue4, + usedValue5, + usedValue6, + usedValue7, + usedValue8, + usedValue9, + usedValue10, + usedValue11, + usedValue12, + usedValue13, + usedValue14, + usedValue15, + usedValue16, + usedValue17, + usedValue18, + usedValue19, + usedValue20, + usedValue21, + usedValue22, + usedValue23, + usedValue24 +]; From c278edf27c7eb50b56a7e16550ff060a1a502520 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sun, 5 May 2019 20:18:57 +0200 Subject: [PATCH 07/19] Split test, regenerate iterator on demand --- src/utils/pluginDriver.ts | 4 +- .../resolve-id-external/_config.js | 80 ++++++ .../pure-modules/resolve-id-external/main.js | 25 ++ .../pure-modules/resolve-id/_config.js | 272 +++--------------- .../samples/pure-modules/resolve-id/main.js | 135 ++------- 5 files changed, 162 insertions(+), 354 deletions(-) create mode 100644 test/function/samples/pure-modules/resolve-id-external/_config.js create mode 100644 test/function/samples/pure-modules/resolve-id-external/main.js diff --git a/src/utils/pluginDriver.ts b/src/utils/pluginDriver.ts index 204de313634..0e8c9dbe7ec 100644 --- a/src/utils/pluginDriver.ts +++ b/src/utils/pluginDriver.ts @@ -185,7 +185,9 @@ export function createPluginDriver( meta: { rollupVersion }, - moduleIds: graph.moduleById.keys(), + get moduleIds() { + return graph.moduleById.keys(); + }, parse: graph.contextParse, resolveId(source, importer) { return graph.moduleLoader diff --git a/test/function/samples/pure-modules/resolve-id-external/_config.js b/test/function/samples/pure-modules/resolve-id-external/_config.js new file mode 100644 index 00000000000..c0fab465153 --- /dev/null +++ b/test/function/samples/pure-modules/resolve-id-external/_config.js @@ -0,0 +1,80 @@ +const assert = require('assert'); +const sideEffects = []; + +module.exports = { + description: 'does not include modules without used exports if moduleSideEffect is false', + context: { + require(id) { + sideEffects.push(id); + return { value: id }; + } + }, + exports() { + assert.deepStrictEqual(sideEffects, [ + 'pure-false-pureext-false', + 'pure-false-pureext-false-unused-import', + 'pure-false-pureext-false-used-import', + 'pure-null-pureext-false', + 'pure-null-pureext-false-unused-import', + 'pure-null-pureext-false-used-import', + 'pure-true-pureext-false-used-import', + 'pure-false-pureext-true', + 'pure-false-pureext-true-unused-import', + 'pure-false-pureext-true-used-import', + 'pure-null-pureext-true-used-import', + 'pure-true-pureext-true-used-import' + ]); + }, + options: { + treeshake: { + pureExternalModules(id) { + return JSON.parse(id.split('-')[3]); + } + }, + plugins: { + name: 'test-plugin', + resolveId(id) { + if (id[0] !== '/') { + return { + id, + external: true, + pure: JSON.parse(id.split('-')[1]) + }; + } + }, + buildEnd() { + assert.deepStrictEqual( + Array.from(this.moduleIds) + .filter(id => id[0] !== '/') + .sort() + .map(id => ({ id, isPure: this.getModuleInfo(id).isPure })), + [ + { id: 'pure-false-pureext-false', isPure: false }, + { id: 'pure-false-pureext-false-unused-import', isPure: false }, + { id: 'pure-false-pureext-false-used-import', isPure: false }, + { id: 'pure-false-pureext-true', isPure: false }, + { id: 'pure-false-pureext-true-unused-import', isPure: false }, + { id: 'pure-false-pureext-true-used-import', isPure: false }, + { id: 'pure-null-pureext-false', isPure: false }, + { id: 'pure-null-pureext-false-unused-import', isPure: false }, + { id: 'pure-null-pureext-false-used-import', isPure: false }, + { id: 'pure-null-pureext-true', isPure: true }, + { id: 'pure-null-pureext-true-unused-import', isPure: true }, + { id: 'pure-null-pureext-true-used-import', isPure: true }, + { id: 'pure-true-pureext-false', isPure: true }, + { id: 'pure-true-pureext-false-unused-import', isPure: true }, + { id: 'pure-true-pureext-false-used-import', isPure: true }, + { id: 'pure-true-pureext-true', isPure: true }, + { id: 'pure-true-pureext-true-unused-import', isPure: true }, + { id: 'pure-true-pureext-true-used-import', isPure: true } + ] + ); + } + } + }, + warnings(warnings) { + for (const warning of warnings) { + assert.strictEqual(warning.code, 'UNUSED_EXTERNAL_IMPORT'); + } + } +}; diff --git a/test/function/samples/pure-modules/resolve-id-external/main.js b/test/function/samples/pure-modules/resolve-id-external/main.js new file mode 100644 index 00000000000..82aba2a8f81 --- /dev/null +++ b/test/function/samples/pure-modules/resolve-id-external/main.js @@ -0,0 +1,25 @@ +import 'pure-false-pureext-false'; +import { value as unusedValue1 } from 'pure-false-pureext-false-unused-import'; +import { value as usedValue1 } from 'pure-false-pureext-false-used-import'; + +import 'pure-null-pureext-false'; +import { value as unusedValue2 } from 'pure-null-pureext-false-unused-import'; +import { value as usedValue2 } from 'pure-null-pureext-false-used-import'; + +import 'pure-true-pureext-false'; +import { value as unusedValue3 } from 'pure-true-pureext-false-unused-import'; +import { value as usedValue3 } from 'pure-true-pureext-false-used-import'; + +import 'pure-false-pureext-true'; +import { value as unusedValue4 } from 'pure-false-pureext-true-unused-import'; +import { value as usedValue4 } from 'pure-false-pureext-true-used-import'; + +import 'pure-null-pureext-true'; +import { value as unusedValue5 } from 'pure-null-pureext-true-unused-import'; +import { value as usedValue5 } from 'pure-null-pureext-true-used-import'; + +import 'pure-true-pureext-true'; +import { value as unusedValue6 } from 'pure-true-pureext-true-unused-import'; +import { value as usedValue6 } from 'pure-true-pureext-true-used-import'; + +export const values = [usedValue1, usedValue2, usedValue3, usedValue4, usedValue5, usedValue6]; diff --git a/test/function/samples/pure-modules/resolve-id/_config.js b/test/function/samples/pure-modules/resolve-id/_config.js index 459f9b02c48..ad0ce7d3f6d 100644 --- a/test/function/samples/pure-modules/resolve-id/_config.js +++ b/test/function/samples/pure-modules/resolve-id/_config.js @@ -5,89 +5,45 @@ const sideEffects = []; module.exports = { description: 'does not include modules without used exports if moduleSideEffect is false', context: { - sideEffects, - require(id) { - sideEffects.push(id); - return { value: id }; - } + sideEffects }, exports() { assert.deepStrictEqual(sideEffects, [ - 'pure-false-external-true-pureint-false-pureext-false', - 'pure-false-external-true-pureint-false-pureext-false-unused-import', - 'pure-false-external-true-pureint-false-pureext-false-used-import', - 'pure-null-external-true-pureint-false-pureext-false', - 'pure-null-external-true-pureint-false-pureext-false-unused-import', - 'pure-null-external-true-pureint-false-pureext-false-used-import', - 'pure-true-external-true-pureint-false-pureext-false-used-import', - 'pure-false-external-true-pureint-true-pureext-false', - 'pure-false-external-true-pureint-true-pureext-false-unused-import', - 'pure-false-external-true-pureint-true-pureext-false-used-import', - 'pure-null-external-true-pureint-true-pureext-false', - 'pure-null-external-true-pureint-true-pureext-false-unused-import', - 'pure-null-external-true-pureint-true-pureext-false-used-import', - 'pure-true-external-true-pureint-true-pureext-false-used-import', - 'pure-false-external-true-pureint-false-pureext-true', - 'pure-false-external-true-pureint-false-pureext-true-unused-import', - 'pure-false-external-true-pureint-false-pureext-true-used-import', - 'pure-null-external-true-pureint-false-pureext-true-used-import', - 'pure-true-external-true-pureint-false-pureext-true-used-import', - 'pure-false-external-true-pureint-true-pureext-true', - 'pure-false-external-true-pureint-true-pureext-true-unused-import', - 'pure-false-external-true-pureint-true-pureext-true-used-import', - 'pure-null-external-true-pureint-true-pureext-true-used-import', - 'pure-true-external-true-pureint-true-pureext-true-used-import', - 'pure-false-external-false-pureint-false-pureext-false', - 'pure-false-external-false-pureint-false-pureext-false-unused-import', - 'pure-false-external-false-pureint-false-pureext-false-used-import', - 'pure-null-external-false-pureint-false-pureext-false', - 'pure-null-external-false-pureint-false-pureext-false-unused-import', - 'pure-null-external-false-pureint-false-pureext-false-used-import', - 'pure-true-external-false-pureint-false-pureext-false-used-import', - 'pure-false-external-false-pureint-true-pureext-false', - 'pure-false-external-false-pureint-true-pureext-false-unused-import', - 'pure-false-external-false-pureint-true-pureext-false-used-import', - 'pure-null-external-false-pureint-true-pureext-false-used-import', - 'pure-true-external-false-pureint-true-pureext-false-used-import', - 'pure-false-external-false-pureint-false-pureext-true', - 'pure-false-external-false-pureint-false-pureext-true-unused-import', - 'pure-false-external-false-pureint-false-pureext-true-used-import', - 'pure-null-external-false-pureint-false-pureext-true', - 'pure-null-external-false-pureint-false-pureext-true-unused-import', - 'pure-null-external-false-pureint-false-pureext-true-used-import', - 'pure-true-external-false-pureint-false-pureext-true-used-import', - 'pure-false-external-false-pureint-true-pureext-true', - 'pure-false-external-false-pureint-true-pureext-true-unused-import', - 'pure-false-external-false-pureint-true-pureext-true-used-import', - 'pure-null-external-false-pureint-true-pureext-true-used-import', - 'pure-true-external-false-pureint-true-pureext-true-used-import' + 'pure-false-pureint-false', + 'pure-false-pureint-false-unused-import', + 'pure-false-pureint-false-used-import', + 'pure-null-pureint-false', + 'pure-null-pureint-false-unused-import', + 'pure-null-pureint-false-used-import', + 'pure-true-pureint-false-used-import', + 'pure-false-pureint-true', + 'pure-false-pureint-true-unused-import', + 'pure-false-pureint-true-used-import', + 'pure-null-pureint-true-used-import', + 'pure-true-pureint-true-used-import' ]); }, options: { treeshake: { - pureExternalModules(id) { - return JSON.parse(id.split('-')[7]); - }, pureInternalModules(id) { - return JSON.parse(id.split('-')[5]); + return JSON.parse(id.split('-')[3]); } }, plugins: { name: 'test-plugin', resolveId(id) { if (id[0] !== '/') { - const [, pure, , external] = id.split('-'); return { id, - external: JSON.parse(external), - pure: JSON.parse(pure) + external: false, + pure: JSON.parse(id.split('-')[1]) }; } }, load(id) { if (id[0] !== '/') { const pure = JSON.parse(id.split('-')[1]); - const pureInt = JSON.parse(id.split('-')[5]); + const pureInt = JSON.parse(id.split('-')[3]); assert.strictEqual(this.getModuleInfo(id).isPure, pure === null ? pureInt : pure); return `export const value = '${id}'; sideEffects.push(value);`; } @@ -99,185 +55,27 @@ module.exports = { .sort() .map(id => ({ id, isPure: this.getModuleInfo(id).isPure })), [ - { id: 'pure-false-external-false-pureint-false-pureext-false', isPure: false }, - { - id: 'pure-false-external-false-pureint-false-pureext-false-unused-import', - isPure: false - }, - { - id: 'pure-false-external-false-pureint-false-pureext-false-used-import', - isPure: false - }, - { id: 'pure-false-external-false-pureint-false-pureext-true', isPure: false }, - { - id: 'pure-false-external-false-pureint-false-pureext-true-unused-import', - isPure: false - }, - { - id: 'pure-false-external-false-pureint-false-pureext-true-used-import', - isPure: false - }, - { id: 'pure-false-external-false-pureint-true-pureext-false', isPure: false }, - { - id: 'pure-false-external-false-pureint-true-pureext-false-unused-import', - isPure: false - }, - { - id: 'pure-false-external-false-pureint-true-pureext-false-used-import', - isPure: false - }, - { id: 'pure-false-external-false-pureint-true-pureext-true', isPure: false }, - { - id: 'pure-false-external-false-pureint-true-pureext-true-unused-import', - isPure: false - }, - { - id: 'pure-false-external-false-pureint-true-pureext-true-used-import', - isPure: false - }, - { id: 'pure-false-external-true-pureint-false-pureext-false', isPure: false }, - { - id: 'pure-false-external-true-pureint-false-pureext-false-unused-import', - isPure: false - }, - { - id: 'pure-false-external-true-pureint-false-pureext-false-used-import', - isPure: false - }, - { id: 'pure-false-external-true-pureint-false-pureext-true', isPure: false }, - { - id: 'pure-false-external-true-pureint-false-pureext-true-unused-import', - isPure: false - }, - { - id: 'pure-false-external-true-pureint-false-pureext-true-used-import', - isPure: false - }, - { id: 'pure-false-external-true-pureint-true-pureext-false', isPure: false }, - { - id: 'pure-false-external-true-pureint-true-pureext-false-unused-import', - isPure: false - }, - { - id: 'pure-false-external-true-pureint-true-pureext-false-used-import', - isPure: false - }, - { id: 'pure-false-external-true-pureint-true-pureext-true', isPure: false }, - { - id: 'pure-false-external-true-pureint-true-pureext-true-unused-import', - isPure: false - }, - { id: 'pure-false-external-true-pureint-true-pureext-true-used-import', isPure: false }, - { id: 'pure-null-external-false-pureint-false-pureext-false', isPure: false }, - { - id: 'pure-null-external-false-pureint-false-pureext-false-unused-import', - isPure: false - }, - { - id: 'pure-null-external-false-pureint-false-pureext-false-used-import', - isPure: false - }, - { id: 'pure-null-external-false-pureint-false-pureext-true', isPure: false }, - { - id: 'pure-null-external-false-pureint-false-pureext-true-unused-import', - isPure: false - }, - { - id: 'pure-null-external-false-pureint-false-pureext-true-used-import', - isPure: false - }, - { id: 'pure-null-external-false-pureint-true-pureext-false', isPure: true }, - { - id: 'pure-null-external-false-pureint-true-pureext-false-unused-import', - isPure: true - }, - { id: 'pure-null-external-false-pureint-true-pureext-false-used-import', isPure: true }, - { id: 'pure-null-external-false-pureint-true-pureext-true', isPure: true }, - { - id: 'pure-null-external-false-pureint-true-pureext-true-unused-import', - isPure: true - }, - { id: 'pure-null-external-false-pureint-true-pureext-true-used-import', isPure: true }, - { id: 'pure-null-external-true-pureint-false-pureext-false', isPure: false }, - { - id: 'pure-null-external-true-pureint-false-pureext-false-unused-import', - isPure: false - }, - { - id: 'pure-null-external-true-pureint-false-pureext-false-used-import', - isPure: false - }, - { id: 'pure-null-external-true-pureint-false-pureext-true', isPure: true }, - { - id: 'pure-null-external-true-pureint-false-pureext-true-unused-import', - isPure: true - }, - { id: 'pure-null-external-true-pureint-false-pureext-true-used-import', isPure: true }, - { id: 'pure-null-external-true-pureint-true-pureext-false', isPure: false }, - { - id: 'pure-null-external-true-pureint-true-pureext-false-unused-import', - isPure: false - }, - { id: 'pure-null-external-true-pureint-true-pureext-false-used-import', isPure: false }, - { id: 'pure-null-external-true-pureint-true-pureext-true', isPure: true }, - { id: 'pure-null-external-true-pureint-true-pureext-true-unused-import', isPure: true }, - { id: 'pure-null-external-true-pureint-true-pureext-true-used-import', isPure: true }, - { id: 'pure-true-external-false-pureint-false-pureext-false', isPure: true }, - { - id: 'pure-true-external-false-pureint-false-pureext-false-unused-import', - isPure: true - }, - { - id: 'pure-true-external-false-pureint-false-pureext-false-used-import', - isPure: true - }, - { id: 'pure-true-external-false-pureint-false-pureext-true', isPure: true }, - { - id: 'pure-true-external-false-pureint-false-pureext-true-unused-import', - isPure: true - }, - { id: 'pure-true-external-false-pureint-false-pureext-true-used-import', isPure: true }, - { id: 'pure-true-external-false-pureint-true-pureext-false', isPure: true }, - { - id: 'pure-true-external-false-pureint-true-pureext-false-unused-import', - isPure: true - }, - { id: 'pure-true-external-false-pureint-true-pureext-false-used-import', isPure: true }, - { id: 'pure-true-external-false-pureint-true-pureext-true', isPure: true }, - { - id: 'pure-true-external-false-pureint-true-pureext-true-unused-import', - isPure: true - }, - { id: 'pure-true-external-false-pureint-true-pureext-true-used-import', isPure: true }, - { id: 'pure-true-external-true-pureint-false-pureext-false', isPure: true }, - { - id: 'pure-true-external-true-pureint-false-pureext-false-unused-import', - isPure: true - }, - { id: 'pure-true-external-true-pureint-false-pureext-false-used-import', isPure: true }, - { id: 'pure-true-external-true-pureint-false-pureext-true', isPure: true }, - { - id: 'pure-true-external-true-pureint-false-pureext-true-unused-import', - isPure: true - }, - { id: 'pure-true-external-true-pureint-false-pureext-true-used-import', isPure: true }, - { id: 'pure-true-external-true-pureint-true-pureext-false', isPure: true }, - { - id: 'pure-true-external-true-pureint-true-pureext-false-unused-import', - isPure: true - }, - { id: 'pure-true-external-true-pureint-true-pureext-false-used-import', isPure: true }, - { id: 'pure-true-external-true-pureint-true-pureext-true', isPure: true }, - { id: 'pure-true-external-true-pureint-true-pureext-true-unused-import', isPure: true }, - { id: 'pure-true-external-true-pureint-true-pureext-true-used-import', isPure: true } + { id: 'pure-false-pureint-false', isPure: false }, + { id: 'pure-false-pureint-false-unused-import', isPure: false }, + { id: 'pure-false-pureint-false-used-import', isPure: false }, + { id: 'pure-false-pureint-true', isPure: false }, + { id: 'pure-false-pureint-true-unused-import', isPure: false }, + { id: 'pure-false-pureint-true-used-import', isPure: false }, + { id: 'pure-null-pureint-false', isPure: false }, + { id: 'pure-null-pureint-false-unused-import', isPure: false }, + { id: 'pure-null-pureint-false-used-import', isPure: false }, + { id: 'pure-null-pureint-true', isPure: true }, + { id: 'pure-null-pureint-true-unused-import', isPure: true }, + { id: 'pure-null-pureint-true-used-import', isPure: true }, + { id: 'pure-true-pureint-false', isPure: true }, + { id: 'pure-true-pureint-false-unused-import', isPure: true }, + { id: 'pure-true-pureint-false-used-import', isPure: true }, + { id: 'pure-true-pureint-true', isPure: true }, + { id: 'pure-true-pureint-true-unused-import', isPure: true }, + { id: 'pure-true-pureint-true-used-import', isPure: true } ] ); } } - }, - warnings(warnings) { - for (const warning of warnings) { - assert.strictEqual(warning.code, 'UNUSED_EXTERNAL_IMPORT'); - } } }; diff --git a/test/function/samples/pure-modules/resolve-id/main.js b/test/function/samples/pure-modules/resolve-id/main.js index d67728d98a4..912110cff64 100644 --- a/test/function/samples/pure-modules/resolve-id/main.js +++ b/test/function/samples/pure-modules/resolve-id/main.js @@ -1,122 +1,25 @@ -import 'pure-false-external-false-pureint-false-pureext-false'; -import { value as unusedValue1 } from 'pure-false-external-false-pureint-false-pureext-false-unused-import'; -import { value as usedValue1 } from 'pure-false-external-false-pureint-false-pureext-false-used-import'; +import 'pure-false-pureint-false'; +import { value as unusedValue1 } from 'pure-false-pureint-false-unused-import'; +import { value as usedValue1 } from 'pure-false-pureint-false-used-import'; -import 'pure-null-external-false-pureint-false-pureext-false'; -import { value as unusedValue2 } from 'pure-null-external-false-pureint-false-pureext-false-unused-import'; -import { value as usedValue2 } from 'pure-null-external-false-pureint-false-pureext-false-used-import'; +import 'pure-null-pureint-false'; +import { value as unusedValue2 } from 'pure-null-pureint-false-unused-import'; +import { value as usedValue2 } from 'pure-null-pureint-false-used-import'; -import 'pure-true-external-false-pureint-false-pureext-false'; -import { value as unusedValue3 } from 'pure-true-external-false-pureint-false-pureext-false-unused-import'; -import { value as usedValue3 } from 'pure-true-external-false-pureint-false-pureext-false-used-import'; +import 'pure-true-pureint-false'; +import { value as unusedValue3 } from 'pure-true-pureint-false-unused-import'; +import { value as usedValue3 } from 'pure-true-pureint-false-used-import'; -import 'pure-false-external-true-pureint-false-pureext-false'; -import { value as unusedValue4 } from 'pure-false-external-true-pureint-false-pureext-false-unused-import'; -import { value as usedValue4 } from 'pure-false-external-true-pureint-false-pureext-false-used-import'; +import 'pure-false-pureint-true'; +import { value as unusedValue4 } from 'pure-false-pureint-true-unused-import'; +import { value as usedValue4 } from 'pure-false-pureint-true-used-import'; -import 'pure-null-external-true-pureint-false-pureext-false'; -import { value as unusedValue5 } from 'pure-null-external-true-pureint-false-pureext-false-unused-import'; -import { value as usedValue5 } from 'pure-null-external-true-pureint-false-pureext-false-used-import'; +import 'pure-null-pureint-true'; +import { value as unusedValue5 } from 'pure-null-pureint-true-unused-import'; +import { value as usedValue5 } from 'pure-null-pureint-true-used-import'; -import 'pure-true-external-true-pureint-false-pureext-false'; -import { value as unusedValue6 } from 'pure-true-external-true-pureint-false-pureext-false-unused-import'; -import { value as usedValue6 } from 'pure-true-external-true-pureint-false-pureext-false-used-import'; +import 'pure-true-pureint-true'; +import { value as unusedValue6 } from 'pure-true-pureint-true-unused-import'; +import { value as usedValue6 } from 'pure-true-pureint-true-used-import'; -import 'pure-false-external-false-pureint-true-pureext-false'; -import { value as unusedValue7 } from 'pure-false-external-false-pureint-true-pureext-false-unused-import'; -import { value as usedValue7 } from 'pure-false-external-false-pureint-true-pureext-false-used-import'; - -import 'pure-null-external-false-pureint-true-pureext-false'; -import { value as unusedValue8 } from 'pure-null-external-false-pureint-true-pureext-false-unused-import'; -import { value as usedValue8 } from 'pure-null-external-false-pureint-true-pureext-false-used-import'; - -import 'pure-true-external-false-pureint-true-pureext-false'; -import { value as unusedValue9 } from 'pure-true-external-false-pureint-true-pureext-false-unused-import'; -import { value as usedValue9 } from 'pure-true-external-false-pureint-true-pureext-false-used-import'; - -import 'pure-false-external-true-pureint-true-pureext-false'; -import { value as unusedValue10 } from 'pure-false-external-true-pureint-true-pureext-false-unused-import'; -import { value as usedValue10 } from 'pure-false-external-true-pureint-true-pureext-false-used-import'; - -import 'pure-null-external-true-pureint-true-pureext-false'; -import { value as unusedValue11 } from 'pure-null-external-true-pureint-true-pureext-false-unused-import'; -import { value as usedValue11 } from 'pure-null-external-true-pureint-true-pureext-false-used-import'; - -import 'pure-true-external-true-pureint-true-pureext-false'; -import { value as unusedValue12 } from 'pure-true-external-true-pureint-true-pureext-false-unused-import'; -import { value as usedValue12 } from 'pure-true-external-true-pureint-true-pureext-false-used-import'; - -import 'pure-false-external-false-pureint-false-pureext-true'; -import { value as unusedValue13 } from 'pure-false-external-false-pureint-false-pureext-true-unused-import'; -import { value as usedValue13 } from 'pure-false-external-false-pureint-false-pureext-true-used-import'; - -import 'pure-null-external-false-pureint-false-pureext-true'; -import { value as unusedValue14 } from 'pure-null-external-false-pureint-false-pureext-true-unused-import'; -import { value as usedValue14 } from 'pure-null-external-false-pureint-false-pureext-true-used-import'; - -import 'pure-true-external-false-pureint-false-pureext-true'; -import { value as unusedValue15 } from 'pure-true-external-false-pureint-false-pureext-true-unused-import'; -import { value as usedValue15 } from 'pure-true-external-false-pureint-false-pureext-true-used-import'; - -import 'pure-false-external-true-pureint-false-pureext-true'; -import { value as unusedValue16 } from 'pure-false-external-true-pureint-false-pureext-true-unused-import'; -import { value as usedValue16 } from 'pure-false-external-true-pureint-false-pureext-true-used-import'; - -import 'pure-null-external-true-pureint-false-pureext-true'; -import { value as unusedValue17 } from 'pure-null-external-true-pureint-false-pureext-true-unused-import'; -import { value as usedValue17 } from 'pure-null-external-true-pureint-false-pureext-true-used-import'; - -import 'pure-true-external-true-pureint-false-pureext-true'; -import { value as unusedValue18 } from 'pure-true-external-true-pureint-false-pureext-true-unused-import'; -import { value as usedValue18 } from 'pure-true-external-true-pureint-false-pureext-true-used-import'; - -import 'pure-false-external-false-pureint-true-pureext-true'; -import { value as unusedValue19 } from 'pure-false-external-false-pureint-true-pureext-true-unused-import'; -import { value as usedValue19 } from 'pure-false-external-false-pureint-true-pureext-true-used-import'; - -import 'pure-null-external-false-pureint-true-pureext-true'; -import { value as unusedValue20 } from 'pure-null-external-false-pureint-true-pureext-true-unused-import'; -import { value as usedValue20 } from 'pure-null-external-false-pureint-true-pureext-true-used-import'; - -import 'pure-true-external-false-pureint-true-pureext-true'; -import { value as unusedValue21 } from 'pure-true-external-false-pureint-true-pureext-true-unused-import'; -import { value as usedValue21 } from 'pure-true-external-false-pureint-true-pureext-true-used-import'; - -import 'pure-false-external-true-pureint-true-pureext-true'; -import { value as unusedValue22 } from 'pure-false-external-true-pureint-true-pureext-true-unused-import'; -import { value as usedValue22 } from 'pure-false-external-true-pureint-true-pureext-true-used-import'; - -import 'pure-null-external-true-pureint-true-pureext-true'; -import { value as unusedValue23 } from 'pure-null-external-true-pureint-true-pureext-true-unused-import'; -import { value as usedValue23 } from 'pure-null-external-true-pureint-true-pureext-true-used-import'; - -import 'pure-true-external-true-pureint-true-pureext-true'; -import { value as unusedValue24 } from 'pure-true-external-true-pureint-true-pureext-true-unused-import'; -import { value as usedValue24 } from 'pure-true-external-true-pureint-true-pureext-true-used-import'; - -export const values = [ - usedValue1, - usedValue2, - usedValue3, - usedValue4, - usedValue5, - usedValue6, - usedValue7, - usedValue8, - usedValue9, - usedValue10, - usedValue11, - usedValue12, - usedValue13, - usedValue14, - usedValue15, - usedValue16, - usedValue17, - usedValue18, - usedValue19, - usedValue20, - usedValue21, - usedValue22, - usedValue23, - usedValue24 -]; +export const values = [usedValue1, usedValue2, usedValue3, usedValue4, usedValue5, usedValue6]; From 3ef7d3620c0a6c8f87208a8479f375373fe79736 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Mon, 6 May 2019 07:58:31 +0200 Subject: [PATCH 08/19] Switch form pure to moduleSideEffects --- src/Chunk.ts | 2 +- src/ExternalModule.ts | 6 +- src/Module.ts | 6 +- src/ModuleLoader.ts | 31 ++++---- src/rollup/types.d.ts | 6 +- src/utils/pluginDriver.ts | 4 +- src/utils/traverseStaticDependencies.ts | 2 +- .../samples/context-resolve/_config.js | 26 ++++--- .../plugin-module-information/_config.js | 16 ++--- .../resolve-id-external/_config.js | 64 ++++++++--------- .../pure-modules/resolve-id-external/main.js | 36 +++++----- .../pure-modules/resolve-id/_config.js | 71 ++++++++++--------- .../samples/pure-modules/resolve-id/main.js | 36 +++++----- test/incremental/index.js | 4 +- 14 files changed, 160 insertions(+), 150 deletions(-) diff --git a/src/Chunk.ts b/src/Chunk.ts index 2e44c5ee294..f27ed7e1ee5 100644 --- a/src/Chunk.ts +++ b/src/Chunk.ts @@ -753,7 +753,7 @@ export default class Chunk { if (depModule instanceof Module) { dependency = depModule.chunk; } else { - if (!depModule.used && depModule.pure) { + if (!(depModule.used || depModule.moduleSideEffects)) { continue; } dependency = depModule; diff --git a/src/ExternalModule.ts b/src/ExternalModule.ts index c787c050348..3086891b7f9 100644 --- a/src/ExternalModule.ts +++ b/src/ExternalModule.ts @@ -13,9 +13,9 @@ export default class ExternalModule { exportsNamespace = false; id: string; isExternal = true; + moduleSideEffects: boolean; mostCommonSuggestion = 0; nameSuggestions: { [name: string]: number }; - pure: boolean; reexported = false; renderPath: string = undefined; renormalizeRenderPath = false; @@ -24,11 +24,11 @@ export default class ExternalModule { private graph: Graph; - constructor(graph: Graph, id: string, pure: boolean) { + constructor(graph: Graph, id: string, moduleSideEffects: boolean) { this.graph = graph; this.id = id; this.execIndex = Infinity; - this.pure = pure; + this.moduleSideEffects = moduleSideEffects; const parts = id.split(/[\\/]/); this.variableName = makeLegal(parts.pop()); diff --git a/src/Module.ts b/src/Module.ts index 347714e7147..f7d676c4ca4 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -192,9 +192,9 @@ export default class Module { isExternal: false; isUserDefinedEntryPoint = false; manualChunkAlias: string = null; + moduleSideEffects: boolean; originalCode: string; originalSourcemap: RawSourceMap | void; - pure: boolean; reexports: { [name: string]: ReexportDescription } = Object.create(null); resolvedIds: ResolvedIdMap; scope: ModuleScope; @@ -212,12 +212,12 @@ export default class Module { private namespaceVariable: NamespaceVariable = undefined; private transformDependencies: string[]; - constructor(graph: Graph, id: string, pure: boolean) { + constructor(graph: Graph, id: string, moduleSideEffects: boolean) { this.id = id; this.graph = graph; this.excludeFromSourcemap = /\0/.test(id); this.context = graph.getModuleContext(id); - this.pure = pure; + this.moduleSideEffects = moduleSideEffects; } basename() { diff --git a/src/ModuleLoader.ts b/src/ModuleLoader.ts index f4bbd3173a4..bfdef4bca07 100644 --- a/src/ModuleLoader.ts +++ b/src/ModuleLoader.ts @@ -241,14 +241,14 @@ export class ModuleLoader { ).then(() => fetchDynamicImportsPromise); } - private fetchModule(id: string, importer: string, pure: boolean): Promise { + private fetchModule(id: string, importer: string, moduleSideEffects: boolean): Promise { const existingModule = this.modulesById.get(id); if (existingModule) { if (existingModule.isExternal) throw new Error(`Cannot fetch external module ${id}`); return Promise.resolve(existingModule); } - const module: Module = new Module(this.graph, id, pure); + const module: Module = new Module(this.graph, id, moduleSideEffects); this.modulesById.set(id, module); const manualChunkAlias = this.getManualChunk(id); if (typeof manualChunkAlias === 'string') { @@ -336,7 +336,7 @@ export class ModuleLoader { if (!this.modulesById.has(resolvedId.id)) { this.modulesById.set( resolvedId.id, - new ExternalModule(this.graph, resolvedId.id, resolvedId.pure) + new ExternalModule(this.graph, resolvedId.id, resolvedId.moduleSideEffects) ); } @@ -346,7 +346,7 @@ export class ModuleLoader { } return Promise.resolve(externalModule); } else { - return this.fetchModule(resolvedId.id, importer, resolvedId.pure); + return this.fetchModule(resolvedId.id, importer, resolvedId.moduleSideEffects); } } @@ -360,7 +360,8 @@ export class ModuleLoader { error(errUnresolvedImport(source, importer)); } this.graph.warn(errUnresolvedImportTreatedAsExternal(source, importer)); - return { id: source, external: true, pure: false }; + // TODO Lukas use proper default from option + return { id: source, external: true, moduleSideEffects: true }; } return resolvedId; } @@ -379,11 +380,9 @@ export class ModuleLoader { resolveIdResult && typeof resolveIdResult === 'object' ? resolveIdResult.id : resolveIdResult; - const pure = - (resolveIdResult && typeof resolveIdResult === 'object' && resolveIdResult.pure) || false; if (typeof id === 'string') { - return this.fetchModule(id, undefined, pure).then(module => { + return this.fetchModule(id, undefined, true).then(module => { if (alias !== null) { if (module.chunkAlias !== null && module.chunkAlias !== alias) { error(errCannotAssignModuleToChunk(module.id, alias, module.chunkAlias)); @@ -403,15 +402,15 @@ export class ModuleLoader { ): ResolvedId | null { let id = ''; let external = false; - let pure = null; + let moduleSideEffects = null; if (resolveIdResult) { if (typeof resolveIdResult === 'object') { id = resolveIdResult.id; if (resolveIdResult.external) { external = true; } - if (typeof resolveIdResult.pure === 'boolean') { - pure = resolveIdResult.pure; + if (typeof resolveIdResult.moduleSideEffects === 'boolean') { + moduleSideEffects = resolveIdResult.moduleSideEffects; } } else { id = resolveIdResult; @@ -432,10 +431,10 @@ export class ModuleLoader { return { external, id, - pure: - pure === null - ? (external ? this.isPureExternalModule : this.isPureInternalModule)(id) || false - : pure + moduleSideEffects: + moduleSideEffects === null + ? !(external ? this.isPureExternalModule : this.isPureInternalModule)(id) + : moduleSideEffects }; } @@ -471,7 +470,7 @@ export class ModuleLoader { } return { external: false, - pure: false, + moduleSideEffects: true, ...resolution }; } diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index c3f5f329e86..7c2d4e8dd28 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -117,10 +117,10 @@ export interface PluginContext extends MinimalPluginContext { getModuleInfo: ( moduleId: string ) => { + hasModuleSideEffects: boolean; id: string; importedIds: string[]; isExternal: boolean; - isPure: boolean; }; /** @deprecated */ isExternal: IsExternal; @@ -142,7 +142,7 @@ export interface PluginContextMeta { export interface ResolvedId { external: boolean; id: string; - pure: boolean; + moduleSideEffects: boolean; } export interface ResolvedIdMap { @@ -152,7 +152,7 @@ export interface ResolvedIdMap { interface PartialResolvedId { external?: boolean; id: string; - pure?: boolean | null; + moduleSideEffects?: boolean | null; } export type ResolveIdResult = string | false | void | PartialResolvedId; diff --git a/src/utils/pluginDriver.ts b/src/utils/pluginDriver.ts index 0e8c9dbe7ec..a08f934c21b 100644 --- a/src/utils/pluginDriver.ts +++ b/src/utils/pluginDriver.ts @@ -174,12 +174,12 @@ export function createPluginDriver( } return { + hasModuleSideEffects: foundModule.moduleSideEffects, id: foundModule.id, importedIds: foundModule.isExternal ? [] : (foundModule as Module).sources.map(id => (foundModule as Module).resolvedIds[id].id), - isExternal: !!foundModule.isExternal, - isPure: foundModule.pure + isExternal: !!foundModule.isExternal }; }, meta: { diff --git a/src/utils/traverseStaticDependencies.ts b/src/utils/traverseStaticDependencies.ts index 8d9e4a7b7cd..e8cc8345308 100644 --- a/src/utils/traverseStaticDependencies.ts +++ b/src/utils/traverseStaticDependencies.ts @@ -11,7 +11,7 @@ export function markModuleAndImpureDependenciesAsExecuted(baseModule: Module) { if ( !(dependency instanceof ExternalModule) && !dependency.isExecuted && - !dependency.pure && + dependency.moduleSideEffects && !visitedModules[dependency.id] ) { dependency.isExecuted = true; diff --git a/test/function/samples/context-resolve/_config.js b/test/function/samples/context-resolve/_config.js index 38ac7c3be67..88aed16c12a 100644 --- a/test/function/samples/context-resolve/_config.js +++ b/test/function/samples/context-resolve/_config.js @@ -4,7 +4,11 @@ const assert = require('assert'); const tests = [ { source: './existing', - expected: { id: path.resolve(__dirname, 'existing.js'), external: false, pure: false } + expected: { + id: path.resolve(__dirname, 'existing.js'), + external: false, + moduleSideEffects: true + } }, { source: './missing-relative', @@ -19,7 +23,7 @@ const tests = [ expected: { id: path.resolve(__dirname, 'marked-directly-external-relative'), external: true, - pure: false + moduleSideEffects: true } }, { @@ -27,32 +31,36 @@ const tests = [ expected: { id: path.resolve(__dirname, 'marked-external-relative'), external: true, - pure: false + moduleSideEffects: true } }, { source: 'marked-external-absolute', - expected: { id: 'marked-external-absolute', external: true, pure: false } + expected: { id: 'marked-external-absolute', external: true, moduleSideEffects: true } }, { source: 'resolved-name', - expected: { id: 'resolved:resolved-name', external: false, pure: false } + expected: { id: 'resolved:resolved-name', external: false, moduleSideEffects: true } }, { source: 'resolved-false', - expected: { id: 'resolved-false', external: true, pure: false } + expected: { id: 'resolved-false', external: true, moduleSideEffects: true } }, { source: 'resolved-object', - expected: { id: 'resolved:resolved-object', external: false, pure: false } + expected: { id: 'resolved:resolved-object', external: false, moduleSideEffects: true } }, { source: 'resolved-object-non-external', - expected: { id: 'resolved:resolved-object-non-external', external: false, pure: false } + expected: { + id: 'resolved:resolved-object-non-external', + external: false, + moduleSideEffects: true + } }, { source: 'resolved-object-external', - expected: { id: 'resolved:resolved-object-external', external: true, pure: false } + expected: { id: 'resolved:resolved-object-external', external: true, moduleSideEffects: true } } ]; diff --git a/test/function/samples/plugin-module-information/_config.js b/test/function/samples/plugin-module-information/_config.js index a8e2188ee89..9ba23d507c4 100644 --- a/test/function/samples/plugin-module-information/_config.js +++ b/test/function/samples/plugin-module-information/_config.js @@ -18,28 +18,28 @@ module.exports = { rendered = true; assert.deepEqual(Array.from(this.moduleIds), [ID_MAIN, ID_FOO, ID_NESTED, ID_PATH]); assert.deepEqual(this.getModuleInfo(ID_MAIN), { + hasModuleSideEffects: true, id: ID_MAIN, importedIds: [ID_FOO, ID_NESTED], - isExternal: false, - isPure: false + isExternal: false }); assert.deepEqual(this.getModuleInfo(ID_FOO), { + hasModuleSideEffects: true, id: ID_FOO, importedIds: [ID_PATH], - isExternal: false, - isPure: false + isExternal: false }); assert.deepEqual(this.getModuleInfo(ID_NESTED), { + hasModuleSideEffects: true, id: ID_NESTED, importedIds: [ID_FOO], - isExternal: false, - isPure: false + isExternal: false }); assert.deepEqual(this.getModuleInfo(ID_PATH), { + hasModuleSideEffects: true, id: ID_PATH, importedIds: [], - isExternal: true, - isPure: false + isExternal: true }); } } diff --git a/test/function/samples/pure-modules/resolve-id-external/_config.js b/test/function/samples/pure-modules/resolve-id-external/_config.js index c0fab465153..f937ce66617 100644 --- a/test/function/samples/pure-modules/resolve-id-external/_config.js +++ b/test/function/samples/pure-modules/resolve-id-external/_config.js @@ -11,18 +11,18 @@ module.exports = { }, exports() { assert.deepStrictEqual(sideEffects, [ - 'pure-false-pureext-false', - 'pure-false-pureext-false-unused-import', - 'pure-false-pureext-false-used-import', - 'pure-null-pureext-false', - 'pure-null-pureext-false-unused-import', - 'pure-null-pureext-false-used-import', - 'pure-true-pureext-false-used-import', - 'pure-false-pureext-true', - 'pure-false-pureext-true-unused-import', - 'pure-false-pureext-true-used-import', - 'pure-null-pureext-true-used-import', - 'pure-true-pureext-true-used-import' + 'sideeffects-false-pureext-false-used-import', + 'sideeffects-null-pureext-false', + 'sideeffects-null-pureext-false-unused-import', + 'sideeffects-null-pureext-false-used-import', + 'sideeffects-true-pureext-false', + 'sideeffects-true-pureext-false-unused-import', + 'sideeffects-true-pureext-false-used-import', + 'sideeffects-false-pureext-true-used-import', + 'sideeffects-null-pureext-true-used-import', + 'sideeffects-true-pureext-true', + 'sideeffects-true-pureext-true-unused-import', + 'sideeffects-true-pureext-true-used-import' ]); }, options: { @@ -38,7 +38,7 @@ module.exports = { return { id, external: true, - pure: JSON.parse(id.split('-')[1]) + moduleSideEffects: JSON.parse(id.split('-')[1]) }; } }, @@ -47,26 +47,26 @@ module.exports = { Array.from(this.moduleIds) .filter(id => id[0] !== '/') .sort() - .map(id => ({ id, isPure: this.getModuleInfo(id).isPure })), + .map(id => ({ id, hasModuleSideEffects: this.getModuleInfo(id).hasModuleSideEffects })), [ - { id: 'pure-false-pureext-false', isPure: false }, - { id: 'pure-false-pureext-false-unused-import', isPure: false }, - { id: 'pure-false-pureext-false-used-import', isPure: false }, - { id: 'pure-false-pureext-true', isPure: false }, - { id: 'pure-false-pureext-true-unused-import', isPure: false }, - { id: 'pure-false-pureext-true-used-import', isPure: false }, - { id: 'pure-null-pureext-false', isPure: false }, - { id: 'pure-null-pureext-false-unused-import', isPure: false }, - { id: 'pure-null-pureext-false-used-import', isPure: false }, - { id: 'pure-null-pureext-true', isPure: true }, - { id: 'pure-null-pureext-true-unused-import', isPure: true }, - { id: 'pure-null-pureext-true-used-import', isPure: true }, - { id: 'pure-true-pureext-false', isPure: true }, - { id: 'pure-true-pureext-false-unused-import', isPure: true }, - { id: 'pure-true-pureext-false-used-import', isPure: true }, - { id: 'pure-true-pureext-true', isPure: true }, - { id: 'pure-true-pureext-true-unused-import', isPure: true }, - { id: 'pure-true-pureext-true-used-import', isPure: true } + { id: 'sideeffects-false-pureext-false', hasModuleSideEffects: false }, + { id: 'sideeffects-false-pureext-false-unused-import', hasModuleSideEffects: false }, + { id: 'sideeffects-false-pureext-false-used-import', hasModuleSideEffects: false }, + { id: 'sideeffects-false-pureext-true', hasModuleSideEffects: false }, + { id: 'sideeffects-false-pureext-true-unused-import', hasModuleSideEffects: false }, + { id: 'sideeffects-false-pureext-true-used-import', hasModuleSideEffects: false }, + { id: 'sideeffects-null-pureext-false', hasModuleSideEffects: true }, + { id: 'sideeffects-null-pureext-false-unused-import', hasModuleSideEffects: true }, + { id: 'sideeffects-null-pureext-false-used-import', hasModuleSideEffects: true }, + { id: 'sideeffects-null-pureext-true', hasModuleSideEffects: false }, + { id: 'sideeffects-null-pureext-true-unused-import', hasModuleSideEffects: false }, + { id: 'sideeffects-null-pureext-true-used-import', hasModuleSideEffects: false }, + { id: 'sideeffects-true-pureext-false', hasModuleSideEffects: true }, + { id: 'sideeffects-true-pureext-false-unused-import', hasModuleSideEffects: true }, + { id: 'sideeffects-true-pureext-false-used-import', hasModuleSideEffects: true }, + { id: 'sideeffects-true-pureext-true', hasModuleSideEffects: true }, + { id: 'sideeffects-true-pureext-true-unused-import', hasModuleSideEffects: true }, + { id: 'sideeffects-true-pureext-true-used-import', hasModuleSideEffects: true } ] ); } diff --git a/test/function/samples/pure-modules/resolve-id-external/main.js b/test/function/samples/pure-modules/resolve-id-external/main.js index 82aba2a8f81..41824345f60 100644 --- a/test/function/samples/pure-modules/resolve-id-external/main.js +++ b/test/function/samples/pure-modules/resolve-id-external/main.js @@ -1,25 +1,25 @@ -import 'pure-false-pureext-false'; -import { value as unusedValue1 } from 'pure-false-pureext-false-unused-import'; -import { value as usedValue1 } from 'pure-false-pureext-false-used-import'; +import 'sideeffects-false-pureext-false'; +import { value as unusedValue1 } from 'sideeffects-false-pureext-false-unused-import'; +import { value as usedValue1 } from 'sideeffects-false-pureext-false-used-import'; -import 'pure-null-pureext-false'; -import { value as unusedValue2 } from 'pure-null-pureext-false-unused-import'; -import { value as usedValue2 } from 'pure-null-pureext-false-used-import'; +import 'sideeffects-null-pureext-false'; +import { value as unusedValue2 } from 'sideeffects-null-pureext-false-unused-import'; +import { value as usedValue2 } from 'sideeffects-null-pureext-false-used-import'; -import 'pure-true-pureext-false'; -import { value as unusedValue3 } from 'pure-true-pureext-false-unused-import'; -import { value as usedValue3 } from 'pure-true-pureext-false-used-import'; +import 'sideeffects-true-pureext-false'; +import { value as unusedValue3 } from 'sideeffects-true-pureext-false-unused-import'; +import { value as usedValue3 } from 'sideeffects-true-pureext-false-used-import'; -import 'pure-false-pureext-true'; -import { value as unusedValue4 } from 'pure-false-pureext-true-unused-import'; -import { value as usedValue4 } from 'pure-false-pureext-true-used-import'; +import 'sideeffects-false-pureext-true'; +import { value as unusedValue4 } from 'sideeffects-false-pureext-true-unused-import'; +import { value as usedValue4 } from 'sideeffects-false-pureext-true-used-import'; -import 'pure-null-pureext-true'; -import { value as unusedValue5 } from 'pure-null-pureext-true-unused-import'; -import { value as usedValue5 } from 'pure-null-pureext-true-used-import'; +import 'sideeffects-null-pureext-true'; +import { value as unusedValue5 } from 'sideeffects-null-pureext-true-unused-import'; +import { value as usedValue5 } from 'sideeffects-null-pureext-true-used-import'; -import 'pure-true-pureext-true'; -import { value as unusedValue6 } from 'pure-true-pureext-true-unused-import'; -import { value as usedValue6 } from 'pure-true-pureext-true-used-import'; +import 'sideeffects-true-pureext-true'; +import { value as unusedValue6 } from 'sideeffects-true-pureext-true-unused-import'; +import { value as usedValue6 } from 'sideeffects-true-pureext-true-used-import'; export const values = [usedValue1, usedValue2, usedValue3, usedValue4, usedValue5, usedValue6]; diff --git a/test/function/samples/pure-modules/resolve-id/_config.js b/test/function/samples/pure-modules/resolve-id/_config.js index ad0ce7d3f6d..87e1f946bf0 100644 --- a/test/function/samples/pure-modules/resolve-id/_config.js +++ b/test/function/samples/pure-modules/resolve-id/_config.js @@ -9,18 +9,18 @@ module.exports = { }, exports() { assert.deepStrictEqual(sideEffects, [ - 'pure-false-pureint-false', - 'pure-false-pureint-false-unused-import', - 'pure-false-pureint-false-used-import', - 'pure-null-pureint-false', - 'pure-null-pureint-false-unused-import', - 'pure-null-pureint-false-used-import', - 'pure-true-pureint-false-used-import', - 'pure-false-pureint-true', - 'pure-false-pureint-true-unused-import', - 'pure-false-pureint-true-used-import', - 'pure-null-pureint-true-used-import', - 'pure-true-pureint-true-used-import' + 'sideeffects-false-pureint-false-used-import', + 'sideeffects-null-pureint-false', + 'sideeffects-null-pureint-false-unused-import', + 'sideeffects-null-pureint-false-used-import', + 'sideeffects-true-pureint-false', + 'sideeffects-true-pureint-false-unused-import', + 'sideeffects-true-pureint-false-used-import', + 'sideeffects-false-pureint-true-used-import', + 'sideeffects-null-pureint-true-used-import', + 'sideeffects-true-pureint-true', + 'sideeffects-true-pureint-true-unused-import', + 'sideeffects-true-pureint-true-used-import' ]); }, options: { @@ -36,15 +36,18 @@ module.exports = { return { id, external: false, - pure: JSON.parse(id.split('-')[1]) + moduleSideEffects: JSON.parse(id.split('-')[1]) }; } }, load(id) { if (id[0] !== '/') { - const pure = JSON.parse(id.split('-')[1]); + const sideEffects = JSON.parse(id.split('-')[1]); const pureInt = JSON.parse(id.split('-')[3]); - assert.strictEqual(this.getModuleInfo(id).isPure, pure === null ? pureInt : pure); + assert.strictEqual( + this.getModuleInfo(id).hasModuleSideEffects, + sideEffects === null ? !pureInt : sideEffects + ); return `export const value = '${id}'; sideEffects.push(value);`; } }, @@ -53,26 +56,26 @@ module.exports = { Array.from(this.moduleIds) .filter(id => id[0] !== '/') .sort() - .map(id => ({ id, isPure: this.getModuleInfo(id).isPure })), + .map(id => ({ id, hasModuleSideEffects: this.getModuleInfo(id).hasModuleSideEffects })), [ - { id: 'pure-false-pureint-false', isPure: false }, - { id: 'pure-false-pureint-false-unused-import', isPure: false }, - { id: 'pure-false-pureint-false-used-import', isPure: false }, - { id: 'pure-false-pureint-true', isPure: false }, - { id: 'pure-false-pureint-true-unused-import', isPure: false }, - { id: 'pure-false-pureint-true-used-import', isPure: false }, - { id: 'pure-null-pureint-false', isPure: false }, - { id: 'pure-null-pureint-false-unused-import', isPure: false }, - { id: 'pure-null-pureint-false-used-import', isPure: false }, - { id: 'pure-null-pureint-true', isPure: true }, - { id: 'pure-null-pureint-true-unused-import', isPure: true }, - { id: 'pure-null-pureint-true-used-import', isPure: true }, - { id: 'pure-true-pureint-false', isPure: true }, - { id: 'pure-true-pureint-false-unused-import', isPure: true }, - { id: 'pure-true-pureint-false-used-import', isPure: true }, - { id: 'pure-true-pureint-true', isPure: true }, - { id: 'pure-true-pureint-true-unused-import', isPure: true }, - { id: 'pure-true-pureint-true-used-import', isPure: true } + { id: 'sideeffects-false-pureint-false', hasModuleSideEffects: false }, + { id: 'sideeffects-false-pureint-false-unused-import', hasModuleSideEffects: false }, + { id: 'sideeffects-false-pureint-false-used-import', hasModuleSideEffects: false }, + { id: 'sideeffects-false-pureint-true', hasModuleSideEffects: false }, + { id: 'sideeffects-false-pureint-true-unused-import', hasModuleSideEffects: false }, + { id: 'sideeffects-false-pureint-true-used-import', hasModuleSideEffects: false }, + { id: 'sideeffects-null-pureint-false', hasModuleSideEffects: true }, + { id: 'sideeffects-null-pureint-false-unused-import', hasModuleSideEffects: true }, + { id: 'sideeffects-null-pureint-false-used-import', hasModuleSideEffects: true }, + { id: 'sideeffects-null-pureint-true', hasModuleSideEffects: false }, + { id: 'sideeffects-null-pureint-true-unused-import', hasModuleSideEffects: false }, + { id: 'sideeffects-null-pureint-true-used-import', hasModuleSideEffects: false }, + { id: 'sideeffects-true-pureint-false', hasModuleSideEffects: true }, + { id: 'sideeffects-true-pureint-false-unused-import', hasModuleSideEffects: true }, + { id: 'sideeffects-true-pureint-false-used-import', hasModuleSideEffects: true }, + { id: 'sideeffects-true-pureint-true', hasModuleSideEffects: true }, + { id: 'sideeffects-true-pureint-true-unused-import', hasModuleSideEffects: true }, + { id: 'sideeffects-true-pureint-true-used-import', hasModuleSideEffects: true } ] ); } diff --git a/test/function/samples/pure-modules/resolve-id/main.js b/test/function/samples/pure-modules/resolve-id/main.js index 912110cff64..7bd7fc1b69f 100644 --- a/test/function/samples/pure-modules/resolve-id/main.js +++ b/test/function/samples/pure-modules/resolve-id/main.js @@ -1,25 +1,25 @@ -import 'pure-false-pureint-false'; -import { value as unusedValue1 } from 'pure-false-pureint-false-unused-import'; -import { value as usedValue1 } from 'pure-false-pureint-false-used-import'; +import 'sideeffects-false-pureint-false'; +import { value as unusedValue1 } from 'sideeffects-false-pureint-false-unused-import'; +import { value as usedValue1 } from 'sideeffects-false-pureint-false-used-import'; -import 'pure-null-pureint-false'; -import { value as unusedValue2 } from 'pure-null-pureint-false-unused-import'; -import { value as usedValue2 } from 'pure-null-pureint-false-used-import'; +import 'sideeffects-null-pureint-false'; +import { value as unusedValue2 } from 'sideeffects-null-pureint-false-unused-import'; +import { value as usedValue2 } from 'sideeffects-null-pureint-false-used-import'; -import 'pure-true-pureint-false'; -import { value as unusedValue3 } from 'pure-true-pureint-false-unused-import'; -import { value as usedValue3 } from 'pure-true-pureint-false-used-import'; +import 'sideeffects-true-pureint-false'; +import { value as unusedValue3 } from 'sideeffects-true-pureint-false-unused-import'; +import { value as usedValue3 } from 'sideeffects-true-pureint-false-used-import'; -import 'pure-false-pureint-true'; -import { value as unusedValue4 } from 'pure-false-pureint-true-unused-import'; -import { value as usedValue4 } from 'pure-false-pureint-true-used-import'; +import 'sideeffects-false-pureint-true'; +import { value as unusedValue4 } from 'sideeffects-false-pureint-true-unused-import'; +import { value as usedValue4 } from 'sideeffects-false-pureint-true-used-import'; -import 'pure-null-pureint-true'; -import { value as unusedValue5 } from 'pure-null-pureint-true-unused-import'; -import { value as usedValue5 } from 'pure-null-pureint-true-used-import'; +import 'sideeffects-null-pureint-true'; +import { value as unusedValue5 } from 'sideeffects-null-pureint-true-unused-import'; +import { value as usedValue5 } from 'sideeffects-null-pureint-true-used-import'; -import 'pure-true-pureint-true'; -import { value as unusedValue6 } from 'pure-true-pureint-true-unused-import'; -import { value as usedValue6 } from 'pure-true-pureint-true-used-import'; +import 'sideeffects-true-pureint-true'; +import { value as unusedValue6 } from 'sideeffects-true-pureint-true-unused-import'; +import { value as usedValue6 } from 'sideeffects-true-pureint-true-used-import'; export const values = [usedValue1, usedValue2, usedValue3, usedValue4, usedValue5, usedValue6]; diff --git a/test/incremental/index.js b/test/incremental/index.js index 0afcbabeec7..ee7a5983208 100644 --- a/test/incremental/index.js +++ b/test/incremental/index.js @@ -240,8 +240,8 @@ describe('incremental', () => { assert.equal(bundle.cache.modules[1].id, 'entry'); assert.deepEqual(bundle.cache.modules[1].resolvedIds, { - foo: { id: 'foo', external: false, pure: false }, - external: { id: 'external', external: true, pure: false } + foo: { id: 'foo', external: false, moduleSideEffects: true }, + external: { id: 'external', external: true, moduleSideEffects: true } }); }); }); From 2f41efcdf63d97d62b0795c58ce9c08409747564 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Tue, 7 May 2019 06:52:04 +0200 Subject: [PATCH 09/19] Replace pureInternalModules with hasModuleSideEffects and refine logic --- src/Graph.ts | 12 ++-- src/ModuleLoader.ts | 57 +++++++++++---- src/rollup/types.d.ts | 7 +- .../resolve-id-external/_config.js | 68 ++++++++++-------- .../pure-modules/resolve-id-external/main.js | 36 +++++----- .../pure-modules/resolve-id/_config.js | 72 ++++++++++--------- .../samples/pure-modules/resolve-id/main.js | 36 +++++----- 7 files changed, 167 insertions(+), 121 deletions(-) diff --git a/src/Graph.ts b/src/Graph.ts index 265dd0b438e..c07b890c9af 100644 --- a/src/Graph.ts +++ b/src/Graph.ts @@ -113,16 +113,16 @@ export default class Graph { this.treeshakingOptions = options.treeshake ? { annotations: (options.treeshake).annotations !== false, + moduleSideEffects: (options.treeshake).moduleSideEffects, propertyReadSideEffects: (options.treeshake).propertyReadSideEffects !== false, - pureExternalModules: (options.treeshake).pureExternalModules, - pureInternalModules: (options.treeshake).pureInternalModules + pureExternalModules: (options.treeshake).pureExternalModules } : { annotations: true, + moduleSideEffects: true, propertyReadSideEffects: true, - pureExternalModules: false, - pureInternalModules: false + pureExternalModules: false }; } @@ -187,8 +187,8 @@ export default class Graph { this.pluginDriver, options.external, typeof options.manualChunks === 'function' && options.manualChunks, - this.treeshake ? this.treeshakingOptions.pureExternalModules : false, - this.treeshake ? this.treeshakingOptions.pureInternalModules : false + this.treeshake ? this.treeshakingOptions.moduleSideEffects : false, + this.treeshake ? this.treeshakingOptions.pureExternalModules : false ); } diff --git a/src/ModuleLoader.ts b/src/ModuleLoader.ts index bfdef4bca07..99001195302 100644 --- a/src/ModuleLoader.ts +++ b/src/ModuleLoader.ts @@ -6,12 +6,13 @@ import { ExternalOption, GetManualChunk, IsExternal, - IsPureModule, ModuleJSON, + ModuleSideEffectsOption, PureModulesOption, ResolvedId, ResolveIdResult, - SourceDescription + SourceDescription, + WarningHandler } from './rollup/types'; import { errBadLoader, @@ -61,6 +62,38 @@ function getIdMatcher>( } } +function getHasModuleSideEffects( + moduleSideEffectsOption: ModuleSideEffectsOption, + pureExternalModules: PureModulesOption, + warn: WarningHandler +): (id: string, external: boolean) => boolean { + if (moduleSideEffectsOption === false) { + return () => false; + } + if (moduleSideEffectsOption === 'no-external') { + // TODO Lukas test + return (_id, external) => !external; + } + if (typeof moduleSideEffectsOption === 'function') { + return (id, ...args) => + !id.startsWith('\0') ? moduleSideEffectsOption(id, ...args) !== false : true; + } + if (Array.isArray(moduleSideEffectsOption)) { + // TODO Lukas test + const ids = new Set(moduleSideEffectsOption); + return id => ids.has(id); + } + if (moduleSideEffectsOption && moduleSideEffectsOption !== true) { + // TODO Lukas test + warn({ + code: 'TODO', + message: 'TODO' + }); + } + const isPureExternalModule = getIdMatcher(pureExternalModules); + return (id, external) => !(external && isPureExternalModule(id)); +} + export class ModuleLoader { readonly isExternal: IsExternal; private readonly entriesByReferenceId = new Map< @@ -70,8 +103,7 @@ export class ModuleLoader { private readonly entryModules: Module[] = []; private readonly getManualChunk: GetManualChunk; private readonly graph: Graph; - private readonly isPureExternalModule: IsPureModule; - private readonly isPureInternalModule: IsPureModule; + private readonly hasModuleSideEffects: (id: string, external: boolean) => boolean; private latestLoadModulesPromise: Promise = Promise.resolve(); private readonly manualChunkModules: Record = {}; private readonly modulesById: Map; @@ -83,15 +115,18 @@ export class ModuleLoader { pluginDriver: PluginDriver, external: ExternalOption, getManualChunk: GetManualChunk | null, - pureExternalModules: PureModulesOption, - pureInternalModules: PureModulesOption + moduleSideEffects: ModuleSideEffectsOption, + pureExternalModules: PureModulesOption ) { this.graph = graph; this.modulesById = modulesById; this.pluginDriver = pluginDriver; this.isExternal = getIdMatcher(external); - this.isPureExternalModule = getIdMatcher(pureExternalModules); - this.isPureInternalModule = getIdMatcher(pureInternalModules); + this.hasModuleSideEffects = getHasModuleSideEffects( + moduleSideEffects, + pureExternalModules, + graph.warn + ); this.getManualChunk = typeof getManualChunk === 'function' ? getManualChunk : () => null; } @@ -360,7 +395,6 @@ export class ModuleLoader { error(errUnresolvedImport(source, importer)); } this.graph.warn(errUnresolvedImportTreatedAsExternal(source, importer)); - // TODO Lukas use proper default from option return { id: source, external: true, moduleSideEffects: true }; } return resolvedId; @@ -431,10 +465,7 @@ export class ModuleLoader { return { external, id, - moduleSideEffects: - moduleSideEffects === null - ? !(external ? this.isPureExternalModule : this.isPureInternalModule)(id) - : moduleSideEffects + moduleSideEffects: moduleSideEffects || this.hasModuleSideEffects(id, external) }; } diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index 7c2d4e8dd28..588ba7ab199 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -167,6 +167,8 @@ export type IsExternal = (source: string, importer: string, isResolved: boolean) export type IsPureModule = (id: string) => boolean | void; +export type HasModuleSideEffects = (id: string, external: boolean) => boolean | void; + export type LoadHook = ( this: PluginContext, id: string @@ -318,11 +320,11 @@ export interface Plugin extends PluginHooks { export interface TreeshakingOptions { annotations?: boolean; + // TODO Lukas adjust docs + moduleSideEffects?: ModuleSideEffectsOption; propertyReadSideEffects?: boolean; // TODO Lukas adjust docs pureExternalModules?: PureModulesOption; - // TODO Lukas adjust docs - pureInternalModules?: PureModulesOption; } export type GetManualChunk = (id: string) => string | void; @@ -333,6 +335,7 @@ export type PureModulesOption = boolean | string[] | IsPureModule; export type GlobalsOption = { [name: string]: string } | ((name: string) => string); export type InputOption = string | string[] | { [entryAlias: string]: string }; export type ManualChunksOption = { [chunkAlias: string]: string[] } | GetManualChunk; +export type ModuleSideEffectsOption = boolean | 'no-external' | string[] | HasModuleSideEffects; export interface InputOptions { acorn?: any; diff --git a/test/function/samples/pure-modules/resolve-id-external/_config.js b/test/function/samples/pure-modules/resolve-id-external/_config.js index f937ce66617..05a3347a2d6 100644 --- a/test/function/samples/pure-modules/resolve-id-external/_config.js +++ b/test/function/samples/pure-modules/resolve-id-external/_config.js @@ -2,6 +2,7 @@ const assert = require('assert'); const sideEffects = []; module.exports = { + solo: true, description: 'does not include modules without used exports if moduleSideEffect is false', context: { require(id) { @@ -11,23 +12,25 @@ module.exports = { }, exports() { assert.deepStrictEqual(sideEffects, [ - 'sideeffects-false-pureext-false-used-import', - 'sideeffects-null-pureext-false', - 'sideeffects-null-pureext-false-unused-import', - 'sideeffects-null-pureext-false-used-import', - 'sideeffects-true-pureext-false', - 'sideeffects-true-pureext-false-unused-import', - 'sideeffects-true-pureext-false-used-import', - 'sideeffects-false-pureext-true-used-import', - 'sideeffects-null-pureext-true-used-import', - 'sideeffects-true-pureext-true', - 'sideeffects-true-pureext-true-unused-import', - 'sideeffects-true-pureext-true-used-import' + 'sideeffects-false-usereffects-false-used-import', + 'sideeffects-null-usereffects-false-used-import', + 'sideeffects-true-usereffects-false', + 'sideeffects-true-usereffects-false-unused-import', + 'sideeffects-true-usereffects-false-used-import', + 'sideeffects-false-usereffects-true', + 'sideeffects-false-usereffects-true-unused-import', + 'sideeffects-false-usereffects-true-used-import', + 'sideeffects-null-usereffects-true', + 'sideeffects-null-usereffects-true-unused-import', + 'sideeffects-null-usereffects-true-used-import', + 'sideeffects-true-usereffects-true', + 'sideeffects-true-usereffects-true-unused-import', + 'sideeffects-true-usereffects-true-used-import' ]); }, options: { treeshake: { - pureExternalModules(id) { + moduleSideEffects(id) { return JSON.parse(id.split('-')[3]); } }, @@ -49,24 +52,27 @@ module.exports = { .sort() .map(id => ({ id, hasModuleSideEffects: this.getModuleInfo(id).hasModuleSideEffects })), [ - { id: 'sideeffects-false-pureext-false', hasModuleSideEffects: false }, - { id: 'sideeffects-false-pureext-false-unused-import', hasModuleSideEffects: false }, - { id: 'sideeffects-false-pureext-false-used-import', hasModuleSideEffects: false }, - { id: 'sideeffects-false-pureext-true', hasModuleSideEffects: false }, - { id: 'sideeffects-false-pureext-true-unused-import', hasModuleSideEffects: false }, - { id: 'sideeffects-false-pureext-true-used-import', hasModuleSideEffects: false }, - { id: 'sideeffects-null-pureext-false', hasModuleSideEffects: true }, - { id: 'sideeffects-null-pureext-false-unused-import', hasModuleSideEffects: true }, - { id: 'sideeffects-null-pureext-false-used-import', hasModuleSideEffects: true }, - { id: 'sideeffects-null-pureext-true', hasModuleSideEffects: false }, - { id: 'sideeffects-null-pureext-true-unused-import', hasModuleSideEffects: false }, - { id: 'sideeffects-null-pureext-true-used-import', hasModuleSideEffects: false }, - { id: 'sideeffects-true-pureext-false', hasModuleSideEffects: true }, - { id: 'sideeffects-true-pureext-false-unused-import', hasModuleSideEffects: true }, - { id: 'sideeffects-true-pureext-false-used-import', hasModuleSideEffects: true }, - { id: 'sideeffects-true-pureext-true', hasModuleSideEffects: true }, - { id: 'sideeffects-true-pureext-true-unused-import', hasModuleSideEffects: true }, - { id: 'sideeffects-true-pureext-true-used-import', hasModuleSideEffects: true } + { id: 'sideeffects-false-usereffects-false', hasModuleSideEffects: false }, + { + id: 'sideeffects-false-usereffects-false-unused-import', + hasModuleSideEffects: false + }, + { id: 'sideeffects-false-usereffects-false-used-import', hasModuleSideEffects: false }, + { id: 'sideeffects-false-usereffects-true', hasModuleSideEffects: true }, + { id: 'sideeffects-false-usereffects-true-unused-import', hasModuleSideEffects: true }, + { id: 'sideeffects-false-usereffects-true-used-import', hasModuleSideEffects: true }, + { id: 'sideeffects-null-usereffects-false', hasModuleSideEffects: false }, + { id: 'sideeffects-null-usereffects-false-unused-import', hasModuleSideEffects: false }, + { id: 'sideeffects-null-usereffects-false-used-import', hasModuleSideEffects: false }, + { id: 'sideeffects-null-usereffects-true', hasModuleSideEffects: true }, + { id: 'sideeffects-null-usereffects-true-unused-import', hasModuleSideEffects: true }, + { id: 'sideeffects-null-usereffects-true-used-import', hasModuleSideEffects: true }, + { id: 'sideeffects-true-usereffects-false', hasModuleSideEffects: true }, + { id: 'sideeffects-true-usereffects-false-unused-import', hasModuleSideEffects: true }, + { id: 'sideeffects-true-usereffects-false-used-import', hasModuleSideEffects: true }, + { id: 'sideeffects-true-usereffects-true', hasModuleSideEffects: true }, + { id: 'sideeffects-true-usereffects-true-unused-import', hasModuleSideEffects: true }, + { id: 'sideeffects-true-usereffects-true-used-import', hasModuleSideEffects: true } ] ); } diff --git a/test/function/samples/pure-modules/resolve-id-external/main.js b/test/function/samples/pure-modules/resolve-id-external/main.js index 41824345f60..dba6f1c07dc 100644 --- a/test/function/samples/pure-modules/resolve-id-external/main.js +++ b/test/function/samples/pure-modules/resolve-id-external/main.js @@ -1,25 +1,25 @@ -import 'sideeffects-false-pureext-false'; -import { value as unusedValue1 } from 'sideeffects-false-pureext-false-unused-import'; -import { value as usedValue1 } from 'sideeffects-false-pureext-false-used-import'; +import 'sideeffects-false-usereffects-false'; +import { value as unusedValue1 } from 'sideeffects-false-usereffects-false-unused-import'; +import { value as usedValue1 } from 'sideeffects-false-usereffects-false-used-import'; -import 'sideeffects-null-pureext-false'; -import { value as unusedValue2 } from 'sideeffects-null-pureext-false-unused-import'; -import { value as usedValue2 } from 'sideeffects-null-pureext-false-used-import'; +import 'sideeffects-null-usereffects-false'; +import { value as unusedValue2 } from 'sideeffects-null-usereffects-false-unused-import'; +import { value as usedValue2 } from 'sideeffects-null-usereffects-false-used-import'; -import 'sideeffects-true-pureext-false'; -import { value as unusedValue3 } from 'sideeffects-true-pureext-false-unused-import'; -import { value as usedValue3 } from 'sideeffects-true-pureext-false-used-import'; +import 'sideeffects-true-usereffects-false'; +import { value as unusedValue3 } from 'sideeffects-true-usereffects-false-unused-import'; +import { value as usedValue3 } from 'sideeffects-true-usereffects-false-used-import'; -import 'sideeffects-false-pureext-true'; -import { value as unusedValue4 } from 'sideeffects-false-pureext-true-unused-import'; -import { value as usedValue4 } from 'sideeffects-false-pureext-true-used-import'; +import 'sideeffects-false-usereffects-true'; +import { value as unusedValue4 } from 'sideeffects-false-usereffects-true-unused-import'; +import { value as usedValue4 } from 'sideeffects-false-usereffects-true-used-import'; -import 'sideeffects-null-pureext-true'; -import { value as unusedValue5 } from 'sideeffects-null-pureext-true-unused-import'; -import { value as usedValue5 } from 'sideeffects-null-pureext-true-used-import'; +import 'sideeffects-null-usereffects-true'; +import { value as unusedValue5 } from 'sideeffects-null-usereffects-true-unused-import'; +import { value as usedValue5 } from 'sideeffects-null-usereffects-true-used-import'; -import 'sideeffects-true-pureext-true'; -import { value as unusedValue6 } from 'sideeffects-true-pureext-true-unused-import'; -import { value as usedValue6 } from 'sideeffects-true-pureext-true-used-import'; +import 'sideeffects-true-usereffects-true'; +import { value as unusedValue6 } from 'sideeffects-true-usereffects-true-unused-import'; +import { value as usedValue6 } from 'sideeffects-true-usereffects-true-used-import'; export const values = [usedValue1, usedValue2, usedValue3, usedValue4, usedValue5, usedValue6]; diff --git a/test/function/samples/pure-modules/resolve-id/_config.js b/test/function/samples/pure-modules/resolve-id/_config.js index 87e1f946bf0..214f36a076c 100644 --- a/test/function/samples/pure-modules/resolve-id/_config.js +++ b/test/function/samples/pure-modules/resolve-id/_config.js @@ -3,29 +3,32 @@ const sideEffects = []; // TODO Lukas implement load and transform hooks module.exports = { + solo: true, description: 'does not include modules without used exports if moduleSideEffect is false', context: { sideEffects }, exports() { assert.deepStrictEqual(sideEffects, [ - 'sideeffects-false-pureint-false-used-import', - 'sideeffects-null-pureint-false', - 'sideeffects-null-pureint-false-unused-import', - 'sideeffects-null-pureint-false-used-import', - 'sideeffects-true-pureint-false', - 'sideeffects-true-pureint-false-unused-import', - 'sideeffects-true-pureint-false-used-import', - 'sideeffects-false-pureint-true-used-import', - 'sideeffects-null-pureint-true-used-import', - 'sideeffects-true-pureint-true', - 'sideeffects-true-pureint-true-unused-import', - 'sideeffects-true-pureint-true-used-import' + 'sideeffects-false-usereffects-false-used-import', + 'sideeffects-null-usereffects-false-used-import', + 'sideeffects-true-usereffects-false', + 'sideeffects-true-usereffects-false-unused-import', + 'sideeffects-true-usereffects-false-used-import', + 'sideeffects-false-usereffects-true', + 'sideeffects-false-usereffects-true-unused-import', + 'sideeffects-false-usereffects-true-used-import', + 'sideeffects-null-usereffects-true', + 'sideeffects-null-usereffects-true-unused-import', + 'sideeffects-null-usereffects-true-used-import', + 'sideeffects-true-usereffects-true', + 'sideeffects-true-usereffects-true-unused-import', + 'sideeffects-true-usereffects-true-used-import' ]); }, options: { treeshake: { - pureInternalModules(id) { + moduleSideEffects(id) { return JSON.parse(id.split('-')[3]); } }, @@ -43,10 +46,10 @@ module.exports = { load(id) { if (id[0] !== '/') { const sideEffects = JSON.parse(id.split('-')[1]); - const pureInt = JSON.parse(id.split('-')[3]); + const userEffects = JSON.parse(id.split('-')[3]); assert.strictEqual( this.getModuleInfo(id).hasModuleSideEffects, - sideEffects === null ? !pureInt : sideEffects + sideEffects || userEffects ); return `export const value = '${id}'; sideEffects.push(value);`; } @@ -58,24 +61,27 @@ module.exports = { .sort() .map(id => ({ id, hasModuleSideEffects: this.getModuleInfo(id).hasModuleSideEffects })), [ - { id: 'sideeffects-false-pureint-false', hasModuleSideEffects: false }, - { id: 'sideeffects-false-pureint-false-unused-import', hasModuleSideEffects: false }, - { id: 'sideeffects-false-pureint-false-used-import', hasModuleSideEffects: false }, - { id: 'sideeffects-false-pureint-true', hasModuleSideEffects: false }, - { id: 'sideeffects-false-pureint-true-unused-import', hasModuleSideEffects: false }, - { id: 'sideeffects-false-pureint-true-used-import', hasModuleSideEffects: false }, - { id: 'sideeffects-null-pureint-false', hasModuleSideEffects: true }, - { id: 'sideeffects-null-pureint-false-unused-import', hasModuleSideEffects: true }, - { id: 'sideeffects-null-pureint-false-used-import', hasModuleSideEffects: true }, - { id: 'sideeffects-null-pureint-true', hasModuleSideEffects: false }, - { id: 'sideeffects-null-pureint-true-unused-import', hasModuleSideEffects: false }, - { id: 'sideeffects-null-pureint-true-used-import', hasModuleSideEffects: false }, - { id: 'sideeffects-true-pureint-false', hasModuleSideEffects: true }, - { id: 'sideeffects-true-pureint-false-unused-import', hasModuleSideEffects: true }, - { id: 'sideeffects-true-pureint-false-used-import', hasModuleSideEffects: true }, - { id: 'sideeffects-true-pureint-true', hasModuleSideEffects: true }, - { id: 'sideeffects-true-pureint-true-unused-import', hasModuleSideEffects: true }, - { id: 'sideeffects-true-pureint-true-used-import', hasModuleSideEffects: true } + { id: 'sideeffects-false-usereffects-false', hasModuleSideEffects: false }, + { + id: 'sideeffects-false-usereffects-false-unused-import', + hasModuleSideEffects: false + }, + { id: 'sideeffects-false-usereffects-false-used-import', hasModuleSideEffects: false }, + { id: 'sideeffects-false-usereffects-true', hasModuleSideEffects: true }, + { id: 'sideeffects-false-usereffects-true-unused-import', hasModuleSideEffects: true }, + { id: 'sideeffects-false-usereffects-true-used-import', hasModuleSideEffects: true }, + { id: 'sideeffects-null-usereffects-false', hasModuleSideEffects: false }, + { id: 'sideeffects-null-usereffects-false-unused-import', hasModuleSideEffects: false }, + { id: 'sideeffects-null-usereffects-false-used-import', hasModuleSideEffects: false }, + { id: 'sideeffects-null-usereffects-true', hasModuleSideEffects: true }, + { id: 'sideeffects-null-usereffects-true-unused-import', hasModuleSideEffects: true }, + { id: 'sideeffects-null-usereffects-true-used-import', hasModuleSideEffects: true }, + { id: 'sideeffects-true-usereffects-false', hasModuleSideEffects: true }, + { id: 'sideeffects-true-usereffects-false-unused-import', hasModuleSideEffects: true }, + { id: 'sideeffects-true-usereffects-false-used-import', hasModuleSideEffects: true }, + { id: 'sideeffects-true-usereffects-true', hasModuleSideEffects: true }, + { id: 'sideeffects-true-usereffects-true-unused-import', hasModuleSideEffects: true }, + { id: 'sideeffects-true-usereffects-true-used-import', hasModuleSideEffects: true } ] ); } diff --git a/test/function/samples/pure-modules/resolve-id/main.js b/test/function/samples/pure-modules/resolve-id/main.js index 7bd7fc1b69f..dba6f1c07dc 100644 --- a/test/function/samples/pure-modules/resolve-id/main.js +++ b/test/function/samples/pure-modules/resolve-id/main.js @@ -1,25 +1,25 @@ -import 'sideeffects-false-pureint-false'; -import { value as unusedValue1 } from 'sideeffects-false-pureint-false-unused-import'; -import { value as usedValue1 } from 'sideeffects-false-pureint-false-used-import'; +import 'sideeffects-false-usereffects-false'; +import { value as unusedValue1 } from 'sideeffects-false-usereffects-false-unused-import'; +import { value as usedValue1 } from 'sideeffects-false-usereffects-false-used-import'; -import 'sideeffects-null-pureint-false'; -import { value as unusedValue2 } from 'sideeffects-null-pureint-false-unused-import'; -import { value as usedValue2 } from 'sideeffects-null-pureint-false-used-import'; +import 'sideeffects-null-usereffects-false'; +import { value as unusedValue2 } from 'sideeffects-null-usereffects-false-unused-import'; +import { value as usedValue2 } from 'sideeffects-null-usereffects-false-used-import'; -import 'sideeffects-true-pureint-false'; -import { value as unusedValue3 } from 'sideeffects-true-pureint-false-unused-import'; -import { value as usedValue3 } from 'sideeffects-true-pureint-false-used-import'; +import 'sideeffects-true-usereffects-false'; +import { value as unusedValue3 } from 'sideeffects-true-usereffects-false-unused-import'; +import { value as usedValue3 } from 'sideeffects-true-usereffects-false-used-import'; -import 'sideeffects-false-pureint-true'; -import { value as unusedValue4 } from 'sideeffects-false-pureint-true-unused-import'; -import { value as usedValue4 } from 'sideeffects-false-pureint-true-used-import'; +import 'sideeffects-false-usereffects-true'; +import { value as unusedValue4 } from 'sideeffects-false-usereffects-true-unused-import'; +import { value as usedValue4 } from 'sideeffects-false-usereffects-true-used-import'; -import 'sideeffects-null-pureint-true'; -import { value as unusedValue5 } from 'sideeffects-null-pureint-true-unused-import'; -import { value as usedValue5 } from 'sideeffects-null-pureint-true-used-import'; +import 'sideeffects-null-usereffects-true'; +import { value as unusedValue5 } from 'sideeffects-null-usereffects-true-unused-import'; +import { value as usedValue5 } from 'sideeffects-null-usereffects-true-used-import'; -import 'sideeffects-true-pureint-true'; -import { value as unusedValue6 } from 'sideeffects-true-pureint-true-unused-import'; -import { value as usedValue6 } from 'sideeffects-true-pureint-true-used-import'; +import 'sideeffects-true-usereffects-true'; +import { value as unusedValue6 } from 'sideeffects-true-usereffects-true-unused-import'; +import { value as usedValue6 } from 'sideeffects-true-usereffects-true-used-import'; export const values = [usedValue1, usedValue2, usedValue3, usedValue4, usedValue5, usedValue6]; From f4105d45230164994d168dac00a6d42fdd04e1ba Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Tue, 7 May 2019 08:52:17 +0200 Subject: [PATCH 10/19] Implement load and transform hook handling --- src/Module.ts | 20 ++++-- src/ModuleLoader.ts | 20 ++---- src/rollup/types.d.ts | 25 ++++--- src/utils/transform.ts | 48 ++++++++----- .../samples/pure-modules/load/_config.js | 42 +++++++++++ .../samples/pure-modules/load/main.js | 9 +++ .../resolve-id-external/_config.js | 1 - .../pure-modules/resolve-id/_config.js | 2 - .../samples/pure-modules/transform/_config.js | 70 +++++++++++++++++++ .../samples/pure-modules/transform/main.js | 29 ++++++++ 10 files changed, 216 insertions(+), 50 deletions(-) create mode 100644 test/function/samples/pure-modules/load/_config.js create mode 100644 test/function/samples/pure-modules/load/main.js create mode 100644 test/function/samples/pure-modules/transform/_config.js create mode 100644 test/function/samples/pure-modules/transform/main.js diff --git a/src/Module.ts b/src/Module.ts index f7d676c4ca4..4b8299aca44 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -34,7 +34,8 @@ import { RawSourceMap, ResolvedIdMap, RollupError, - RollupWarning + RollupWarning, + TransformModuleJSON } from './rollup/types'; import { error } from './utils/error'; import getCodeFrame from './utils/getCodeFrame'; @@ -478,21 +479,25 @@ export default class Module { } setSource({ + ast, code, + customTransformCache, + moduleSideEffects, originalCode, originalSourcemap, - ast, - sourcemapChain, resolvedIds, - transformDependencies, - customTransformCache - }: ModuleJSON) { + sourcemapChain, + transformDependencies + }: TransformModuleJSON) { this.code = code; this.originalCode = originalCode; this.originalSourcemap = originalSourcemap; - this.sourcemapChain = sourcemapChain; + this.sourcemapChain = sourcemapChain as RawSourceMap[]; this.transformDependencies = transformDependencies; this.customTransformCache = customTransformCache; + if (typeof moduleSideEffects === 'boolean') { + this.moduleSideEffects = moduleSideEffects; + } timeStart('generate ast', 3); @@ -568,6 +573,7 @@ export default class Module { customTransformCache: this.customTransformCache, dependencies: this.dependencies.map(module => module.id), id: this.id, + moduleSideEffects: this.moduleSideEffects, originalCode: this.originalCode, originalSourcemap: this.originalSourcemap, resolvedIds: this.resolvedIds, diff --git a/src/ModuleLoader.ts b/src/ModuleLoader.ts index 99001195302..273ae8794bf 100644 --- a/src/ModuleLoader.ts +++ b/src/ModuleLoader.ts @@ -6,12 +6,12 @@ import { ExternalOption, GetManualChunk, IsExternal, - ModuleJSON, ModuleSideEffectsOption, PureModulesOption, ResolvedId, ResolveIdResult, SourceDescription, + TransformModuleJSON, WarningHandler } from './rollup/types'; import { @@ -304,19 +304,11 @@ export class ModuleLoader { }) .then(source => { timeEnd('load modules', 3); - if (typeof source === 'string') return source; + if (typeof source === 'string') return { code: source }; if (source && typeof source === 'object' && typeof source.code === 'string') return source; error(errBadLoader(id)); }) - .then(source => { - const sourceDescription: SourceDescription = - typeof source === 'string' - ? { - ast: null, - code: source - } - : source; - + .then(sourceDescription => { const cachedModule = this.graph.cachedModules.get(id); if ( cachedModule && @@ -331,11 +323,13 @@ export class ModuleLoader { return cachedModule; } + if (typeof sourceDescription.moduleSideEffects === 'boolean') { + module.moduleSideEffects = sourceDescription.moduleSideEffects; + } return transform(this.graph, sourceDescription, module); }) - .then((source: ModuleJSON) => { + .then((source: TransformModuleJSON) => { module.setSource(source); - this.modulesById.set(id, module); return this.fetchAllDependencies(module).then(() => { diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index 588ba7ab199..ae3c61bd6fe 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -65,27 +65,32 @@ export interface SourceDescription { ast?: ESTree.Program; code: string; map?: string | RawSourceMap; + moduleSideEffects?: boolean | null; } export interface TransformSourceDescription extends SourceDescription { dependencies?: string[]; } -export interface ModuleJSON { +export interface TransformModuleJSON { ast: ESTree.Program; code: string; // note if plugins use new this.cache to opt-out auto transform cache customTransformCache: boolean; - dependencies: string[]; - id: string; + moduleSideEffects: boolean | null; originalCode: string; originalSourcemap: RawSourceMap | void; - resolvedIds: ResolvedIdMap; - sourcemapChain: RawSourceMap[]; - transformAssets: Asset[] | void; + resolvedIds?: ResolvedIdMap; + sourcemapChain: (RawSourceMap | { missing: true; plugin: string })[]; transformDependencies: string[] | null; } +export interface ModuleJSON extends TransformModuleJSON { + dependencies: string[]; + id: string; + transformAssets: Asset[] | void; +} + export interface Asset { fileName: string; name: string; @@ -174,15 +179,13 @@ export type LoadHook = ( id: string ) => Promise | SourceDescription | string | null; +export type TransformResult = string | void | TransformSourceDescription; + export type TransformHook = ( this: PluginContext, code: string, id: string -) => - | Promise - | TransformSourceDescription - | string - | void; +) => Promise | TransformResult; export type TransformChunkHook = ( this: PluginContext, diff --git a/src/utils/transform.ts b/src/utils/transform.ts index 21b32dda282..56b1f321d10 100644 --- a/src/utils/transform.ts +++ b/src/utils/transform.ts @@ -1,17 +1,18 @@ -import * as ESTree from 'estree'; import { decode } from 'sourcemap-codec'; -import Program from '../ast/nodes/Program'; import Graph from '../Graph'; import Module from '../Module'; import { Asset, EmitAsset, + ExistingRawSourceMap, Plugin, PluginCache, PluginContext, RawSourceMap, RollupError, RollupWarning, + TransformModuleJSON, + TransformResult, TransformSourceDescription } from '../rollup/types'; import { createTransformEmitAsset } from './assetHooks'; @@ -23,9 +24,9 @@ export default function transform( graph: Graph, source: TransformSourceDescription, module: Module -) { +): Promise { const id = module.id; - const sourcemapChain: RawSourceMap[] = []; + const sourcemapChain: (RawSourceMap | { missing: true; plugin: string })[] = []; const originalSourcemap = typeof source.map === 'string' ? JSON.parse(source.map) : source.map; if (originalSourcemap && typeof originalSourcemap.mappings === 'string') @@ -33,19 +34,25 @@ export default function transform( const baseEmitAsset = graph.pluginDriver.emitAsset; const originalCode = source.code; - let ast = source.ast; + let ast = source.ast; let transformDependencies: string[]; let assets: Asset[]; let customTransformCache = false; + let moduleSideEffects: boolean | null = null; let trackedPluginCache: { cache: PluginCache; used: boolean }; let curPlugin: Plugin; const curSource: string = source.code; - function transformReducer(this: PluginContext, code: string, result: any, plugin: Plugin) { + function transformReducer( + this: PluginContext, + code: string, + result: TransformResult, + plugin: Plugin + ) { // track which plugins use the custom this.cache to opt-out of transform caching if (!customTransformCache && trackedPluginCache.used) customTransformCache = true; if (customTransformCache) { - if (result && Array.isArray(result.dependencies)) { + if (result && typeof result === 'object' && Array.isArray(result.dependencies)) { for (const dep of result.dependencies) { const depId = resolve(dirname(id), dep); if (!graph.watchFiles[depId]) graph.watchFiles[depId] = true; @@ -55,7 +62,7 @@ export default function transform( // assets emitted by transform are transformDependencies if (assets.length) module.transformAssets = assets; - if (result && Array.isArray(result.dependencies)) { + if (result && typeof result === 'object' && Array.isArray(result.dependencies)) { // not great, but a useful way to track this without assuming WeakMap if (!(curPlugin).warnedTransformDependencies) this.warn({ @@ -69,7 +76,7 @@ export default function transform( } } - if (result == null) return code; + if (!result) return code; if (typeof result === 'string') { result = { @@ -77,18 +84,26 @@ export default function transform( code: result, map: undefined }; - } else if (typeof result.map === 'string') { - // `result.map` can only be a string if `result` isn't - result.map = JSON.parse(result.map); + } else { + if (typeof result.map === 'string') { + result.map = JSON.parse(result.map); + } + if (typeof result.moduleSideEffects === 'boolean') { + moduleSideEffects = result.moduleSideEffects; + } } - if (result.map && typeof result.map.mappings === 'string') { - result.map.mappings = decode(result.map.mappings); + if (result.map && typeof (result.map as ExistingRawSourceMap).mappings === 'string') { + (result.map as ExistingRawSourceMap).mappings = decode( + (result.map as ExistingRawSourceMap).mappings + ); } // strict null check allows 'null' maps to not be pushed to the chain, while 'undefined' gets the missing map warning if (result.map !== null) { - sourcemapChain.push(result.map || { missing: true, plugin: plugin.name }); + sourcemapChain.push( + (result.map as ExistingRawSourceMap) || { missing: true, plugin: plugin.name } + ); } ast = result.ast; @@ -162,9 +177,10 @@ export default function transform( if (!customTransformCache && setAssetSourceErr) throw setAssetSourceErr; return { - ast: ast, + ast, code, customTransformCache, + moduleSideEffects, originalCode, originalSourcemap, sourcemapChain, diff --git a/test/function/samples/pure-modules/load/_config.js b/test/function/samples/pure-modules/load/_config.js new file mode 100644 index 00000000000..aecd82796e4 --- /dev/null +++ b/test/function/samples/pure-modules/load/_config.js @@ -0,0 +1,42 @@ +const assert = require('assert'); +const sideEffects = []; + +module.exports = { + description: 'handles setting moduleSideEffects in the load hook', + context: { + sideEffects + }, + exports() { + assert.deepStrictEqual(sideEffects, [ + 'sideeffects-null-load-null', + 'sideeffects-true-load-null', + 'sideeffects-false-load-true', + 'sideeffects-null-load-true', + 'sideeffects-true-load-true' + ]); + }, + options: { + treeshake: { + moduleSideEffects(id) { + return JSON.parse(id.split('-')[1]); + } + }, + plugins: { + name: 'test-plugin', + resolveId(id) { + if (id[0] !== '/') { + return id; + } + }, + load(id) { + if (id[0] !== '/') { + const moduleSideEffects = JSON.parse(id.split('-')[3]); + return { + code: `export const value = '${id}'; sideEffects.push(value);`, + moduleSideEffects + }; + } + } + } + } +}; diff --git a/test/function/samples/pure-modules/load/main.js b/test/function/samples/pure-modules/load/main.js new file mode 100644 index 00000000000..d8ca61ae01e --- /dev/null +++ b/test/function/samples/pure-modules/load/main.js @@ -0,0 +1,9 @@ +import 'sideeffects-false-load-false'; +import 'sideeffects-null-load-false'; +import 'sideeffects-true-load-false'; +import 'sideeffects-false-load-null'; +import 'sideeffects-null-load-null'; +import 'sideeffects-true-load-null'; +import 'sideeffects-false-load-true'; +import 'sideeffects-null-load-true'; +import 'sideeffects-true-load-true'; diff --git a/test/function/samples/pure-modules/resolve-id-external/_config.js b/test/function/samples/pure-modules/resolve-id-external/_config.js index 05a3347a2d6..e8f393b27b0 100644 --- a/test/function/samples/pure-modules/resolve-id-external/_config.js +++ b/test/function/samples/pure-modules/resolve-id-external/_config.js @@ -2,7 +2,6 @@ const assert = require('assert'); const sideEffects = []; module.exports = { - solo: true, description: 'does not include modules without used exports if moduleSideEffect is false', context: { require(id) { diff --git a/test/function/samples/pure-modules/resolve-id/_config.js b/test/function/samples/pure-modules/resolve-id/_config.js index 214f36a076c..cb8a00da9df 100644 --- a/test/function/samples/pure-modules/resolve-id/_config.js +++ b/test/function/samples/pure-modules/resolve-id/_config.js @@ -1,9 +1,7 @@ const assert = require('assert'); const sideEffects = []; -// TODO Lukas implement load and transform hooks module.exports = { - solo: true, description: 'does not include modules without used exports if moduleSideEffect is false', context: { sideEffects diff --git a/test/function/samples/pure-modules/transform/_config.js b/test/function/samples/pure-modules/transform/_config.js new file mode 100644 index 00000000000..724e66ced38 --- /dev/null +++ b/test/function/samples/pure-modules/transform/_config.js @@ -0,0 +1,70 @@ +const assert = require('assert'); +const sideEffects = []; + +module.exports = { + description: 'handles setting moduleSideEffects in the transform hook', + context: { + sideEffects + }, + exports() { + assert.deepStrictEqual(sideEffects, [ + 'sideeffects-null-1-null-2-null', + 'sideeffects-true-1-null-2-null', + 'sideeffects-false-1-true-2-null', + 'sideeffects-null-1-true-2-null', + 'sideeffects-true-1-true-2-null', + 'sideeffects-false-1-false-2-true', + 'sideeffects-null-1-false-2-true', + 'sideeffects-true-1-false-2-true', + 'sideeffects-false-1-null-2-true', + 'sideeffects-null-1-null-2-true', + 'sideeffects-true-1-null-2-true', + 'sideeffects-false-1-true-2-true', + 'sideeffects-null-1-true-2-true', + 'sideeffects-true-1-true-2-true' + ]); + }, + options: { + treeshake: { + moduleSideEffects(id) { + return JSON.parse(id.split('-')[1]); + } + }, + plugins: [ + { + name: 'test-plugin-1', + resolveId(id) { + if (id[0] !== '/') { + return id; + } + }, + load(id) { + if (id[0] !== '/') { + return `export const value = '${id}'; sideEffects.push(value);`; + } + }, + transform(code, id) { + if (id[0] !== '/') { + const moduleSideEffects = JSON.parse(id.split('-')[3]); + return { + code, + moduleSideEffects + }; + } + } + }, + { + name: 'test-plugin-2', + transform(code, id) { + if (id[0] !== '/') { + const moduleSideEffects = JSON.parse(id.split('-')[5]); + return { + code, + moduleSideEffects + }; + } + } + } + ] + } +}; diff --git a/test/function/samples/pure-modules/transform/main.js b/test/function/samples/pure-modules/transform/main.js new file mode 100644 index 00000000000..7e52713945b --- /dev/null +++ b/test/function/samples/pure-modules/transform/main.js @@ -0,0 +1,29 @@ +import 'sideeffects-false-1-false-2-false'; +import 'sideeffects-null-1-false-2-false'; +import 'sideeffects-true-1-false-2-false'; +import 'sideeffects-false-1-null-2-false'; +import 'sideeffects-null-1-null-2-false'; +import 'sideeffects-true-1-null-2-false'; +import 'sideeffects-false-1-true-2-false'; +import 'sideeffects-null-1-true-2-false'; +import 'sideeffects-true-1-true-2-false'; + +import 'sideeffects-false-1-false-2-null'; +import 'sideeffects-null-1-false-2-null'; +import 'sideeffects-true-1-false-2-null'; +import 'sideeffects-false-1-null-2-null'; +import 'sideeffects-null-1-null-2-null'; +import 'sideeffects-true-1-null-2-null'; +import 'sideeffects-false-1-true-2-null'; +import 'sideeffects-null-1-true-2-null'; +import 'sideeffects-true-1-true-2-null'; + +import 'sideeffects-false-1-false-2-true'; +import 'sideeffects-null-1-false-2-true'; +import 'sideeffects-true-1-false-2-true'; +import 'sideeffects-false-1-null-2-true'; +import 'sideeffects-null-1-null-2-true'; +import 'sideeffects-true-1-null-2-true'; +import 'sideeffects-false-1-true-2-true'; +import 'sideeffects-null-1-true-2-true'; +import 'sideeffects-true-1-true-2-true'; From 3b14521158f24911d65d24039e4120241d0bd4cf Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Wed, 8 May 2019 07:42:31 +0200 Subject: [PATCH 11/19] Test all versions of the moduleSideEffects option --- src/Graph.ts | 2 +- src/ModuleLoader.ts | 22 ++++---- src/rollup/types.d.ts | 5 +- src/utils/error.ts | 8 +++ .../samples/pure-modules/array/_config.js | 55 +++++++++++++++++++ .../samples/pure-modules/array/main.js | 8 +++ .../pure-modules/external-false/_config.js | 41 ++++++++++++++ .../pure-modules/external-false/main.js | 3 + .../pure-modules/global-false/_config.js | 39 +++++++++++++ .../samples/pure-modules/global-false/main.js | 3 + .../pure-modules/invalid-option/_config.js | 15 +++++ .../pure-modules/invalid-option/main.js | 1 + 12 files changed, 188 insertions(+), 14 deletions(-) create mode 100644 test/function/samples/pure-modules/array/_config.js create mode 100644 test/function/samples/pure-modules/array/main.js create mode 100644 test/function/samples/pure-modules/external-false/_config.js create mode 100644 test/function/samples/pure-modules/external-false/main.js create mode 100644 test/function/samples/pure-modules/global-false/_config.js create mode 100644 test/function/samples/pure-modules/global-false/main.js create mode 100644 test/function/samples/pure-modules/invalid-option/_config.js create mode 100644 test/function/samples/pure-modules/invalid-option/main.js diff --git a/src/Graph.ts b/src/Graph.ts index c07b890c9af..c45b0ed1528 100644 --- a/src/Graph.ts +++ b/src/Graph.ts @@ -187,7 +187,7 @@ export default class Graph { this.pluginDriver, options.external, typeof options.manualChunks === 'function' && options.manualChunks, - this.treeshake ? this.treeshakingOptions.moduleSideEffects : false, + this.treeshake ? this.treeshakingOptions.moduleSideEffects : null, this.treeshake ? this.treeshakingOptions.pureExternalModules : false ); } diff --git a/src/ModuleLoader.ts b/src/ModuleLoader.ts index 273ae8794bf..f4db1473842 100644 --- a/src/ModuleLoader.ts +++ b/src/ModuleLoader.ts @@ -11,8 +11,7 @@ import { ResolvedId, ResolveIdResult, SourceDescription, - TransformModuleJSON, - WarningHandler + TransformModuleJSON } from './rollup/types'; import { errBadLoader, @@ -21,6 +20,7 @@ import { errChunkReferenceIdNotFoundForFilename, errEntryCannotBeExternal, errInternalIdCannotBeExternal, + errInvalidOption, errNamespaceConflict, error, errUnresolvedEntry, @@ -65,13 +65,12 @@ function getIdMatcher>( function getHasModuleSideEffects( moduleSideEffectsOption: ModuleSideEffectsOption, pureExternalModules: PureModulesOption, - warn: WarningHandler + graph: Graph ): (id: string, external: boolean) => boolean { if (moduleSideEffectsOption === false) { return () => false; } if (moduleSideEffectsOption === 'no-external') { - // TODO Lukas test return (_id, external) => !external; } if (typeof moduleSideEffectsOption === 'function') { @@ -79,21 +78,22 @@ function getHasModuleSideEffects( !id.startsWith('\0') ? moduleSideEffectsOption(id, ...args) !== false : true; } if (Array.isArray(moduleSideEffectsOption)) { - // TODO Lukas test const ids = new Set(moduleSideEffectsOption); return id => ids.has(id); } if (moduleSideEffectsOption && moduleSideEffectsOption !== true) { - // TODO Lukas test - warn({ - code: 'TODO', - message: 'TODO' - }); + graph.warn( + errInvalidOption( + 'treeshake.moduleSideEffects', + 'please use one of false, "no-external", a function or an array' + ) + ); } const isPureExternalModule = getIdMatcher(pureExternalModules); return (id, external) => !(external && isPureExternalModule(id)); } +// TODO Lukas document load and transform hook changes export class ModuleLoader { readonly isExternal: IsExternal; private readonly entriesByReferenceId = new Map< @@ -125,7 +125,7 @@ export class ModuleLoader { this.hasModuleSideEffects = getHasModuleSideEffects( moduleSideEffects, pureExternalModules, - graph.warn + graph ); this.getManualChunk = typeof getManualChunk === 'function' ? getManualChunk : () => null; } diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index ae3c61bd6fe..03c206f5ce1 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -327,13 +327,14 @@ export interface TreeshakingOptions { moduleSideEffects?: ModuleSideEffectsOption; propertyReadSideEffects?: boolean; // TODO Lukas adjust docs + // TODO Lukas explain deprecations in JSDoc + /** @deprecated */ pureExternalModules?: PureModulesOption; } export type GetManualChunk = (id: string) => string | void; -// TODO Lukas test and document true -export type ExternalOption = boolean | string[] | IsExternal; +export type ExternalOption = string[] | IsExternal; export type PureModulesOption = boolean | string[] | IsPureModule; export type GlobalsOption = { [name: string]: string } | ((name: string) => string); export type InputOption = string | string[] | { [entryAlias: string]: string }; diff --git a/src/utils/error.ts b/src/utils/error.ts index 95e1f460805..2ef5f2fce2d 100644 --- a/src/utils/error.ts +++ b/src/utils/error.ts @@ -42,6 +42,7 @@ export enum Errors { INVALID_ASSET_NAME = 'INVALID_ASSET_NAME', INVALID_CHUNK = 'INVALID_CHUNK', INVALID_EXTERNAL_ID = 'INVALID_EXTERNAL_ID', + INVALID_OPTION = 'INVALID_OPTION', INVALID_PLUGIN_HOOK = 'INVALID_PLUGIN_HOOK', INVALID_ROLLUP_PHASE = 'INVALID_ROLLUP_PHASE', NAMESPACE_CONFLICT = 'NAMESPACE_CONFLICT', @@ -149,6 +150,13 @@ export function errInternalIdCannotBeExternal(source: string, importer: string) }; } +export function errInvalidOption(option: string, explanation: string) { + return { + code: Errors.INVALID_OPTION, + message: `Invalid value for option "${option}" - ${explanation}.` + }; +} + export function errInvalidRollupPhaseForAddWatchFile() { return { code: Errors.INVALID_ROLLUP_PHASE, diff --git a/test/function/samples/pure-modules/array/_config.js b/test/function/samples/pure-modules/array/_config.js new file mode 100644 index 00000000000..e8b90ac2573 --- /dev/null +++ b/test/function/samples/pure-modules/array/_config.js @@ -0,0 +1,55 @@ +const assert = require('assert'); +const sideEffects = []; + +module.exports = { + description: 'supports setting module side effects via an array', + context: { + require(id) { + sideEffects.push(id); + return { value: id }; + }, + sideEffects + }, + exports() { + assert.deepStrictEqual(sideEffects, [ + 'pluginsideeffects-null-external-listed', + 'pluginsideeffects-true-external-listed', + 'pluginsideeffects-true', + 'pluginsideeffects-null-listed', + 'pluginsideeffects-true-listed' + ]); + }, + options: { + external: [ + 'pluginsideeffects-null-external', + 'pluginsideeffects-true-external', + 'pluginsideeffects-null-external-listed', + 'pluginsideeffects-true-external-listed' + ], + treeshake: { + moduleSideEffects: [ + 'pluginsideeffects-null-listed', + 'pluginsideeffects-true-listed', + 'pluginsideeffects-null-external-listed', + 'pluginsideeffects-true-external-listed' + ] + }, + plugins: { + name: 'test-plugin', + resolveId(id) { + if (id[0] !== '/') { + const moduleSideEffects = JSON.parse(id.split('-')[1]); + if (moduleSideEffects) { + return { id, moduleSideEffects }; + } + return id; + } + }, + load(id) { + if (id[0] !== '/') { + return `export const value = '${id}'; sideEffects.push(value);`; + } + } + } + } +}; diff --git a/test/function/samples/pure-modules/array/main.js b/test/function/samples/pure-modules/array/main.js new file mode 100644 index 00000000000..c04fccbe59e --- /dev/null +++ b/test/function/samples/pure-modules/array/main.js @@ -0,0 +1,8 @@ +import 'pluginsideeffects-null'; +import 'pluginsideeffects-true'; +import 'pluginsideeffects-null-external'; +import 'pluginsideeffects-true-external'; +import 'pluginsideeffects-null-listed'; +import 'pluginsideeffects-true-listed'; +import 'pluginsideeffects-null-external-listed'; +import 'pluginsideeffects-true-external-listed'; diff --git a/test/function/samples/pure-modules/external-false/_config.js b/test/function/samples/pure-modules/external-false/_config.js new file mode 100644 index 00000000000..37954326caa --- /dev/null +++ b/test/function/samples/pure-modules/external-false/_config.js @@ -0,0 +1,41 @@ +const assert = require('assert'); +const sideEffects = []; + +module.exports = { + description: 'supports setting module side effects to false for external modules', + context: { + require(id) { + sideEffects.push(id); + return { value: id }; + }, + sideEffects + }, + exports() { + assert.deepStrictEqual(sideEffects, ['pluginsideeffects-true', 'internal']); + }, + options: { + treeshake: { + moduleSideEffects: 'no-external' + }, + plugins: { + name: 'test-plugin', + resolveId(id) { + if (id[0] !== '/') { + if (id === 'internal') { + return id; + } + const moduleSideEffects = JSON.parse(id.split('-')[1]); + if (moduleSideEffects) { + return { id, moduleSideEffects, external: true }; + } + return { id, external: true }; + } + }, + load(id) { + if (id[0] !== '/') { + return `export const value = '${id}'; sideEffects.push(value);`; + } + } + } + } +}; diff --git a/test/function/samples/pure-modules/external-false/main.js b/test/function/samples/pure-modules/external-false/main.js new file mode 100644 index 00000000000..7a0b4385f92 --- /dev/null +++ b/test/function/samples/pure-modules/external-false/main.js @@ -0,0 +1,3 @@ +import 'pluginsideeffects-false'; +import 'pluginsideeffects-true'; +import 'internal'; diff --git a/test/function/samples/pure-modules/global-false/_config.js b/test/function/samples/pure-modules/global-false/_config.js new file mode 100644 index 00000000000..e645303de33 --- /dev/null +++ b/test/function/samples/pure-modules/global-false/_config.js @@ -0,0 +1,39 @@ +const assert = require('assert'); +const sideEffects = []; + +module.exports = { + description: 'supports setting module side effects to false for all modules', + context: { + require(id) { + sideEffects.push(id); + return { value: id }; + }, + sideEffects + }, + exports() { + assert.deepStrictEqual(sideEffects, ['pluginsideeffects-true']); + }, + options: { + external: ['external'], + treeshake: { + moduleSideEffects: false + }, + plugins: { + name: 'test-plugin', + resolveId(id) { + if (id[0] !== '/') { + const moduleSideEffects = JSON.parse(id.split('-')[1]); + if (moduleSideEffects) { + return { id, moduleSideEffects }; + } + return id; + } + }, + load(id) { + if (id[0] !== '/') { + return `export const value = '${id}'; sideEffects.push(value);`; + } + } + } + } +}; diff --git a/test/function/samples/pure-modules/global-false/main.js b/test/function/samples/pure-modules/global-false/main.js new file mode 100644 index 00000000000..788c803a4a3 --- /dev/null +++ b/test/function/samples/pure-modules/global-false/main.js @@ -0,0 +1,3 @@ +import 'pluginsideeffects-null'; +import 'pluginsideeffects-true'; +import 'external'; diff --git a/test/function/samples/pure-modules/invalid-option/_config.js b/test/function/samples/pure-modules/invalid-option/_config.js new file mode 100644 index 00000000000..0641fcdbfb2 --- /dev/null +++ b/test/function/samples/pure-modules/invalid-option/_config.js @@ -0,0 +1,15 @@ +module.exports = { + description: 'warns for invalid options', + options: { + treeshake: { + moduleSideEffects: 'what-is-this?' + } + }, + warnings: [ + { + code: 'INVALID_OPTION', + message: + 'Invalid value for option "treeshake.moduleSideEffects" - please use one of false, "no-external", a function or an array.' + } + ] +}; diff --git a/test/function/samples/pure-modules/invalid-option/main.js b/test/function/samples/pure-modules/invalid-option/main.js new file mode 100644 index 00000000000..46d3ca8c61f --- /dev/null +++ b/test/function/samples/pure-modules/invalid-option/main.js @@ -0,0 +1 @@ +export const value = 42; From aa7498de442635a671dc1319161f3eaacd90ebcf Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Wed, 8 May 2019 07:57:55 +0200 Subject: [PATCH 12/19] Explain deprecation alternatives in JSDoc --- src/rollup/types.d.ts | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index 03c206f5ce1..b0dcc50d066 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -127,16 +127,16 @@ export interface PluginContext extends MinimalPluginContext { importedIds: string[]; isExternal: boolean; }; - /** @deprecated */ + /** @deprecated Use `this.resolve` instead */ isExternal: IsExternal; moduleIds: IterableIterator; parse: (input: string, options: any) => ESTree.Program; resolve: (source: string, importer: string) => Promise; - /** @deprecated */ + /** @deprecated Use `this.resolve` instead */ resolveId: (source: string, importer: string) => Promise; setAssetSource: (assetReferenceId: string, source: string | Buffer) => void; warn: (warning: RollupWarning | string, pos?: { column: number; line: number }) => void; - /** @deprecated */ + /** @deprecated Use `this.addWatchFile` and the `watchChange` hook instead */ watcher: EventEmitter; } @@ -280,13 +280,13 @@ export interface PluginHooks { isWrite: boolean ) => void | Promise; load?: LoadHook; - /** @deprecated */ + /** @deprecated Use `generateBundle` instead */ ongenerate?: ( this: PluginContext, options: OnGenerateOptions, chunk: OutputChunk ) => void | Promise; - /** @deprecated */ + /** @deprecated Use `writeBundle` instead */ onwrite?: ( this: PluginContext, options: OnWriteOptions, @@ -297,16 +297,16 @@ export interface PluginHooks { renderChunk?: RenderChunkHook; renderError?: (this: PluginContext, err?: Error) => Promise | void; renderStart?: (this: PluginContext) => Promise | void; - /** @deprecated */ + /** @deprecated Use `resolveFileUrl` instead */ resolveAssetUrl?: ResolveAssetUrlHook; resolveDynamicImport?: ResolveDynamicImportHook; resolveFileUrl?: ResolveFileUrlHook; resolveId?: ResolveIdHook; resolveImportMeta?: ResolveImportMetaHook; transform?: TransformHook; - /** @deprecated */ + /** @deprecated Use `renderChunk` instead */ transformBundle?: TransformChunkHook; - /** @deprecated */ + /** @deprecated Use `renderChunk` instead */ transformChunk?: TransformChunkHook; watchChange?: (id: string) => void; writeBundle?: (this: PluginContext, bundle: OutputBundle) => void | Promise; @@ -327,8 +327,7 @@ export interface TreeshakingOptions { moduleSideEffects?: ModuleSideEffectsOption; propertyReadSideEffects?: boolean; // TODO Lukas adjust docs - // TODO Lukas explain deprecations in JSDoc - /** @deprecated */ + /** @deprecated Use `moduleSideEffects` instead */ pureExternalModules?: PureModulesOption; } From f6ebed885a3a24d878b9a6857b114b6bc1bb5f8a Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Thu, 9 May 2019 18:13:41 +0200 Subject: [PATCH 13/19] Document new options --- docs/05-plugins.md | 38 ++++++----- docs/999-big-list-of-options.md | 111 ++++++++++++++++++++++++-------- src/ModuleLoader.ts | 1 - src/rollup/types.d.ts | 2 - 4 files changed, 107 insertions(+), 45 deletions(-) diff --git a/docs/05-plugins.md b/docs/05-plugins.md index 4049760b826..c8714ab86be 100644 --- a/docs/05-plugins.md +++ b/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 }`
+Type: `(id: string) => string | null | { code: string, map?: string | SourceMap, ast? : ESTree.Program, moduleSideEffects?: boolean | null }`
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 time 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 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`
@@ -224,10 +228,10 @@ resolveFileUrl({fileName}) { ``` #### `resolveId` -Type: `(source: string, importer: string) => string | false | null | {id: string, external?: boolean}`
+Type: `(source: string, importer: string) => string | false | null | {id: string, external?: boolean, moduleSideEffects?: boolean | null}`
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, the `treeshake.moduleSideEffects` option is not set to `true`, 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`
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` -
+Type: `(code: string, id: string) => string | null | { code: string, map?: string | SourceMap, ast? : ESTree.Program, moduleSideEffects?: boolean | null }`
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 time 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`
@@ -346,11 +357,12 @@ 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 + 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 } ``` @@ -502,10 +514,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. diff --git a/docs/999-big-list-of-options.md b/docs/999-big-list-of-options.md index ca20a3acff9..ee3df53fa1e 100755 --- a/docs/999-big-list-of-options.md +++ b/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 }`
+Type: `boolean | { annotations?: boolean, moduleSideEffects?: ModuleSideEffectsOption, propertyReadSideEffects?: boolean }`
CLI: `--treeshake`/`--no-treeshake`
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`
-CLI: `--treeshake.propertyReadSideEffects`/`--no-treeshake.propertyReadSideEffects`
-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**
Type: `boolean`
CLI: `--treeshake.annotations`/`--no-treeshake.annotations`
@@ -774,12 +755,12 @@ class Impure { /*@__PURE__*/new Impure(); ``` -**treeshake.pureExternalModules**
-Type: `boolean`
-CLI: `--treeshake.pureExternalModules`/`--no-treeshake.pureExternalModules`
-Default: `false` +**treeshake.moduleSideEffects**
+Type: `boolean | null | "no-external" | string[] | (id: string, external: boolean) => boolean | null`
+CLI: `--treeshake.moduleSideEffects`/`--no-treeshake.moduleSideEffects`
+Default: `null` -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 === null 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 === null +console.log('side-effect'); + console.log(42); ``` +```javascript +// output with treeshake.moduleSideEffects === false +console.log(42); +``` + +The value `"no-external"` will only remove external imports if possible. You can also supply a list of modules with side-effects or a function to determine it for each module individually. Using `true` will behave the same as `null` except that it cannot be overridden by the `resolveId` hook of plugins. + +**treeshake.propertyReadSideEffects** +Type: `boolean`
+CLI: `--treeshake.propertyReadSideEffects`/`--no-treeshake.propertyReadSideEffects`
+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`
+CLI: `--treeshake.pureExternalModules`/`--no-treeshake.pureExternalModules`
+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. diff --git a/src/ModuleLoader.ts b/src/ModuleLoader.ts index f4db1473842..f4f4ba6cb0c 100644 --- a/src/ModuleLoader.ts +++ b/src/ModuleLoader.ts @@ -93,7 +93,6 @@ function getHasModuleSideEffects( return (id, external) => !(external && isPureExternalModule(id)); } -// TODO Lukas document load and transform hook changes export class ModuleLoader { readonly isExternal: IsExternal; private readonly entriesByReferenceId = new Map< diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index b0dcc50d066..8060cf44f77 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -323,10 +323,8 @@ export interface Plugin extends PluginHooks { export interface TreeshakingOptions { annotations?: boolean; - // TODO Lukas adjust docs moduleSideEffects?: ModuleSideEffectsOption; propertyReadSideEffects?: boolean; - // TODO Lukas adjust docs /** @deprecated Use `moduleSideEffects` instead */ pureExternalModules?: PureModulesOption; } From bddaa9312151a98440372a71be1b4eaf6a335ff0 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Thu, 9 May 2019 18:38:18 +0200 Subject: [PATCH 14/19] Try to fix Windows tests --- test/function/samples/pure-modules/array/_config.js | 5 +++-- .../samples/pure-modules/external-false/_config.js | 5 +++-- .../samples/pure-modules/global-false/_config.js | 5 +++-- test/function/samples/pure-modules/load/_config.js | 5 +++-- .../samples/pure-modules/resolve-id-external/_config.js | 5 +++-- test/function/samples/pure-modules/resolve-id/_config.js | 7 ++++--- test/function/samples/pure-modules/transform/_config.js | 9 +++++---- 7 files changed, 24 insertions(+), 17 deletions(-) diff --git a/test/function/samples/pure-modules/array/_config.js b/test/function/samples/pure-modules/array/_config.js index e8b90ac2573..979d9f7d3d4 100644 --- a/test/function/samples/pure-modules/array/_config.js +++ b/test/function/samples/pure-modules/array/_config.js @@ -1,4 +1,5 @@ const assert = require('assert'); +const path = require('path'); const sideEffects = []; module.exports = { @@ -37,7 +38,7 @@ module.exports = { plugins: { name: 'test-plugin', resolveId(id) { - if (id[0] !== '/') { + if (!path.isAbsolute(id)) { const moduleSideEffects = JSON.parse(id.split('-')[1]); if (moduleSideEffects) { return { id, moduleSideEffects }; @@ -46,7 +47,7 @@ module.exports = { } }, load(id) { - if (id[0] !== '/') { + if (!path.isAbsolute(id)) { return `export const value = '${id}'; sideEffects.push(value);`; } } diff --git a/test/function/samples/pure-modules/external-false/_config.js b/test/function/samples/pure-modules/external-false/_config.js index 37954326caa..41d1e313413 100644 --- a/test/function/samples/pure-modules/external-false/_config.js +++ b/test/function/samples/pure-modules/external-false/_config.js @@ -1,4 +1,5 @@ const assert = require('assert'); +const path = require('path'); const sideEffects = []; module.exports = { @@ -20,7 +21,7 @@ module.exports = { plugins: { name: 'test-plugin', resolveId(id) { - if (id[0] !== '/') { + if (!path.isAbsolute(id)) { if (id === 'internal') { return id; } @@ -32,7 +33,7 @@ module.exports = { } }, load(id) { - if (id[0] !== '/') { + if (!path.isAbsolute(id)) { return `export const value = '${id}'; sideEffects.push(value);`; } } diff --git a/test/function/samples/pure-modules/global-false/_config.js b/test/function/samples/pure-modules/global-false/_config.js index e645303de33..c45c07363a0 100644 --- a/test/function/samples/pure-modules/global-false/_config.js +++ b/test/function/samples/pure-modules/global-false/_config.js @@ -1,4 +1,5 @@ const assert = require('assert'); +const path = require('path'); const sideEffects = []; module.exports = { @@ -21,7 +22,7 @@ module.exports = { plugins: { name: 'test-plugin', resolveId(id) { - if (id[0] !== '/') { + if (!path.isAbsolute(id)) { const moduleSideEffects = JSON.parse(id.split('-')[1]); if (moduleSideEffects) { return { id, moduleSideEffects }; @@ -30,7 +31,7 @@ module.exports = { } }, load(id) { - if (id[0] !== '/') { + if (!path.isAbsolute(id)) { return `export const value = '${id}'; sideEffects.push(value);`; } } diff --git a/test/function/samples/pure-modules/load/_config.js b/test/function/samples/pure-modules/load/_config.js index aecd82796e4..f9ce4f8def8 100644 --- a/test/function/samples/pure-modules/load/_config.js +++ b/test/function/samples/pure-modules/load/_config.js @@ -1,4 +1,5 @@ const assert = require('assert'); +const path = require('path'); const sideEffects = []; module.exports = { @@ -24,12 +25,12 @@ module.exports = { plugins: { name: 'test-plugin', resolveId(id) { - if (id[0] !== '/') { + if (!path.isAbsolute(id)) { return id; } }, load(id) { - if (id[0] !== '/') { + if (!path.isAbsolute(id)) { const moduleSideEffects = JSON.parse(id.split('-')[3]); return { code: `export const value = '${id}'; sideEffects.push(value);`, diff --git a/test/function/samples/pure-modules/resolve-id-external/_config.js b/test/function/samples/pure-modules/resolve-id-external/_config.js index e8f393b27b0..47b0e2c18aa 100644 --- a/test/function/samples/pure-modules/resolve-id-external/_config.js +++ b/test/function/samples/pure-modules/resolve-id-external/_config.js @@ -1,4 +1,5 @@ const assert = require('assert'); +const path = require('path'); const sideEffects = []; module.exports = { @@ -36,7 +37,7 @@ module.exports = { plugins: { name: 'test-plugin', resolveId(id) { - if (id[0] !== '/') { + if (!path.isAbsolute(id)) { return { id, external: true, @@ -47,7 +48,7 @@ module.exports = { buildEnd() { assert.deepStrictEqual( Array.from(this.moduleIds) - .filter(id => id[0] !== '/') + .filter(id => !path.isAbsolute(id)) .sort() .map(id => ({ id, hasModuleSideEffects: this.getModuleInfo(id).hasModuleSideEffects })), [ diff --git a/test/function/samples/pure-modules/resolve-id/_config.js b/test/function/samples/pure-modules/resolve-id/_config.js index cb8a00da9df..5bb328351c3 100644 --- a/test/function/samples/pure-modules/resolve-id/_config.js +++ b/test/function/samples/pure-modules/resolve-id/_config.js @@ -1,4 +1,5 @@ const assert = require('assert'); +const path = require('path'); const sideEffects = []; module.exports = { @@ -33,7 +34,7 @@ module.exports = { plugins: { name: 'test-plugin', resolveId(id) { - if (id[0] !== '/') { + if (!path.isAbsolute(id)) { return { id, external: false, @@ -42,7 +43,7 @@ module.exports = { } }, load(id) { - if (id[0] !== '/') { + if (!path.isAbsolute(id)) { const sideEffects = JSON.parse(id.split('-')[1]); const userEffects = JSON.parse(id.split('-')[3]); assert.strictEqual( @@ -55,7 +56,7 @@ module.exports = { buildEnd() { assert.deepStrictEqual( Array.from(this.moduleIds) - .filter(id => id[0] !== '/') + .filter(id => !path.isAbsolute(id)) .sort() .map(id => ({ id, hasModuleSideEffects: this.getModuleInfo(id).hasModuleSideEffects })), [ diff --git a/test/function/samples/pure-modules/transform/_config.js b/test/function/samples/pure-modules/transform/_config.js index 724e66ced38..95e29e40eba 100644 --- a/test/function/samples/pure-modules/transform/_config.js +++ b/test/function/samples/pure-modules/transform/_config.js @@ -1,4 +1,5 @@ const assert = require('assert'); +const path = require('path'); const sideEffects = []; module.exports = { @@ -34,17 +35,17 @@ module.exports = { { name: 'test-plugin-1', resolveId(id) { - if (id[0] !== '/') { + if (!path.isAbsolute(id)) { return id; } }, load(id) { - if (id[0] !== '/') { + if (!path.isAbsolute(id)) { return `export const value = '${id}'; sideEffects.push(value);`; } }, transform(code, id) { - if (id[0] !== '/') { + if (!path.isAbsolute(id)) { const moduleSideEffects = JSON.parse(id.split('-')[3]); return { code, @@ -56,7 +57,7 @@ module.exports = { { name: 'test-plugin-2', transform(code, id) { - if (id[0] !== '/') { + if (!path.isAbsolute(id)) { const moduleSideEffects = JSON.parse(id.split('-')[5]); return { code, From f266045babc3e6ad0067862380ed4c01d08ecf1b Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sat, 11 May 2019 10:38:31 +0200 Subject: [PATCH 15/19] Rename test folder, mark modules as executed in LocalVariable --- src/Module.ts | 3 --- src/ast/variables/LocalVariable.ts | 4 ++++ .../array/_config.js | 0 .../array/main.js | 0 .../external-false/_config.js | 0 .../external-false/main.js | 0 .../global-false/_config.js | 0 .../global-false/main.js | 0 .../invalid-option/_config.js | 0 .../invalid-option/main.js | 0 .../load/_config.js | 0 .../load/main.js | 0 .../module-side-effects/reexports/_config.js | 17 +++++++++++++++++ .../module-side-effects/reexports/dep1.js | 3 +++ .../module-side-effects/reexports/dep2.js | 3 +++ .../module-side-effects/reexports/lib.js | 4 ++++ .../module-side-effects/reexports/main.js | 1 + .../resolve-id-external/_config.js | 0 .../resolve-id-external/main.js | 0 .../resolve-id/_config.js | 0 .../resolve-id/main.js | 0 .../transform/_config.js | 0 .../transform/main.js | 0 23 files changed, 32 insertions(+), 3 deletions(-) rename test/function/samples/{pure-modules => module-side-effects}/array/_config.js (100%) rename test/function/samples/{pure-modules => module-side-effects}/array/main.js (100%) rename test/function/samples/{pure-modules => module-side-effects}/external-false/_config.js (100%) rename test/function/samples/{pure-modules => module-side-effects}/external-false/main.js (100%) rename test/function/samples/{pure-modules => module-side-effects}/global-false/_config.js (100%) rename test/function/samples/{pure-modules => module-side-effects}/global-false/main.js (100%) rename test/function/samples/{pure-modules => module-side-effects}/invalid-option/_config.js (100%) rename test/function/samples/{pure-modules => module-side-effects}/invalid-option/main.js (100%) rename test/function/samples/{pure-modules => module-side-effects}/load/_config.js (100%) rename test/function/samples/{pure-modules => module-side-effects}/load/main.js (100%) create mode 100644 test/function/samples/module-side-effects/reexports/_config.js create mode 100644 test/function/samples/module-side-effects/reexports/dep1.js create mode 100644 test/function/samples/module-side-effects/reexports/dep2.js create mode 100644 test/function/samples/module-side-effects/reexports/lib.js create mode 100644 test/function/samples/module-side-effects/reexports/main.js rename test/function/samples/{pure-modules => module-side-effects}/resolve-id-external/_config.js (100%) rename test/function/samples/{pure-modules => module-side-effects}/resolve-id-external/main.js (100%) rename test/function/samples/{pure-modules => module-side-effects}/resolve-id/_config.js (100%) rename test/function/samples/{pure-modules => module-side-effects}/resolve-id/main.js (100%) rename test/function/samples/{pure-modules => module-side-effects}/transform/_config.js (100%) rename test/function/samples/{pure-modules => module-side-effects}/transform/main.js (100%) diff --git a/src/Module.ts b/src/Module.ts index 4b8299aca44..87137446bcb 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -775,9 +775,6 @@ export default class Module { const variableModule = variable.module; if (!variable.included) { variable.include(); - if (variableModule instanceof Module && !variableModule.isExecuted) { - markModuleAndImpureDependenciesAsExecuted(variableModule); - } this.graph.needsTreeshakingPass = true; } if (variableModule && variableModule !== this) { diff --git a/src/ast/variables/LocalVariable.ts b/src/ast/variables/LocalVariable.ts index 54d82cc255a..964f83a622a 100644 --- a/src/ast/variables/LocalVariable.ts +++ b/src/ast/variables/LocalVariable.ts @@ -1,4 +1,5 @@ import Module, { AstContext } from '../../Module'; +import { markModuleAndImpureDependenciesAsExecuted } from '../../utils/traverseStaticDependencies'; import CallOptions from '../CallOptions'; import { DeoptimizableEntity } from '../DeoptimizableEntity'; import { ExecutionPathOptions } from '../ExecutionPathOptions'; @@ -173,6 +174,9 @@ export default class LocalVariable extends Variable { include() { if (!this.included) { this.included = true; + if (!this.module.isExecuted) { + markModuleAndImpureDependenciesAsExecuted(this.module); + } for (const declaration of this.declarations) { // If node is a default export, it can save a tree-shaking run to include the full declaration now if (!declaration.included) declaration.include(false); diff --git a/test/function/samples/pure-modules/array/_config.js b/test/function/samples/module-side-effects/array/_config.js similarity index 100% rename from test/function/samples/pure-modules/array/_config.js rename to test/function/samples/module-side-effects/array/_config.js diff --git a/test/function/samples/pure-modules/array/main.js b/test/function/samples/module-side-effects/array/main.js similarity index 100% rename from test/function/samples/pure-modules/array/main.js rename to test/function/samples/module-side-effects/array/main.js diff --git a/test/function/samples/pure-modules/external-false/_config.js b/test/function/samples/module-side-effects/external-false/_config.js similarity index 100% rename from test/function/samples/pure-modules/external-false/_config.js rename to test/function/samples/module-side-effects/external-false/_config.js diff --git a/test/function/samples/pure-modules/external-false/main.js b/test/function/samples/module-side-effects/external-false/main.js similarity index 100% rename from test/function/samples/pure-modules/external-false/main.js rename to test/function/samples/module-side-effects/external-false/main.js diff --git a/test/function/samples/pure-modules/global-false/_config.js b/test/function/samples/module-side-effects/global-false/_config.js similarity index 100% rename from test/function/samples/pure-modules/global-false/_config.js rename to test/function/samples/module-side-effects/global-false/_config.js diff --git a/test/function/samples/pure-modules/global-false/main.js b/test/function/samples/module-side-effects/global-false/main.js similarity index 100% rename from test/function/samples/pure-modules/global-false/main.js rename to test/function/samples/module-side-effects/global-false/main.js diff --git a/test/function/samples/pure-modules/invalid-option/_config.js b/test/function/samples/module-side-effects/invalid-option/_config.js similarity index 100% rename from test/function/samples/pure-modules/invalid-option/_config.js rename to test/function/samples/module-side-effects/invalid-option/_config.js diff --git a/test/function/samples/pure-modules/invalid-option/main.js b/test/function/samples/module-side-effects/invalid-option/main.js similarity index 100% rename from test/function/samples/pure-modules/invalid-option/main.js rename to test/function/samples/module-side-effects/invalid-option/main.js diff --git a/test/function/samples/pure-modules/load/_config.js b/test/function/samples/module-side-effects/load/_config.js similarity index 100% rename from test/function/samples/pure-modules/load/_config.js rename to test/function/samples/module-side-effects/load/_config.js diff --git a/test/function/samples/pure-modules/load/main.js b/test/function/samples/module-side-effects/load/main.js similarity index 100% rename from test/function/samples/pure-modules/load/main.js rename to test/function/samples/module-side-effects/load/main.js diff --git a/test/function/samples/module-side-effects/reexports/_config.js b/test/function/samples/module-side-effects/reexports/_config.js new file mode 100644 index 00000000000..3e5604fcb08 --- /dev/null +++ b/test/function/samples/module-side-effects/reexports/_config.js @@ -0,0 +1,17 @@ +const assert = require('assert'); +const sideEffects = []; + +module.exports = { + description: 'handles reexporting values when module side-effects are false', + context: { + sideEffects + }, + exports() { + assert.deepStrictEqual(sideEffects, ['dep1']); + }, + options: { + treeshake: { + moduleSideEffects: false + } + } +}; diff --git a/test/function/samples/module-side-effects/reexports/dep1.js b/test/function/samples/module-side-effects/reexports/dep1.js new file mode 100644 index 00000000000..46bc1ac980b --- /dev/null +++ b/test/function/samples/module-side-effects/reexports/dep1.js @@ -0,0 +1,3 @@ +sideEffects.push('dep1'); + +export const value = 'dep1'; diff --git a/test/function/samples/module-side-effects/reexports/dep2.js b/test/function/samples/module-side-effects/reexports/dep2.js new file mode 100644 index 00000000000..b2e6c9091ba --- /dev/null +++ b/test/function/samples/module-side-effects/reexports/dep2.js @@ -0,0 +1,3 @@ +sideEffects.push('dep2'); + +export const value = 'dep2'; diff --git a/test/function/samples/module-side-effects/reexports/lib.js b/test/function/samples/module-side-effects/reexports/lib.js new file mode 100644 index 00000000000..135998ae68f --- /dev/null +++ b/test/function/samples/module-side-effects/reexports/lib.js @@ -0,0 +1,4 @@ +sideEffects.push('main'); + +export { value as value1 } from './dep1.js'; +export { value as value2 } from './dep2.js'; diff --git a/test/function/samples/module-side-effects/reexports/main.js b/test/function/samples/module-side-effects/reexports/main.js new file mode 100644 index 00000000000..802983788a3 --- /dev/null +++ b/test/function/samples/module-side-effects/reexports/main.js @@ -0,0 +1 @@ +export { value1 } from './lib.js'; diff --git a/test/function/samples/pure-modules/resolve-id-external/_config.js b/test/function/samples/module-side-effects/resolve-id-external/_config.js similarity index 100% rename from test/function/samples/pure-modules/resolve-id-external/_config.js rename to test/function/samples/module-side-effects/resolve-id-external/_config.js diff --git a/test/function/samples/pure-modules/resolve-id-external/main.js b/test/function/samples/module-side-effects/resolve-id-external/main.js similarity index 100% rename from test/function/samples/pure-modules/resolve-id-external/main.js rename to test/function/samples/module-side-effects/resolve-id-external/main.js diff --git a/test/function/samples/pure-modules/resolve-id/_config.js b/test/function/samples/module-side-effects/resolve-id/_config.js similarity index 100% rename from test/function/samples/pure-modules/resolve-id/_config.js rename to test/function/samples/module-side-effects/resolve-id/_config.js diff --git a/test/function/samples/pure-modules/resolve-id/main.js b/test/function/samples/module-side-effects/resolve-id/main.js similarity index 100% rename from test/function/samples/pure-modules/resolve-id/main.js rename to test/function/samples/module-side-effects/resolve-id/main.js diff --git a/test/function/samples/pure-modules/transform/_config.js b/test/function/samples/module-side-effects/transform/_config.js similarity index 100% rename from test/function/samples/pure-modules/transform/_config.js rename to test/function/samples/module-side-effects/transform/_config.js diff --git a/test/function/samples/pure-modules/transform/main.js b/test/function/samples/module-side-effects/transform/main.js similarity index 100% rename from test/function/samples/pure-modules/transform/main.js rename to test/function/samples/module-side-effects/transform/main.js From fbf9bcee71c4d57123874ccc9397670326068cd2 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sun, 12 May 2019 09:44:58 +0200 Subject: [PATCH 16/19] Refine and simplify interaction of plugins with the user option. Now plugins always override the user option. --- docs/05-plugins.md | 8 ++++---- docs/999-big-list-of-options.md | 10 +++++----- src/ModuleLoader.ts | 15 +++++++++------ src/rollup/types.d.ts | 2 +- .../resolve-id-external/_config.js | 8 +++----- .../module-side-effects/resolve-id/_config.js | 10 ++++------ 6 files changed, 26 insertions(+), 27 deletions(-) diff --git a/docs/05-plugins.md b/docs/05-plugins.md index c8714ab86be..0cf88678aea 100644 --- a/docs/05-plugins.md +++ b/docs/05-plugins.md @@ -143,9 +143,9 @@ Cf. [`output.intro/output.outro`](guide/en#output-intro-output-outro). Type: `(id: string) => string | null | { code: string, map?: string | SourceMap, ast? : ESTree.Program, moduleSideEffects?: boolean | null }`
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). To prevent additional parsing time 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. +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 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. +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. @@ -244,7 +244,7 @@ resolveId(source) { } ``` -If `false` is returned for `moduleSideEffects` in the first hook that resolves a module id, the `treeshake.moduleSideEffects` option is not set to `true`, 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. +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`
@@ -272,7 +272,7 @@ Note that since this hook has access to the filename of the current chunk, its r Type: `(code: string, id: string) => string | null | { code: string, map?: string | SourceMap, ast? : ESTree.Program, moduleSideEffects?: boolean | null }`
Kind: `async, sequential` -Can be used to transform individual modules. To prevent additional parsing time 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. +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. diff --git a/docs/999-big-list-of-options.md b/docs/999-big-list-of-options.md index ee3df53fa1e..538136052c1 100755 --- a/docs/999-big-list-of-options.md +++ b/docs/999-big-list-of-options.md @@ -756,9 +756,9 @@ class Impure { ``` **treeshake.moduleSideEffects**
-Type: `boolean | null | "no-external" | string[] | (id: string, external: boolean) => boolean | null`
+Type: `boolean | "no-external" | string[] | (id: string, external: boolean) => boolean`
CLI: `--treeshake.moduleSideEffects`/`--no-treeshake.moduleSideEffects`
-Default: `null` +Default: `true` 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: @@ -770,7 +770,7 @@ console.log(42); ``` ```javascript -// output with treeshake.moduleSideEffects === null +// output with treeshake.moduleSideEffects === true import 'external-a'; import 'external-b'; console.log(42); @@ -793,7 +793,7 @@ console.log('side-effect'); ``` ```javascript -// output with treeshake.moduleSideEffects === null +// output with treeshake.moduleSideEffects === true console.log('side-effect'); console.log(42); @@ -804,7 +804,7 @@ console.log(42); console.log(42); ``` -The value `"no-external"` will only remove external imports if possible. You can also supply a list of modules with side-effects or a function to determine it for each module individually. Using `true` will behave the same as `null` except that it cannot be overridden by the `resolveId` hook of plugins. +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`
diff --git a/src/ModuleLoader.ts b/src/ModuleLoader.ts index f4f4ba6cb0c..57642891419 100644 --- a/src/ModuleLoader.ts +++ b/src/ModuleLoader.ts @@ -67,21 +67,21 @@ function getHasModuleSideEffects( pureExternalModules: PureModulesOption, graph: Graph ): (id: string, external: boolean) => boolean { - if (moduleSideEffectsOption === false) { - return () => false; + if (typeof moduleSideEffectsOption === 'boolean') { + return () => moduleSideEffectsOption; } if (moduleSideEffectsOption === 'no-external') { return (_id, external) => !external; } if (typeof moduleSideEffectsOption === 'function') { - return (id, ...args) => - !id.startsWith('\0') ? moduleSideEffectsOption(id, ...args) !== false : true; + return (id, external) => + !id.startsWith('\0') ? moduleSideEffectsOption(id, external) !== false : true; } if (Array.isArray(moduleSideEffectsOption)) { const ids = new Set(moduleSideEffectsOption); return id => ids.has(id); } - if (moduleSideEffectsOption && moduleSideEffectsOption !== true) { + if (moduleSideEffectsOption) { graph.warn( errInvalidOption( 'treeshake.moduleSideEffects', @@ -458,7 +458,10 @@ export class ModuleLoader { return { external, id, - moduleSideEffects: moduleSideEffects || this.hasModuleSideEffects(id, external) + moduleSideEffects: + typeof moduleSideEffects === 'boolean' + ? moduleSideEffects + : this.hasModuleSideEffects(id, external) }; } diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index 8060cf44f77..f8a8c98c2b4 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -172,7 +172,7 @@ export type IsExternal = (source: string, importer: string, isResolved: boolean) export type IsPureModule = (id: string) => boolean | void; -export type HasModuleSideEffects = (id: string, external: boolean) => boolean | void; +export type HasModuleSideEffects = (id: string, external: boolean) => boolean; export type LoadHook = ( this: PluginContext, diff --git a/test/function/samples/module-side-effects/resolve-id-external/_config.js b/test/function/samples/module-side-effects/resolve-id-external/_config.js index 47b0e2c18aa..e3ea26cb413 100644 --- a/test/function/samples/module-side-effects/resolve-id-external/_config.js +++ b/test/function/samples/module-side-effects/resolve-id-external/_config.js @@ -17,8 +17,6 @@ module.exports = { 'sideeffects-true-usereffects-false', 'sideeffects-true-usereffects-false-unused-import', 'sideeffects-true-usereffects-false-used-import', - 'sideeffects-false-usereffects-true', - 'sideeffects-false-usereffects-true-unused-import', 'sideeffects-false-usereffects-true-used-import', 'sideeffects-null-usereffects-true', 'sideeffects-null-usereffects-true-unused-import', @@ -58,9 +56,9 @@ module.exports = { hasModuleSideEffects: false }, { id: 'sideeffects-false-usereffects-false-used-import', hasModuleSideEffects: false }, - { id: 'sideeffects-false-usereffects-true', hasModuleSideEffects: true }, - { id: 'sideeffects-false-usereffects-true-unused-import', hasModuleSideEffects: true }, - { id: 'sideeffects-false-usereffects-true-used-import', hasModuleSideEffects: true }, + { id: 'sideeffects-false-usereffects-true', hasModuleSideEffects: false }, + { id: 'sideeffects-false-usereffects-true-unused-import', hasModuleSideEffects: false }, + { id: 'sideeffects-false-usereffects-true-used-import', hasModuleSideEffects: false }, { id: 'sideeffects-null-usereffects-false', hasModuleSideEffects: false }, { id: 'sideeffects-null-usereffects-false-unused-import', hasModuleSideEffects: false }, { id: 'sideeffects-null-usereffects-false-used-import', hasModuleSideEffects: false }, diff --git a/test/function/samples/module-side-effects/resolve-id/_config.js b/test/function/samples/module-side-effects/resolve-id/_config.js index 5bb328351c3..1c23ce2ad35 100644 --- a/test/function/samples/module-side-effects/resolve-id/_config.js +++ b/test/function/samples/module-side-effects/resolve-id/_config.js @@ -14,8 +14,6 @@ module.exports = { 'sideeffects-true-usereffects-false', 'sideeffects-true-usereffects-false-unused-import', 'sideeffects-true-usereffects-false-used-import', - 'sideeffects-false-usereffects-true', - 'sideeffects-false-usereffects-true-unused-import', 'sideeffects-false-usereffects-true-used-import', 'sideeffects-null-usereffects-true', 'sideeffects-null-usereffects-true-unused-import', @@ -48,7 +46,7 @@ module.exports = { const userEffects = JSON.parse(id.split('-')[3]); assert.strictEqual( this.getModuleInfo(id).hasModuleSideEffects, - sideEffects || userEffects + typeof sideEffects === 'boolean' ? sideEffects : userEffects ); return `export const value = '${id}'; sideEffects.push(value);`; } @@ -66,9 +64,9 @@ module.exports = { hasModuleSideEffects: false }, { id: 'sideeffects-false-usereffects-false-used-import', hasModuleSideEffects: false }, - { id: 'sideeffects-false-usereffects-true', hasModuleSideEffects: true }, - { id: 'sideeffects-false-usereffects-true-unused-import', hasModuleSideEffects: true }, - { id: 'sideeffects-false-usereffects-true-used-import', hasModuleSideEffects: true }, + { id: 'sideeffects-false-usereffects-true', hasModuleSideEffects: false }, + { id: 'sideeffects-false-usereffects-true-unused-import', hasModuleSideEffects: false }, + { id: 'sideeffects-false-usereffects-true-used-import', hasModuleSideEffects: false }, { id: 'sideeffects-null-usereffects-false', hasModuleSideEffects: false }, { id: 'sideeffects-null-usereffects-false-unused-import', hasModuleSideEffects: false }, { id: 'sideeffects-null-usereffects-false-used-import', hasModuleSideEffects: false }, From e0ed3b93f243481184c86771ce57fbcf2eab622e Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sun, 12 May 2019 18:42:37 +0200 Subject: [PATCH 17/19] Add an option for this.resolve to skip the plugin calling it --- docs/05-plugins.md | 4 +- src/ModuleLoader.ts | 4 +- src/rollup/types.d.ts | 6 ++- src/utils/pluginDriver.ts | 16 ++++--- .../context-resolve-skipself/_config.js | 42 +++++++++++++++++++ .../context-resolve-skipself/existing.js | 1 + .../samples/context-resolve-skipself/main.js | 16 +++++++ 7 files changed, 80 insertions(+), 9 deletions(-) create mode 100644 test/function/samples/context-resolve-skipself/_config.js create mode 100644 test/function/samples/context-resolve-skipself/existing.js create mode 100644 test/function/samples/context-resolve-skipself/main.js diff --git a/docs/05-plugins.md b/docs/05-plugins.md index 0cf88678aea..42bb2ff8107 100644 --- a/docs/05-plugins.md +++ b/docs/05-plugins.md @@ -386,9 +386,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. diff --git a/src/ModuleLoader.ts b/src/ModuleLoader.ts index 57642891419..ea300ccc28b 100644 --- a/src/ModuleLoader.ts +++ b/src/ModuleLoader.ts @@ -211,11 +211,11 @@ export class ModuleLoader { return fileName; } - resolveId(source: string, importer: string): Promise { + resolveId(source: string, importer: string, skip?: number | null): Promise { return Promise.resolve( this.isExternal(source, importer, false) ? { id: source, external: true } - : this.pluginDriver.hookFirst('resolveId', [source, importer]) + : this.pluginDriver.hookFirst('resolveId', [source, importer], null, skip) ).then((result: ResolveIdResult) => this.normalizeResolveIdResult(result, importer, source)); } diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index f8a8c98c2b4..70045292460 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -131,7 +131,11 @@ export interface PluginContext extends MinimalPluginContext { isExternal: IsExternal; moduleIds: IterableIterator; parse: (input: string, options: any) => ESTree.Program; - resolve: (source: string, importer: string) => Promise; + resolve: ( + source: string, + importer: string, + options?: { skipSelf: boolean } + ) => Promise; /** @deprecated Use `this.resolve` instead */ resolveId: (source: string, importer: string) => Promise; setAssetSource: (assetReferenceId: string, source: string | Buffer) => void; diff --git a/src/utils/pluginDriver.ts b/src/utils/pluginDriver.ts index a08f934c21b..848386e8066 100644 --- a/src/utils/pluginDriver.ts +++ b/src/utils/pluginDriver.ts @@ -32,7 +32,8 @@ export interface PluginDriver { hookFirst>( hook: H, args: Args, - hookContext?: HookContext + hookContext?: HookContext | null, + skip?: number ): Promise; hookFirstSync>( hook: H, @@ -194,8 +195,12 @@ export function createPluginDriver( .resolveId(source, importer) .then(resolveId => resolveId && resolveId.id); }, - resolve(source, importer) { - return graph.moduleLoader.resolveId(source, importer); + resolve(source, importer, options?: { skipSelf: boolean }) { + return graph.moduleLoader.resolveId( + source, + importer, + options && options.skipSelf ? pidx : null + ); }, setAssetSource, warn(warning) { @@ -265,7 +270,7 @@ export function createPluginDriver( args: any[], pluginIndex: number, permitValues = false, - hookContext?: HookContext + hookContext?: HookContext | null ): Promise { const plugin = plugins[pluginIndex]; let context = pluginContexts[pluginIndex]; @@ -325,9 +330,10 @@ export function createPluginDriver( }, // chains, first non-null result stops and returns - hookFirst(name, args, hookContext) { + hookFirst(name, args, hookContext, skip) { let promise: Promise = Promise.resolve(); for (let i = 0; i < plugins.length; i++) { + if (skip === i) continue; promise = promise.then((result: any) => { if (result != null) return result; return runHook(name, args, i, false, hookContext); diff --git a/test/function/samples/context-resolve-skipself/_config.js b/test/function/samples/context-resolve-skipself/_config.js new file mode 100644 index 00000000000..8ed89faf6e3 --- /dev/null +++ b/test/function/samples/context-resolve-skipself/_config.js @@ -0,0 +1,42 @@ +const path = require('path'); + +module.exports = { + description: 'allows a plugin to skip its own resolveId hook when using this.resolve', + options: { + plugins: [ + { + resolveId(id) { + if (id === 'resolutions') { + return id; + } + if (id.startsWith('test')) { + return 'own-resolution'; + } + }, + load(id) { + if (id === 'resolutions') { + const importer = path.resolve(__dirname, 'main.js'); + return Promise.all([ + this.resolve('test', importer).then(result => ({ id: result.id, text: 'all' })), + this.resolve('test', importer, { skipSelf: false }).then(result => ({ + id: result.id, + text: 'unskipped' + })), + this.resolve('test', importer, { skipSelf: true }).then(result => ({ + id: result.id, + text: 'skipped' + })) + ]).then(result => `export default ${JSON.stringify(result)};`); + } + } + }, + { + resolveId(id) { + if (id.startsWith('test')) { + return 'other-resolution'; + } + } + } + ] + } +}; diff --git a/test/function/samples/context-resolve-skipself/existing.js b/test/function/samples/context-resolve-skipself/existing.js new file mode 100644 index 00000000000..9486260537f --- /dev/null +++ b/test/function/samples/context-resolve-skipself/existing.js @@ -0,0 +1 @@ +console.log('existing'); diff --git a/test/function/samples/context-resolve-skipself/main.js b/test/function/samples/context-resolve-skipself/main.js new file mode 100644 index 00000000000..a0be821e3ed --- /dev/null +++ b/test/function/samples/context-resolve-skipself/main.js @@ -0,0 +1,16 @@ +import resolutions from 'resolutions'; + +assert.deepStrictEqual(resolutions, [ + { + id: 'own-resolution', + text: 'all' + }, + { + id: 'own-resolution', + text: 'unskipped' + }, + { + id: 'other-resolution', + text: 'skipped' + } +]); From d73d1008efddf286228cb7b8a79deb410593a644 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Mon, 13 May 2019 06:38:14 +0200 Subject: [PATCH 18/19] Add "isEntry" to "getModuleInfo" --- docs/05-plugins.md | 1 + src/rollup/types.d.ts | 1 + src/utils/pluginDriver.ts | 11 +++++++---- .../samples/plugin-module-information/_config.js | 4 ++++ 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/docs/05-plugins.md b/docs/05-plugins.md index 42bb2ff8107..80d43290036 100644 --- a/docs/05-plugins.md +++ b/docs/05-plugins.md @@ -360,6 +360,7 @@ Returns additional information about the module in question in the form ``` { 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 diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index 70045292460..eec9479c6ab 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -125,6 +125,7 @@ export interface PluginContext extends MinimalPluginContext { hasModuleSideEffects: boolean; id: string; importedIds: string[]; + isEntry: boolean; isExternal: boolean; }; /** @deprecated Use `this.resolve` instead */ diff --git a/src/utils/pluginDriver.ts b/src/utils/pluginDriver.ts index 848386e8066..0101233a8df 100644 --- a/src/utils/pluginDriver.ts +++ b/src/utils/pluginDriver.ts @@ -1,5 +1,6 @@ import { EventEmitter } from 'events'; import { version as rollupVersion } from 'package.json'; +import ExternalModule from '../ExternalModule'; import Graph from '../Graph'; import Module from '../Module'; import { @@ -177,10 +178,12 @@ export function createPluginDriver( return { hasModuleSideEffects: foundModule.moduleSideEffects, id: foundModule.id, - importedIds: foundModule.isExternal - ? [] - : (foundModule as Module).sources.map(id => (foundModule as Module).resolvedIds[id].id), - isExternal: !!foundModule.isExternal + importedIds: + foundModule instanceof ExternalModule + ? [] + : foundModule.sources.map(id => foundModule.resolvedIds[id].id), + isEntry: foundModule instanceof Module && foundModule.isEntryPoint, + isExternal: foundModule instanceof ExternalModule }; }, meta: { diff --git a/test/function/samples/plugin-module-information/_config.js b/test/function/samples/plugin-module-information/_config.js index 9ba23d507c4..d0d85651590 100644 --- a/test/function/samples/plugin-module-information/_config.js +++ b/test/function/samples/plugin-module-information/_config.js @@ -21,24 +21,28 @@ module.exports = { hasModuleSideEffects: true, id: ID_MAIN, importedIds: [ID_FOO, ID_NESTED], + isEntry: true, isExternal: false }); assert.deepEqual(this.getModuleInfo(ID_FOO), { hasModuleSideEffects: true, id: ID_FOO, importedIds: [ID_PATH], + isEntry: false, isExternal: false }); assert.deepEqual(this.getModuleInfo(ID_NESTED), { hasModuleSideEffects: true, id: ID_NESTED, importedIds: [ID_FOO], + isEntry: false, isExternal: false }); assert.deepEqual(this.getModuleInfo(ID_PATH), { hasModuleSideEffects: true, id: ID_PATH, importedIds: [], + isEntry: false, isExternal: true }); } From 1ef16d89f6ae8fc452ca64ca8c2cf8115c9483db Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Mon, 13 May 2019 07:12:44 +0200 Subject: [PATCH 19/19] Provide "isEntry" information already in the load hook. --- src/Module.ts | 5 +- src/ModuleLoader.ts | 32 +++++--- src/utils/executionOrder.ts | 1 - .../plugin-module-information/_config.js | 77 ++++++++++--------- 4 files changed, 68 insertions(+), 47 deletions(-) diff --git a/src/Module.ts b/src/Module.ts index 87137446bcb..afd720b00dc 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -188,7 +188,7 @@ export default class Module { importDescriptions: { [name: string]: ImportDescription } = Object.create(null); importMetas: MetaProperty[] = []; imports = new Set(); - isEntryPoint = false; + isEntryPoint: boolean; isExecuted = false; isExternal: false; isUserDefinedEntryPoint = false; @@ -213,12 +213,13 @@ export default class Module { private namespaceVariable: NamespaceVariable = undefined; private transformDependencies: string[]; - constructor(graph: Graph, id: string, moduleSideEffects: boolean) { + constructor(graph: Graph, id: string, moduleSideEffects: boolean, isEntry: boolean) { this.id = id; this.graph = graph; this.excludeFromSourcemap = /\0/.test(id); this.context = graph.getModuleContext(id); this.moduleSideEffects = moduleSideEffects; + this.isEntryPoint = isEntry; } basename() { diff --git a/src/ModuleLoader.ts b/src/ModuleLoader.ts index ea300ccc28b..a729469fc20 100644 --- a/src/ModuleLoader.ts +++ b/src/ModuleLoader.ts @@ -159,7 +159,9 @@ export class ModuleLoader { newEntryModules: Module[]; }> { const loadNewEntryModulesPromise = Promise.all( - unresolvedEntryModules.map(this.loadEntryModule) + unresolvedEntryModules.map(unresolvedEntryModule => + this.loadEntryModule(unresolvedEntryModule, true) + ) ).then(entryModules => { for (const entryModule of entryModules) { entryModule.isUserDefinedEntryPoint = entryModule.isUserDefinedEntryPoint || isUserDefined; @@ -186,7 +188,9 @@ export class ModuleLoader { } } const loadNewManualChunkModulesPromise = Promise.all( - unresolvedManualChunks.map(this.loadEntryModule) + unresolvedManualChunks.map(unresolvedManualChunk => + this.loadEntryModule(unresolvedManualChunk, false) + ) ).then(manualChunkModules => { for (let index = 0; index < manualChunkModules.length; index++) { this.addToManualChunk( @@ -275,14 +279,21 @@ export class ModuleLoader { ).then(() => fetchDynamicImportsPromise); } - private fetchModule(id: string, importer: string, moduleSideEffects: boolean): Promise { + private fetchModule( + id: string, + importer: string, + moduleSideEffects: boolean, + isEntry: boolean + ): Promise { const existingModule = this.modulesById.get(id); if (existingModule) { - if (existingModule.isExternal) throw new Error(`Cannot fetch external module ${id}`); - return Promise.resolve(existingModule); + if (existingModule instanceof ExternalModule) + throw new Error(`Cannot fetch external module ${id}`); + existingModule.isEntryPoint = existingModule.isEntryPoint || isEntry; + return Promise.resolve(existingModule); } - const module: Module = new Module(this.graph, id, moduleSideEffects); + const module: Module = new Module(this.graph, id, moduleSideEffects, isEntry); this.modulesById.set(id, module); const manualChunkAlias = this.getManualChunk(id); if (typeof manualChunkAlias === 'string') { @@ -374,7 +385,7 @@ export class ModuleLoader { } return Promise.resolve(externalModule); } else { - return this.fetchModule(resolvedId.id, importer, resolvedId.moduleSideEffects); + return this.fetchModule(resolvedId.id, importer, resolvedId.moduleSideEffects, false); } } @@ -393,7 +404,10 @@ export class ModuleLoader { return resolvedId; } - private loadEntryModule = ({ alias, unresolvedId }: UnresolvedModuleWithAlias): Promise => + private loadEntryModule = ( + { alias, unresolvedId }: UnresolvedModuleWithAlias, + isEntry: boolean + ): Promise => this.pluginDriver .hookFirst('resolveId', [unresolvedId, undefined]) .then((resolveIdResult: ResolveIdResult) => { @@ -409,7 +423,7 @@ export class ModuleLoader { : resolveIdResult; if (typeof id === 'string') { - return this.fetchModule(id, undefined, true).then(module => { + return this.fetchModule(id, undefined, true, isEntry).then(module => { if (alias !== null) { if (module.chunkAlias !== null && module.chunkAlias !== alias) { error(errCannotAssignModuleToChunk(module.id, alias, module.chunkAlias)); diff --git a/src/utils/executionOrder.ts b/src/utils/executionOrder.ts index c09f259472f..fb02212b2ae 100644 --- a/src/utils/executionOrder.ts +++ b/src/utils/executionOrder.ts @@ -53,7 +53,6 @@ export function analyseModuleExecution(entryModules: Module[]) { }; for (const curEntry of entryModules) { - curEntry.isEntryPoint = true; if (!parents[curEntry.id]) { parents[curEntry.id] = null; analyseModule(curEntry); diff --git a/test/function/samples/plugin-module-information/_config.js b/test/function/samples/plugin-module-information/_config.js index d0d85651590..6a9a20b4734 100644 --- a/test/function/samples/plugin-module-information/_config.js +++ b/test/function/samples/plugin-module-information/_config.js @@ -12,42 +12,49 @@ module.exports = { description: 'provides module information on the plugin context', options: { external: ['path'], - plugins: [ - { - renderStart() { - rendered = true; - assert.deepEqual(Array.from(this.moduleIds), [ID_MAIN, ID_FOO, ID_NESTED, ID_PATH]); - assert.deepEqual(this.getModuleInfo(ID_MAIN), { - hasModuleSideEffects: true, - id: ID_MAIN, - importedIds: [ID_FOO, ID_NESTED], - isEntry: true, - isExternal: false - }); - assert.deepEqual(this.getModuleInfo(ID_FOO), { - hasModuleSideEffects: true, - id: ID_FOO, - importedIds: [ID_PATH], - isEntry: false, - isExternal: false - }); - assert.deepEqual(this.getModuleInfo(ID_NESTED), { - hasModuleSideEffects: true, - id: ID_NESTED, - importedIds: [ID_FOO], - isEntry: false, - isExternal: false - }); - assert.deepEqual(this.getModuleInfo(ID_PATH), { - hasModuleSideEffects: true, - id: ID_PATH, - importedIds: [], - isEntry: false, - isExternal: true - }); - } + plugins: { + load(id) { + assert.deepStrictEqual(this.getModuleInfo(id), { + hasModuleSideEffects: true, + id, + importedIds: [], + isEntry: id === ID_MAIN, + isExternal: false + }); + }, + renderStart() { + rendered = true; + assert.deepStrictEqual(Array.from(this.moduleIds), [ID_MAIN, ID_FOO, ID_NESTED, ID_PATH]); + assert.deepStrictEqual(this.getModuleInfo(ID_MAIN), { + hasModuleSideEffects: true, + id: ID_MAIN, + importedIds: [ID_FOO, ID_NESTED], + isEntry: true, + isExternal: false + }); + assert.deepStrictEqual(this.getModuleInfo(ID_FOO), { + hasModuleSideEffects: true, + id: ID_FOO, + importedIds: [ID_PATH], + isEntry: false, + isExternal: false + }); + assert.deepStrictEqual(this.getModuleInfo(ID_NESTED), { + hasModuleSideEffects: true, + id: ID_NESTED, + importedIds: [ID_FOO], + isEntry: false, + isExternal: false + }); + assert.deepStrictEqual(this.getModuleInfo(ID_PATH), { + hasModuleSideEffects: true, + id: ID_PATH, + importedIds: [], + isEntry: false, + isExternal: true + }); } - ] + } }, bundle() { assert.ok(rendered);