Partially revert #5658 and re-apply #5644 (#5667)

It appears that this might cause some build failures that need further investigation.
rollup · Sep 20, 2024 · d5ff63d · d5ff63d
1 parent 0a821d9
commit d5ff63d
Show file tree

Hide file tree

Showing 6 changed files with 16 additions and 16 deletions.
diff --git a/src/Graph.ts b/src/Graph.ts
@@ -172,7 +172,6 @@ export default class Graph {
 				this.needsTreeshakingPass = false;
 				for (const module of this.modules) {
 					if (module.isExecuted) {
-						module.hasTreeShakingPassStarted = true;
 						if (module.info.moduleSideEffects === 'no-treeshake') {
 							module.includeAllInBundle();
 						} else {

diff --git a/src/Module.ts b/src/Module.ts
@@ -220,7 +220,6 @@ export default class Module {
 	readonly dynamicImports: DynamicImport[] = [];
 	excludeFromSourcemap: boolean;
 	execIndex = Infinity;
-	hasTreeShakingPassStarted = false;
 	readonly implicitlyLoadedAfter = new Set<Module>();
 	readonly implicitlyLoadedBefore = new Set<Module>();
 	readonly importDescriptions = new Map<string, ImportDescription>();

diff --git a/src/ast/nodes/Identifier.ts b/src/ast/nodes/Identifier.ts
@@ -220,10 +220,9 @@ export default class Identifier extends NodeBase implements PatternNode {
 				this.variable instanceof LocalVariable &&
 				this.variable.kind &&
 				tdzVariableKinds.has(this.variable.kind) &&
-				// We ignore modules that did not receive a treeshaking pass yet as that
-				// causes many false positives due to circular dependencies or disabled
-				// moduleSideEffects.
-				this.variable.module.hasTreeShakingPassStarted
+				// we ignore possible TDZs due to circular module dependencies as
+				// otherwise we get many false positives
+				this.variable.module === this.scope.context.module
 			)
 		) {
 			return (this.isTDZAccess = false);
@@ -242,7 +241,9 @@ export default class Identifier extends NodeBase implements PatternNode {
 			return (this.isTDZAccess = true);
 		}
 
-		if (!this.variable.initReached) {
+		// We ignore the case where the module is not yet executed because
+		// moduleSideEffects are false.
+		if (!this.variable.initReached && this.scope.context.module.isExecuted) {
 			// Either a const/let TDZ violation or
 			// var use before declaration was encountered.
 			return (this.isTDZAccess = true);
@@ -293,10 +294,6 @@ export default class Identifier extends NodeBase implements PatternNode {
 	protected applyDeoptimizations(): void {
 		this.deoptimized = true;
 		if (this.variable instanceof LocalVariable) {
-			// When accessing a variable from a module without side effects, this
-			// means we use an export of that module and therefore need to potentially
-			// include it in the bundle.
-			this.variable.module.isExecuted = true;
 			this.variable.consolidateInitializers();
 			this.scope.context.requestTreeshakingPass();
 		}

diff --git a/src/utils/renderChunks.ts b/src/utils/renderChunks.ts
@@ -55,10 +55,11 @@ export async function renderChunks(
 	const getHash = hasherByType[outputOptions.hashCharacters];
 	const chunkGraph = getChunkGraph(chunks);
 	const {
+		hashDependenciesByPlaceholder,
 		initialHashesByPlaceholder,
 		nonHashedChunksWithPlaceholders,
-		renderedChunksByPlaceholder,
-		hashDependenciesByPlaceholder
+		placeholders,
+		renderedChunksByPlaceholder
 	} = await transformChunksAndGenerateContentHashes(
 		renderedChunks,
 		chunkGraph,
@@ -71,6 +72,7 @@ export async function renderChunks(
 		renderedChunksByPlaceholder,
 		hashDependenciesByPlaceholder,
 		initialHashesByPlaceholder,
+		placeholders,
 		bundle,
 		getHash
 	);
@@ -283,6 +285,7 @@ async function transformChunksAndGenerateContentHashes(
 		hashDependenciesByPlaceholder,
 		initialHashesByPlaceholder,
 		nonHashedChunksWithPlaceholders,
+		placeholders,
 		renderedChunksByPlaceholder
 	};
 }
@@ -291,11 +294,13 @@ function generateFinalHashes(
 	renderedChunksByPlaceholder: Map<string, RenderedChunkWithPlaceholders>,
 	hashDependenciesByPlaceholder: Map<string, HashResult>,
 	initialHashesByPlaceholder: Map<string, string>,
+	placeholders: Set<string>,
 	bundle: OutputBundleWithPlaceholders,
 	getHash: GetHash
 ) {
 	const hashesByPlaceholder = new Map<string, string>(initialHashesByPlaceholder);
-	for (const [placeholder, { fileName }] of renderedChunksByPlaceholder) {
+	for (const placeholder of placeholders) {
+		const { fileName } = renderedChunksByPlaceholder.get(placeholder)!;
 		let contentToHash = '';
 		const hashDependencyPlaceholders = new Set<string>([placeholder]);
 		for (const dependencyPlaceholder of hashDependencyPlaceholders) {

diff --git a/test/function/samples/tree-shake-module-side-effects-update/_config.js b/test/function/samples/tree-shake-module-side-effects-update/_config.js
@@ -1,4 +1,5 @@
 module.exports = defineTest({
+	skip: true,
 	description: 'detects variable updates in modules without side effects (#5408)',
 	options: {
 		treeshake: {

diff --git a/test/misc/misc.js b/test/misc/misc.js
@@ -114,8 +114,7 @@ describe('misc', () => {
 			});
 	});
 
-	// Skipped until potential issues are fixed, see #5659
-	it.skip('applies consistent hashes regardless of chunk transform order', async () => {
+	it('applies consistent hashes regardless of chunk transform order', async () => {
 		const FILES = {
 			main: `
             import('folder1/dupe').then(({dupe}) => console.log(dupe));