Skip to content

Commit 59cec3e

Browse files
lukastaegertlukastaegert
authored
Ensure impure dependencies of pure modules are added (#5669)
* Ensure impure dependencies of pure modules are added * 4.22.3-0
1 parent b86ffd7 commit 59cec3e

File tree

14 files changed

+44
-16
lines changed

14 files changed

+44
-16
lines changed

‎browser/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@rollup/browser",
3-
"version": "4.22.2",
3+
"version": "4.22.3-0",
44
"description": "Next-generation ES module bundler browser build",
55
"main": "dist/rollup.browser.js",
66
"module": "dist/es/rollup.browser.js",

‎package-lock.json

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "rollup",
3-
"version": "4.22.2",
3+
"version": "4.22.3-0",
44
"description": "Next-generation ES module bundler",
55
"main": "dist/rollup.js",
66
"module": "dist/es/rollup.js",

‎src/Bundle.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import type {
1010
NormalizedOutputOptions,
1111
OutputBundle
1212
} from './rollup/types';
13-
import type { PluginDriver } from './utils/PluginDriver';
1413
import { getChunkAssignments } from './utils/chunkAssignment';
1514
import commondir from './utils/commondir';
1615
import { sortByExecutionOrder } from './utils/executionOrder';
@@ -28,6 +27,7 @@ import type { OutputBundleWithPlaceholders } from './utils/outputBundle';
2827
import { getOutputBundle, removeUnreferencedAssets } from './utils/outputBundle';
2928
import { parseAst } from './utils/parseAst';
3029
import { isAbsolute } from './utils/path';
30+
import type { PluginDriver } from './utils/PluginDriver';
3131
import { renderChunks } from './utils/renderChunks';
3232
import { timeEnd, timeStart } from './utils/timers';
3333
import {

‎src/Chunk.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
import MagicString, { Bundle as MagicStringBundle, type SourceMap } from 'magic-string';
22
import { relative } from '../browser/src/path';
3-
import ExternalChunk from './ExternalChunk';
4-
import ExternalModule from './ExternalModule';
5-
import Module from './Module';
63
import ExportDefaultDeclaration from './ast/nodes/ExportDefaultDeclaration';
74
import FunctionDeclaration from './ast/nodes/FunctionDeclaration';
85
import type ImportExpression from './ast/nodes/ImportExpression';
@@ -13,7 +10,10 @@ import LocalVariable from './ast/variables/LocalVariable';
1310
import NamespaceVariable from './ast/variables/NamespaceVariable';
1411
import SyntheticNamedExportVariable from './ast/variables/SyntheticNamedExportVariable';
1512
import type Variable from './ast/variables/Variable';
13+
import ExternalChunk from './ExternalChunk';
14+
import ExternalModule from './ExternalModule';
1615
import finalisers from './finalisers/index';
16+
import Module from './Module';
1717
import type {
1818
GetInterop,
1919
GlobalsOption,
@@ -26,7 +26,6 @@ import type {
2626
RenderedChunk,
2727
RenderedModule
2828
} from './rollup/types';
29-
import type { PluginDriver } from './utils/PluginDriver';
3029
import { createAddons } from './utils/addons';
3130
import { deconflictChunk, type DependenciesToBeDeconflicted } from './utils/deconflictChunk';
3231
import { escapeId } from './utils/escapeId';
@@ -58,6 +57,7 @@ import {
5857
import type { OutputBundleWithPlaceholders } from './utils/outputBundle';
5958
import { FILE_PLACEHOLDER } from './utils/outputBundle';
6059
import { basename, extname, isAbsolute, normalize, resolve } from './utils/path';
60+
import type { PluginDriver } from './utils/PluginDriver';
6161
import { getAliasName, getImportPath } from './utils/relativeId';
6262
import type { RenderOptions } from './utils/renderHelpers';
6363
import { makeUnique, renderNamePattern } from './utils/renderNamePattern';

‎src/Graph.ts

+1
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ export default class Graph {
172172
this.needsTreeshakingPass = false;
173173
for (const module of this.modules) {
174174
if (module.isExecuted) {
175+
module.hasTreeShakingPassStarted = true;
175176
if (module.info.moduleSideEffects === 'no-treeshake') {
176177
module.includeAllInBundle();
177178
} else {

‎src/Module.ts

+1
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ export default class Module {
220220
readonly dynamicImports: DynamicImport[] = [];
221221
excludeFromSourcemap: boolean;
222222
execIndex = Infinity;
223+
hasTreeShakingPassStarted = false;
223224
readonly implicitlyLoadedAfter = new Set<Module>();
224225
readonly implicitlyLoadedBefore = new Set<Module>();
225226
readonly importDescriptions = new Map<string, ImportDescription>();

‎src/ast/nodes/Identifier.ts

+12-6
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { BLANK } from '../../utils/blank';
55
import { logIllegalImportReassignment } from '../../utils/logs';
66
import { PureFunctionKey } from '../../utils/pureFunctions';
77
import type { NodeRenderOptions, RenderOptions } from '../../utils/renderHelpers';
8+
import { markModuleAndImpureDependenciesAsExecuted } from '../../utils/traverseStaticDependencies';
89
import type { DeoptimizableEntity } from '../DeoptimizableEntity';
910
import type { HasEffectsContext, InclusionContext } from '../ExecutionContext';
1011
import type { NodeInteraction, NodeInteractionCalled } from '../NodeInteractions';
@@ -220,9 +221,10 @@ export default class Identifier extends NodeBase implements PatternNode {
220221
this.variable instanceof LocalVariable &&
221222
this.variable.kind &&
222223
tdzVariableKinds.has(this.variable.kind) &&
223-
// we ignore possible TDZs due to circular module dependencies as
224-
// otherwise we get many false positives
225-
this.variable.module === this.scope.context.module
224+
// We ignore modules that did not receive a treeshaking pass yet as that
225+
// causes many false positives due to circular dependencies or disabled
226+
// moduleSideEffects.
227+
this.variable.module.hasTreeShakingPassStarted
226228
)
227229
) {
228230
return (this.isTDZAccess = false);
@@ -241,9 +243,7 @@ export default class Identifier extends NodeBase implements PatternNode {
241243
return (this.isTDZAccess = true);
242244
}
243245

244-
// We ignore the case where the module is not yet executed because
245-
// moduleSideEffects are false.
246-
if (!this.variable.initReached && this.scope.context.module.isExecuted) {
246+
if (!this.variable.initReached) {
247247
// Either a const/let TDZ violation or
248248
// var use before declaration was encountered.
249249
return (this.isTDZAccess = true);
@@ -294,6 +294,12 @@ export default class Identifier extends NodeBase implements PatternNode {
294294
protected applyDeoptimizations(): void {
295295
this.deoptimized = true;
296296
if (this.variable instanceof LocalVariable) {
297+
// When accessing a variable from a module without side effects, this
298+
// means we use an export of that module and therefore need to potentially
299+
// include it in the bundle.
300+
if (!this.variable.module.isExecuted) {
301+
markModuleAndImpureDependenciesAsExecuted(this.variable.module);
302+
}
297303
this.variable.consolidateInitializers();
298304
this.scope.context.requestTreeshakingPass();
299305
}

‎test/function/samples/tree-shake-module-side-effects-dependencies/_config.js

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
module.exports = defineTest({
2+
description: 'includes impure dependencies of modules without side effects',
3+
options: {
4+
treeshake: {
5+
moduleSideEffects: id => id.endsWith('effect.js')
6+
}
7+
}
8+
});

‎test/function/samples/tree-shake-module-side-effects-dependencies/dep.js

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
import './effect.js';
2+
export const value = true;

‎test/function/samples/tree-shake-module-side-effects-dependencies/effect.js

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
import { updateSharedValue } from './shared';
2+
updateSharedValue();

‎test/function/samples/tree-shake-module-side-effects-dependencies/main.js

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { value } from './dep.js';
2+
import { sharedValue } from './shared';
3+
4+
assert.ok(value);
5+
assert.strictEqual(sharedValue, 'updated');

‎test/function/samples/tree-shake-module-side-effects-dependencies/shared.js

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
export let sharedValue = 'original';
2+
export function updateSharedValue() {
3+
sharedValue = 'updated';
4+
}

‎test/function/samples/tree-shake-module-side-effects-update/_config.js

-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
module.exports = defineTest({
2-
skip: true,
32
description: 'detects variable updates in modules without side effects (#5408)',
43
options: {
54
treeshake: {

0 commit comments

Comments
 (0)