Only set asset names when finalizing (#4919)

* Only set asset names when finalizing

* Improve coverage

* Prefer shorter file names over longer file names

src/utils/FileEmitter.ts

@@ -21,6 +21,7 @@ import {
 	errorInvalidRollupPhaseForChunkEmission,
 	errorNoAssetSourceSet
 } from './error';
+import { getOrCreate } from './getOrCreate';
 import { defaultHashSize } from './hashPlaceholders';
 import type { OutputBundleWithPlaceholders } from './outputBundle';
 import { FILE_PLACEHOLDER, lowercaseBundleKeys } from './outputBundle';
@@ -74,13 +75,15 @@ interface ConsumedChunk {
 	fileName: string | undefined;
 	module: null | Module;
 	name: string;
+	referenceId: string;
 	type: 'chunk';
 }
 
 interface ConsumedAsset {
 	fileName: string | undefined;
 	name: string | undefined;
 	needsCodeReference: boolean;
+	referenceId: string;
 	source: string | Uint8Array | undefined;
 	type: 'asset';
 }
@@ -231,11 +234,11 @@ export class FileEmitter {
 		}
 		const source = getValidSource(requestedSource, consumedFile, referenceId);
 		if (this.output) {
-			this.finalizeAsset(consumedFile, source, referenceId, this.output);
+			this.finalizeAdditionalAsset(consumedFile, source, this.output);
 		} else {
 			consumedFile.source = source;
 			for (const emitter of this.outputFileEmitters) {
-				emitter.finalizeAsset(consumedFile, source, referenceId, emitter.output!);
+				emitter.finalizeAdditionalAsset(consumedFile, source, emitter.output!);
 			}
 		}
 	};
@@ -258,11 +261,20 @@ export class FileEmitter {
 				reserveFileNameInBundle(emittedFile.fileName, output, this.options.onwarn);
 			}
 		}
-		for (const [referenceId, consumedFile] of this.filesByReferenceId) {
+		const consumedAssetsByHash = new Map<string, ConsumedAsset[]>();
+		for (const consumedFile of this.filesByReferenceId.values()) {
 			if (consumedFile.type === 'asset' && consumedFile.source !== undefined) {
-				this.finalizeAsset(consumedFile, consumedFile.source, referenceId, output);
+				if (consumedFile.fileName) {
+					this.finalizeAdditionalAsset(consumedFile, consumedFile.source, output);
+				} else {
+					const sourceHash = getSourceHash(consumedFile.source);
+					getOrCreate(consumedAssetsByHash, sourceHash, () => []).push(consumedFile);
+				}
 			}
 		}
+		for (const [sourceHash, consumedFiles] of consumedAssetsByHash) {
+			this.finalizeAssetsWithSameSource(consumedFiles, sourceHash, output);
+		}
 	};
 
 	private addOutputFileEmitter(outputFileEmitter: FileEmitter) {
@@ -278,6 +290,7 @@ export class FileEmitter {
 			this.filesByReferenceId.has(referenceId) ||
 			this.outputFileEmitters.some(({ filesByReferenceId }) => filesByReferenceId.has(referenceId))
 		);
+		file.referenceId = referenceId;
 		this.filesByReferenceId.set(referenceId, file);
 		for (const { filesByReferenceId } of this.outputFileEmitters) {
 			filesByReferenceId.set(referenceId, file);
@@ -294,6 +307,7 @@ export class FileEmitter {
 			fileName: emittedAsset.fileName,
 			name: emittedAsset.name,
 			needsCodeReference: !!emittedAsset.needsCodeReference,
+			referenceId: '',
 			source,
 			type: 'asset'
 		};
@@ -302,26 +316,25 @@ export class FileEmitter {
 			emittedAsset.fileName || emittedAsset.name || String(this.nextIdBase++)
 		);
 		if (this.output) {
-			this.emitAssetWithReferenceId(consumedAsset, referenceId, this.output);
+			this.emitAssetWithReferenceId(consumedAsset, this.output);
 		} else {
 			for (const fileEmitter of this.outputFileEmitters) {
-				fileEmitter.emitAssetWithReferenceId(consumedAsset, referenceId, fileEmitter.output!);
+				fileEmitter.emitAssetWithReferenceId(consumedAsset, fileEmitter.output!);
 			}
 		}
 		return referenceId;
 	}
 
 	private emitAssetWithReferenceId(
 		consumedAsset: Readonly<ConsumedAsset>,
-		referenceId: string,
 		output: FileEmitterOutput
 	) {
 		const { fileName, source } = consumedAsset;
 		if (fileName) {
 			reserveFileNameInBundle(fileName, output, this.options.onwarn);
 		}
 		if (source !== undefined) {
-			this.finalizeAsset(consumedAsset, source, referenceId, output);
+			this.finalizeAdditionalAsset(consumedAsset, source, output);
 		}
 	}
 
@@ -340,6 +353,7 @@ export class FileEmitter {
 			fileName: emittedChunk.fileName,
 			module: null,
 			name: emittedChunk.name || emittedChunk.id,
+			referenceId: '',
 			type: 'chunk'
 		};
 		this.graph.moduleLoader
@@ -353,48 +367,86 @@ export class FileEmitter {
 		return this.assignReferenceId(consumedChunk, emittedChunk.id);
 	}
 
-	private finalizeAsset(
+	private finalizeAdditionalAsset(
 		consumedFile: Readonly<ConsumedAsset>,
 		source: string | Uint8Array,
-		referenceId: string,
 		{ bundle, fileNamesBySource, outputOptions }: FileEmitterOutput
 	): void {
-		let fileName = consumedFile.fileName;
+		let { fileName, needsCodeReference, referenceId } = consumedFile;
 
 		// Deduplicate assets if an explicit fileName is not provided
 		if (!fileName) {
 			const sourceHash = getSourceHash(source);
 			fileName = fileNamesBySource.get(sourceHash);
-			const newFileName = generateAssetFileName(
-				consumedFile.name,
+			if (!fileName) {
+				fileName = generateAssetFileName(
+					consumedFile.name,
+					source,
+					sourceHash,
+					outputOptions,
+					bundle
+				);
+				fileNamesBySource.set(sourceHash, fileName);
+			}
+		}
+
+		// We must not modify the original assets to avoid interaction between outputs
+		const assetWithFileName = { ...consumedFile, fileName, source };
+		this.filesByReferenceId.set(referenceId, assetWithFileName);
+
+		const existingAsset = bundle[fileName];
+		if (existingAsset?.type === 'asset') {
+			existingAsset.needsCodeReference &&= needsCodeReference;
+		} else {
+			bundle[fileName] = {
+				fileName,
+				name: consumedFile.name,
+				needsCodeReference,
 				source,
+				type: 'asset'
+			};
+		}
+	}
+
+	private finalizeAssetsWithSameSource(
+		consumedFiles: ReadonlyArray<ConsumedAsset>,
+		sourceHash: string,
+		{ bundle, fileNamesBySource, outputOptions }: FileEmitterOutput
+	): void {
+		let fileName = '';
+		let usedConsumedFile: ConsumedAsset;
+		let needsCodeReference = true;
+		for (const consumedFile of consumedFiles) {
+			needsCodeReference &&= consumedFile.needsCodeReference;
+			const assetFileName = generateAssetFileName(
+				consumedFile.name,
+				consumedFile.source!,
 				sourceHash,
 				outputOptions,
 				bundle
 			);
-			// make sure file name deterministic in parallel emits, always use the shorter and smaller file name
 			if (
 				!fileName ||
-				fileName.length > newFileName.length ||
-				(fileName.length === newFileName.length && fileName > newFileName)
+				assetFileName.length < fileName.length ||
+				(assetFileName.length === fileName.length && assetFileName < fileName)
 			) {
-				if (fileName) {
-					delete bundle[fileName];
-				}
-				fileName = newFileName;
-				fileNamesBySource.set(sourceHash, fileName);
+				fileName = assetFileName;
+				usedConsumedFile = consumedFile;
 			}
 		}
+		fileNamesBySource.set(sourceHash, fileName);
 
-		// We must not modify the original assets to avoid interaction between outputs
-		const assetWithFileName = { ...consumedFile, fileName, source };
-		this.filesByReferenceId.set(referenceId, assetWithFileName);
+		for (const consumedFile of consumedFiles) {
+			// We must not modify the original assets to avoid interaction between outputs
+			const assetWithFileName = { ...consumedFile, fileName };
+			this.filesByReferenceId.set(consumedFile.referenceId, assetWithFileName);
+		}
 
 		bundle[fileName] = {
 			fileName,
-			name: consumedFile.name,
-			needsCodeReference: consumedFile.needsCodeReference,
-			source,
+			name: usedConsumedFile!.name,
+			needsCodeReference,
+			source: usedConsumedFile!.source!,
 			type: 'asset'
 		};
 	}

test/chunking-form/samples/asset-needs-code-reference-same-source/_config.js

@@ -0,0 +1,98 @@
+const assert = require('node:assert');
+
+module.exports = {
+	description:
+		'emits unreferenced assets if needsCodeReference is true if they are also emitted without that flag',
+	options: {
+		output: {
+			assetFileNames: '[name][extname]'
+		},
+		plugins: [
+			{
+				name: 'test',
+				buildStart() {
+					this.emitFile({
+						type: 'asset',
+						name: 'needs-reference1.txt',
+						needsCodeReference: true,
+						source: 'source1'
+					});
+					this.emitFile({
+						type: 'asset',
+						name: 'file1.txt',
+						source: 'source1'
+					});
+					this.emitFile({
+						type: 'asset',
+						name: 'file2.txt',
+						source: 'source2'
+					});
+					this.emitFile({
+						type: 'asset',
+						name: 'needs-reference2.txt',
+						needsCodeReference: true,
+						source: 'source2'
+					});
+					this.emitFile({
+						type: 'asset',
+						name: 'needs-reference3.txt',
+						needsCodeReference: true,
+						source: 'source3'
+					});
+					this.emitFile({
+						type: 'asset',
+						name: 'file4.txt',
+						source: 'source4'
+					});
+					this.emitFile({
+						type: 'asset',
+						name: 'needs-reference5.txt',
+						needsCodeReference: true,
+						source: 'source5'
+					});
+					this.emitFile({
+						type: 'asset',
+						name: 'file6.txt',
+						source: 'source6'
+					});
+				},
+				renderStart() {
+					this.emitFile({
+						type: 'asset',
+						name: 'file3.txt',
+						source: 'source3'
+					});
+					this.emitFile({
+						type: 'asset',
+						name: 'needs-reference4.txt',
+						needsCodeReference: true,
+						source: 'source4'
+					});
+				},
+				generateBundle(_, bundle) {
+					this.emitFile({
+						type: 'asset',
+						name: 'file5.txt',
+						source: 'source5'
+					});
+					this.emitFile({
+						type: 'asset',
+						name: 'needs-reference6.txt',
+						needsCodeReference: true,
+						source: 'source6'
+					});
+
+					assert.deepEqual(Object.keys(bundle).sort(), [
+						'file1.txt',
+						'file2.txt',
+						'file4.txt',
+						'file6.txt',
+						'main.js',
+						'needs-reference3.txt',
+						'needs-reference5.txt'
+					]);
+				}
+			}
+		]
+	}
+};

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/amd/file1.txt

@@ -0,0 +1 @@
+source1

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/amd/file2.txt

@@ -0,0 +1 @@
+source2

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/amd/file4.txt

@@ -0,0 +1 @@
+source4

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/amd/file6.txt

@@ -0,0 +1 @@
+source6

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/amd/main.js

@@ -0,0 +1,5 @@
+define((function () { 'use strict';
+
+	console.log('main');
+
+}));

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/amd/needs-reference3.txt

@@ -0,0 +1 @@
+source3

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/amd/needs-reference5.txt

@@ -0,0 +1 @@
+source5

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/cjs/file1.txt

@@ -0,0 +1 @@
+source1

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/cjs/file2.txt

@@ -0,0 +1 @@
+source2

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/cjs/file4.txt

@@ -0,0 +1 @@
+source4

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/cjs/file6.txt

@@ -0,0 +1 @@
+source6

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/cjs/main.js

@@ -0,0 +1,3 @@
+'use strict';
+
+console.log('main');

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/cjs/needs-reference3.txt

@@ -0,0 +1 @@
+source3

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/cjs/needs-reference5.txt

@@ -0,0 +1 @@
+source5

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/es/file1.txt

@@ -0,0 +1 @@
+source1

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/es/file2.txt

@@ -0,0 +1 @@
+source2

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/es/file4.txt

@@ -0,0 +1 @@
+source4

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/es/file6.txt

@@ -0,0 +1 @@
+source6

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/es/main.js

@@ -0,0 +1 @@
+console.log('main');

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/es/needs-reference3.txt

@@ -0,0 +1 @@
+source3

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/es/needs-reference5.txt

@@ -0,0 +1 @@
+source5

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/system/file1.txt

@@ -0,0 +1 @@
+source1

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/system/file2.txt

@@ -0,0 +1 @@
+source2

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/system/file4.txt

@@ -0,0 +1 @@
+source4

test/chunking-form/samples/asset-needs-code-reference-same-source/_expected/system/file6.txt

@@ -0,0 +1 @@
+source6