Skip to content

Commit e0b1060

Browse files
lukastaegertlukastaegert
authored
Deprecate hasModuleSideEffects in favor of moduleSideEffects and ensure it is mutable on ModuleInfo (#4379)
* Deprecate hasModuleSideEffects in favor of moduleSideEffects and ensure it is mutable on ModuleInfo * Add notes about ModuleInfo reliability
1 parent d57cfa0 commit e0b1060

45 files changed

Lines changed: 652 additions & 129 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎docs/05-plugin-development.md‎

Lines changed: 51 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -167,33 +167,58 @@ For those cases, the `isEntry` option will tell you if we are resolving a user d
167167
You can use this for instance as a mechanism to define custom proxy modules for entry points. The following plugin will proxy all entry points to inject a polyfill import.
168168

169169
```js
170+
// We prefix the polyfill id with \0 to tell other plugins not to try to load or
171+
// transform it
172+
const POLYFILL_ID = '\0polyfill';
173+
const PROXY_SUFFIX = '?inject-polyfill-proxy';
174+
170175
function injectPolyfillPlugin() {
171176
return {
172177
name: 'inject-polyfill',
173178
async resolveId(source, importer, options) {
179+
if (source === POLYFILL_ID) {
180+
// It is important that side effects are always respected for polyfills,
181+
// otherwise using "treeshake.moduleSideEffects: false" may prevent the
182+
// polyfill from being included.
183+
return { id: POLYFILL_ID, moduleSideEffects: true };
184+
}
174185
if (options.isEntry) {
175-
// We need to skip this plugin to avoid an infinite loop
186+
// Determine what the actual entry would have been. We need "skipSelf"
187+
// to avoid an infinite loop.
176188
const resolution = await this.resolve(source, importer, { skipSelf: true, ...options });
177189
// If it cannot be resolved or is external, just return it so that
178190
// Rollup can display an error
179191
if (!resolution || resolution.external) return resolution;
180-
// In the load hook of the proxy, we want to use this.load to find out
181-
// if the entry has a default export. In the load hook, however, we no
182-
// longer have the full "resolution" object that may contain meta-data
183-
// from other plugins that is only added on first load. Therefore we
184-
// trigger loading here without waiting for it.
185-
this.load(resolution);
186-
return `${resolution.id}?entry-proxy`;
192+
// In the load hook of the proxy, we need to know if the entry has a
193+
// default export. There, however, we no longer have the full
194+
// "resolution" object that may contain meta-data from other plugins
195+
// that is only added on first load. Therefore we trigger loading here.
196+
const moduleInfo = await this.load(resolution);
197+
// We need to make sure side effects in the original entry point
198+
// are respected even for treeshake.moduleSideEffects: false.
199+
// "moduleSideEffects" is a writable property on ModuleInfo.
200+
moduleInfo.moduleSideEffects = true;
201+
// It is important that the new entry does not start with \0 and
202+
// has the same directory as the original one to not mess up
203+
// relative external import generation. Also keeping the name and
204+
// just adding a "?query" to the end ensures that preserveModules
205+
// will generate the original entry name for this entry.
206+
return `${resolution.id}${PROXY_SUFFIX}`;
187207
}
188208
return null;
189209
},
190-
async load(id) {
191-
if (id.endsWith('?entry-proxy')) {
192-
const entryId = id.slice(0, -'?entry-proxy'.length);
193-
// We need to load and parse the original entry first because we need
194-
// to know if it has a default export
195-
const { hasDefaultExport } = await this.load({ id: entryId });
196-
let code = `import 'polyfill';export * from ${JSON.stringify(entryId)};`;
210+
load(id) {
211+
if (id === POLYFILL_ID) {
212+
// Replace with actual polyfill
213+
return "console.log('polyfill');";
214+
}
215+
if (id.endsWith(PROXY_SUFFIX)) {
216+
const entryId = id.slice(0, -PROXY_SUFFIX.length);
217+
// We know ModuleInfo.hasDefaultExport is reliable because we awaited
218+
// this.load in resolveId
219+
const { hasDefaultExport } = this.getModuleInfo(entryId);
220+
let code =
221+
`import ${JSON.stringify(POLYFILL_ID)};` + `export * from ${JSON.stringify(entryId)};`;
197222
// Namespace reexports do not reexport default, so we need special
198223
// handling here
199224
if (hasDefaultExport) {
@@ -700,8 +725,8 @@ type ModuleInfo = {
700725
dynamicImporters: string[]; // the ids of all modules that import this module via dynamic import()
701726
implicitlyLoadedAfterOneOf: string[]; // implicit relationships, declared via this.emitFile
702727
implicitlyLoadedBefore: string[]; // implicit relationships, declared via this.emitFile
703-
hasModuleSideEffects: boolean | 'no-treeshake'; // are imports of this module included if nothing is imported from it
704728
meta: { [plugin: string]: any }; // custom module meta-data
729+
moduleSideEffects: boolean | 'no-treeshake'; // are imports of this module included if nothing is imported from it
705730
syntheticNamedExports: boolean | string; // final value of synthetic named exports
706731
};
707732

@@ -714,7 +739,16 @@ type ResolvedId = {
714739
};
715740
```
716741
717-
During the build, this object represents currently available information about the module. Before the [`buildEnd`](guide/en/#buildend) hook, this information may be incomplete as e.g. the `importedIds` are not yet resolved or additional `importers` are discovered.
742+
During the build, this object represents currently available information about the module which may be inaccurate before the [`buildEnd`](guide/en/#buildend) hook:
743+
744+
- `id` and `isExternal` will never change.
745+
- `code`, `ast` and `hasDefaultExport` are only available after parsing, i.e. in the [`moduleParsed`](guide/en/#moduleparsed) hook or after awaiting [`this.load`](guide/en/#thisload). At that point, they will no longer change.
746+
- if `isEntry` is `true`, it will no longer change. It is however possible for modules to become entry points after they are parsed, either via [`this.emitFile`](guide/en/#thisemitfile) or because a plugin inspects a potential entry point via [`this.load`](guide/en/#thisload) in the [`resolveId`](guide/en/#resolveid) hook when resolving an entry point. Therefore, it is not recommended relying on this flag in the [`transform`](guide/en/#transform) hook. It will no longer change after `buildEnd`.
747+
- Similarly, `implicitlyLoadedAfterOneOf` can receive additional entries at any time before `buildEnd` via [`this.emitFile`](guide/en/#thisemitfile).
748+
- `importers`, `dynamicImporters` and `implicitlyLoadedBefore` will start as empty arrays, which receive additional entries as new importers and implicit dependents are discovered. They will no longer change after `buildEnd`.
749+
- `isIncluded` is only available after `buildEnd`, at which point it will no longer change.
750+
- `importedIds`, `importedIdResolutions`, `dynamicallyImportedIds` and `dynamicallyImportedIdResolutions` are available when a module has been parsed and its dependencies have been resolved. This is the case in the `moduleParsed` hook or after awaiting [`this.load`](guide/en/#thisload) with the `resolveDependencies` flag. At that point, they will no longer change.
751+
- `meta`, `moduleSideEffects` and `syntheticNamedExports` can be changed by [`load`](guide/en/#load) and [`transform`](guide/en/#transform) hooks. Moreover, while most properties are read-only, `moduleSideEffects` is writable and changes will be picked up if they occur before the `buildEnd` hook is triggered. `meta` should not be overwritten, but it is ok to mutate its properties at any time to store meta information about a module. The advantage of doing this instead of keeping state in a plugin is that `meta` is persisted to and restored from the cache if it is used, e.g. when using watch mode from the CLI.
718752
719753
Returns `null` if the module id cannot be found.
720754

‎src/Chunk.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ export default class Chunk {
241241
}
242242
if (
243243
!chunk.dependencies.has(chunkByModule.get(facadedModule)!) &&
244-
facadedModule.info.hasModuleSideEffects &&
244+
facadedModule.info.moduleSideEffects &&
245245
facadedModule.hasEffects()
246246
) {
247247
chunk.dependencies.add(chunkByModule.get(facadedModule)!);

‎src/ExternalModule.ts‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type {
66
NormalizedOutputOptions
77
} from './rollup/types';
88
import { EMPTY_ARRAY } from './utils/blank';
9+
import { warnDeprecation } from './utils/error';
910
import { makeLegal } from './utils/identifierHelpers';
1011
import { normalize, relative } from './utils/path';
1112
import { printQuotedStringList } from './utils/printStringList';
@@ -31,14 +32,14 @@ export default class ExternalModule {
3132
constructor(
3233
private readonly options: NormalizedInputOptions,
3334
public readonly id: string,
34-
hasModuleSideEffects: boolean | 'no-treeshake',
35+
moduleSideEffects: boolean | 'no-treeshake',
3536
meta: CustomPluginOptions,
3637
public readonly renormalizeRenderPath: boolean
3738
) {
3839
this.suggestedVariableName = makeLegal(id.split(/[\\/]/).pop()!);
3940

4041
const { importers, dynamicImporters } = this;
41-
this.info = {
42+
const info: ModuleInfo = (this.info = {
4243
ast: null,
4344
code: null,
4445
dynamicallyImportedIdResolutions: EMPTY_ARRAY,
@@ -47,7 +48,14 @@ export default class ExternalModule {
4748
return dynamicImporters.sort();
4849
},
4950
hasDefaultExport: null,
50-
hasModuleSideEffects,
51+
get hasModuleSideEffects() {
52+
warnDeprecation(
53+
'Accessing ModuleInfo.hasModuleSideEffects from plugins is deprecated. Please use ModuleInfo.moduleSideEffects instead.',
54+
false,
55+
options
56+
);
57+
return info.moduleSideEffects;
58+
},
5159
id,
5260
implicitlyLoadedAfterOneOf: EMPTY_ARRAY,
5361
implicitlyLoadedBefore: EMPTY_ARRAY,
@@ -60,8 +68,14 @@ export default class ExternalModule {
6068
isExternal: true,
6169
isIncluded: null,
6270
meta,
71+
moduleSideEffects,
6372
syntheticNamedExports: false
64-
};
73+
});
74+
// Hide the deprecated key so that it only warns when accessed explicitly
75+
Object.defineProperty(this.info, 'hasModuleSideEffects', {
76+
...Object.getOwnPropertyDescriptor(this.info, 'hasModuleSideEffects'),
77+
enumerable: false
78+
});
6579
}
6680

6781
getVariableForExportName(name: string): [variable: ExternalVariable] {

‎src/Graph.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ export default class Graph {
189189
this.needsTreeshakingPass = false;
190190
for (const module of this.modules) {
191191
if (module.isExecuted) {
192-
if (module.info.hasModuleSideEffects === 'no-treeshake') {
192+
if (module.info.moduleSideEffects === 'no-treeshake') {
193193
module.includeAllInBundle();
194194
} else {
195195
module.include();

‎src/Module.ts‎

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ import {
5454
errMissingExport,
5555
errNamespaceConflict,
5656
error,
57-
errSyntheticNamedExportsNeedNamespaceExport
57+
errSyntheticNamedExportsNeedNamespaceExport,
58+
warnDeprecation
5859
} from './utils/error';
5960
import { getId } from './utils/getId';
6061
import { getOrCreate } from './utils/getOrCreate';
@@ -247,7 +248,7 @@ export default class Module {
247248
public readonly id: string,
248249
private readonly options: NormalizedInputOptions,
249250
isEntry: boolean,
250-
hasModuleSideEffects: boolean | 'no-treeshake',
251+
moduleSideEffects: boolean | 'no-treeshake',
251252
syntheticNamedExports: boolean | string,
252253
meta: CustomPluginOptions
253254
) {
@@ -257,61 +258,83 @@ export default class Module {
257258

258259
// eslint-disable-next-line @typescript-eslint/no-this-alias
259260
const module = this;
261+
const {
262+
dynamicImports,
263+
dynamicImporters,
264+
reexportDescriptions,
265+
implicitlyLoadedAfter,
266+
implicitlyLoadedBefore,
267+
sources,
268+
importers
269+
} = this;
260270
this.info = {
261271
ast: null,
262272
code: null,
263273
get dynamicallyImportedIdResolutions() {
264-
return module.dynamicImports
274+
return dynamicImports
265275
.map(({ argument }) => typeof argument === 'string' && module.resolvedIds[argument])
266276
.filter(Boolean) as ResolvedId[];
267277
},
268278
get dynamicallyImportedIds() {
269279
const dynamicallyImportedIds: string[] = [];
270-
for (const { id } of module.dynamicImports) {
280+
for (const { id } of dynamicImports) {
271281
if (id) {
272282
dynamicallyImportedIds.push(id);
273283
}
274284
}
275285
return dynamicallyImportedIds;
276286
},
277287
get dynamicImporters() {
278-
return module.dynamicImporters.sort();
288+
return dynamicImporters.sort();
279289
},
280290
get hasDefaultExport() {
281291
// This information is only valid after parsing
282292
if (!module.ast) {
283293
return null;
284294
}
285-
return 'default' in module.exports || 'default' in module.reexportDescriptions;
295+
return 'default' in module.exports || 'default' in reexportDescriptions;
296+
},
297+
get hasModuleSideEffects() {
298+
warnDeprecation(
299+
'Accessing ModuleInfo.hasModuleSideEffects from plugins is deprecated. Please use ModuleInfo.moduleSideEffects instead.',
300+
false,
301+
options
302+
);
303+
return module.info.moduleSideEffects;
286304
},
287-
hasModuleSideEffects,
288305
id,
289306
get implicitlyLoadedAfterOneOf() {
290-
return Array.from(module.implicitlyLoadedAfter, getId).sort();
307+
return Array.from(implicitlyLoadedAfter, getId).sort();
291308
},
292309
get implicitlyLoadedBefore() {
293-
return Array.from(module.implicitlyLoadedBefore, getId).sort();
310+
return Array.from(implicitlyLoadedBefore, getId).sort();
294311
},
295312
get importedIdResolutions() {
296-
return Array.from(module.sources, source => module.resolvedIds[source]).filter(Boolean);
313+
return Array.from(sources, source => module.resolvedIds[source]).filter(Boolean);
297314
},
298315
get importedIds() {
299-
return Array.from(module.sources, source => module.resolvedIds[source]?.id).filter(Boolean);
316+
return Array.from(sources, source => module.resolvedIds[source]?.id).filter(Boolean);
300317
},
301318
get importers() {
302-
return module.importers.sort();
319+
return importers.sort();
303320
},
304321
isEntry,
305322
isExternal: false,
306323
get isIncluded() {
307-
if (module.graph.phase !== BuildPhase.GENERATE) {
324+
if (graph.phase !== BuildPhase.GENERATE) {
308325
return null;
309326
}
310327
return module.isIncluded();
311328
},
312329
meta: { ...meta },
330+
moduleSideEffects,
313331
syntheticNamedExports
314332
};
333+
// Hide the deprecated key so that it only warns when accessed explicitly
334+
Object.defineProperty(this.info, 'hasModuleSideEffects', {
335+
...Object.getOwnPropertyDescriptor(this.info, 'hasModuleSideEffects'),
336+
enumerable: false
337+
});
315338
}
316339

317340
basename(): string {
@@ -393,7 +416,7 @@ export default class Module {
393416
}
394417
necessaryDependencies.add(variable.module!);
395418
}
396-
if (!this.options.treeshake || this.info.hasModuleSideEffects === 'no-treeshake') {
419+
if (!this.options.treeshake || this.info.moduleSideEffects === 'no-treeshake') {
397420
for (const dependency of this.dependencies) {
398421
relevantDependencies.add(dependency);
399422
}
@@ -600,7 +623,7 @@ export default class Module {
600623

601624
hasEffects(): boolean {
602625
return (
603-
this.info.hasModuleSideEffects === 'no-treeshake' ||
626+
this.info.moduleSideEffects === 'no-treeshake' ||
604627
(this.ast!.included && this.ast!.hasEffects(createHasEffectsContext()))
605628
);
606629
}
@@ -766,7 +789,7 @@ export default class Module {
766789
dependencies: Array.from(this.dependencies, getId),
767790
id: this.id,
768791
meta: this.info.meta,
769-
moduleSideEffects: this.info.hasModuleSideEffects,
792+
moduleSideEffects: this.info.moduleSideEffects,
770793
originalCode: this.originalCode,
771794
originalSourcemap: this.originalSourcemap,
772795
resolvedIds: this.resolvedIds,
@@ -835,7 +858,7 @@ export default class Module {
835858
syntheticNamedExports
836859
}: Partial<PartialNull<ModuleOptions>>): void {
837860
if (moduleSideEffects != null) {
838-
this.info.hasModuleSideEffects = moduleSideEffects;
861+
this.info.moduleSideEffects = moduleSideEffects;
839862
}
840863
if (syntheticNamedExports != null) {
841864
this.info.syntheticNamedExports = syntheticNamedExports;
@@ -1008,7 +1031,7 @@ export default class Module {
10081031
relevantDependencies.add(dependency);
10091032
continue;
10101033
}
1011-
if (!(dependency.info.hasModuleSideEffects || alwaysCheckedDependencies.has(dependency))) {
1034+
if (!(dependency.info.moduleSideEffects || alwaysCheckedDependencies.has(dependency))) {
10121035
continue;
10131036
}
10141037
if (dependency instanceof ExternalModule || dependency.hasEffects()) {

‎src/ModuleLoader.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ export class ModuleLoader {
444444
module.dependencies.add(dependency);
445445
dependency.importers.push(module.id);
446446
}
447-
if (!this.options.treeshake || module.info.hasModuleSideEffects === 'no-treeshake') {
447+
if (!this.options.treeshake || module.info.moduleSideEffects === 'no-treeshake') {
448448
for (const dependency of module.dependencies) {
449449
if (dependency instanceof Module) {
450450
dependency.importedFromNotTreeshaken = true;

‎src/rollup/types.d.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -156,13 +156,14 @@ export type EmitChunk = (id: string, options?: { name?: string }) => string;
156156

157157
export type EmitFile = (emittedFile: EmittedFile) => string;
158158

159-
interface ModuleInfo {
159+
interface ModuleInfo extends ModuleOptions {
160160
ast: AcornNode | null;
161161
code: string | null;
162162
dynamicImporters: readonly string[];
163163
dynamicallyImportedIdResolutions: readonly ResolvedId[];
164164
dynamicallyImportedIds: readonly string[];
165165
hasDefaultExport: boolean | null;
166+
/** @deprecated Use `moduleSideEffects` instead */
166167
hasModuleSideEffects: boolean | 'no-treeshake';
167168
id: string;
168169
implicitlyLoadedAfterOneOf: readonly string[];
@@ -173,8 +174,6 @@ interface ModuleInfo {
173174
isEntry: boolean;
174175
isExternal: boolean;
175176
isIncluded: boolean | null;
176-
meta: CustomPluginOptions;
177-
syntheticNamedExports: boolean | string;
178177
}
179178

180179
export type GetModuleInfo = (moduleId: string) => ModuleInfo | null;

‎src/utils/traverseStaticDependencies.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ export function markModuleAndImpureDependenciesAsExecuted(baseModule: Module): v
1010
if (
1111
!(dependency instanceof ExternalModule) &&
1212
!dependency.isExecuted &&
13-
(dependency.info.hasModuleSideEffects || module.implicitlyLoadedBefore.has(dependency)) &&
13+
(dependency.info.moduleSideEffects || module.implicitlyLoadedBefore.has(dependency)) &&
1414
!visitedModules.has(dependency.id)
1515
) {
1616
dependency.isExecuted = true;

0 commit comments

Comments
 (0)