Always create facades for explicit file names

rollup · Aug 1, 2019 · 689ba18 · 689ba18
1 parent 5078443
commit 689ba18
Show file tree

Hide file tree

Showing 14 changed files with 136 additions and 58 deletions.
diff --git a/src/Chunk.ts b/src/Chunk.ts
@@ -115,17 +115,19 @@ export function isChunkRendered(chunk: Chunk): boolean {
 }
 
 export default class Chunk {
-	static generateFacade(graph: Graph, facadedModule: Module, { name }: FacadeName): Chunk {
+	private static generateFacade(
+		graph: Graph,
+		facadedModule: Module,
+		facadeName: FacadeName
+	): Chunk {
 		const chunk = new Chunk(graph, []);
-		chunk.dependencies = [facadedModule.chunk as Chunk];
-		if (name) {
-			chunk.chunkName = name;
-		}
-		chunk.dynamicDependencies = [];
-		chunk.facadeModule = facadedModule;
+		chunk.assignFacadeName(facadeName, facadedModule);
 		if (!facadedModule.facadeChunk) {
 			facadedModule.facadeChunk = chunk;
 		}
+		chunk.dependencies = [facadedModule.chunk as Chunk];
+		chunk.dynamicDependencies = [];
+		chunk.facadeModule = facadedModule;
 		for (const exportName of facadedModule.getAllExportNames()) {
 			const tracedVariable = facadedModule.getVariableForExportName(exportName);
 			chunk.exports.add(tracedVariable);
@@ -150,6 +152,8 @@ export default class Chunk {
 	usedModules: Module[] = undefined as any;
 
 	variableName: string;
+	// TODO Lukas remove chunk
+	private chunkFileName: string | null = null;
 	private chunkName: string | null = null;
 	private dependencies: (ExternalModule | Chunk)[] = undefined as any;
 	private dynamicDependencies: (ExternalModule | Chunk)[] = undefined as any;
@@ -193,10 +197,7 @@ export default class Chunk {
 		if (entryModule) {
 			this.variableName = makeLegal(
 				basename(
-					(entryModule.chunkFileName && getAliasName(entryModule.chunkFileName)) ||
-						entryModule.chunkName ||
-						entryModule.manualChunkAlias ||
-						getAliasName(entryModule.id)
+					entryModule.chunkName || entryModule.manualChunkAlias || getAliasName(entryModule.id)
 				)
 			);
 		} else {
@@ -220,6 +221,10 @@ export default class Chunk {
 			const requiredFacades: FacadeName[] = Array.from(module.userChunkNames).map(name => ({
 				name
 			}));
+			if (requiredFacades.length === 0 && module.isUserDefinedEntryPoint) {
+				requiredFacades.push({});
+			}
+			requiredFacades.push(...Array.from(module.chunkFileNames).map(fileName => ({ fileName })));
 			if (requiredFacades.length === 0) {
 				requiredFacades.push({});
 			}
@@ -233,13 +238,7 @@ export default class Chunk {
 							this.exportNames[exportName] = variable;
 						}
 					}
-					const facadeName = requiredFacades.shift() as { name?: string };
-					// TODO Lukas completely handle the case of a facade module here
-					if (facadeName.name) {
-						this.chunkName = facadeName.name;
-					} else if (module.chunkName) {
-						this.chunkName = module.chunkName;
-					}
+					this.assignFacadeName(requiredFacades.shift() as FacadeName, module);
 				}
 			}
 
@@ -257,6 +256,9 @@ export default class Chunk {
 		options: OutputOptions,
 		existingNames: Record<string, any>
 	): string {
+		if (this.chunkFileName !== null) {
+			return this.chunkFileName;
+		}
 		return makeUnique(
 			renderNamePattern(pattern, patternName, type => {
 				switch (type) {
@@ -325,7 +327,7 @@ export default class Chunk {
 	}
 
 	getChunkName(): string {
-		return this.chunkName || (this.chunkName = this.computeChunkName());
+		return this.chunkName || (this.chunkName = sanitizeFileName(this.getFallbackChunkName()));
 	}
 
 	getDynamicImportIds(): string[] {
@@ -803,21 +805,15 @@ export default class Chunk {
 		}
 	}
 
-	private computeChunkName(): string {
-		if (this.facadeModule !== null) {
-			return sanitizeFileName(
-				(this.facadeModule.chunkFileName && getAliasName(this.facadeModule.chunkFileName)) ||
-					this.facadeModule.chunkName ||
-					getAliasName(this.facadeModule.id)
+	private assignFacadeName({ fileName, name }: FacadeName, facadedModule: Module) {
+		if (fileName) {
+			// TODO Lukas all assignments to chunkFileName should be sanitized
+			this.chunkFileName = fileName;
+		} else {
+			this.chunkName = sanitizeFileName(
+				name || facadedModule.chunkName || getAliasName(facadedModule.id)
 			);
 		}
-		if (this.manualChunkAlias) {
-			return sanitizeFileName(this.manualChunkAlias);
-		}
-		for (const module of this.orderedModules) {
-			if (module.chunkName) return sanitizeFileName(module.chunkName);
-		}
-		return 'chunk';
 	}
 
 	private computeContentHashWithDependencies(addons: Addons, options: OutputOptions): string {
@@ -1010,6 +1006,19 @@ export default class Chunk {
 		return exports;
 	}
 
+	private getFallbackChunkName(): string {
+		if (this.manualChunkAlias) {
+			return this.manualChunkAlias;
+		}
+		if (this.chunkFileName) {
+			return getAliasName(this.chunkFileName);
+		}
+		for (const module of this.orderedModules) {
+			if (module.chunkName) return module.chunkName;
+		}
+		return 'chunk';
+	}
+
 	private inlineChunkDependencies(chunk: Chunk, deep: boolean) {
 		for (const dep of chunk.dependencies) {
 			if (dep instanceof ExternalModule) {

diff --git a/src/Graph.ts b/src/Graph.ts
@@ -313,6 +313,7 @@ export default class Graph {
 			const facades: Chunk[] = [];
 			for (const chunk of chunks) {
 				for (const facade of chunk.generateFacades()) {
+					// TODO Lukas use spread operator
 					facades.push(facade);
 				}
 			}

diff --git a/src/Module.ts b/src/Module.ts
@@ -169,7 +169,7 @@ const MISSING_EXPORT_SHIM_DESCRIPTION: ExportDescription = {
 
 export default class Module {
 	chunk?: Chunk;
-	chunkFileName: string | null = null;
+	chunkFileNames = new Set<string>();
 	chunkName: string | null = null;
 	code!: string;
 	comments: CommentDescription[] = [];

diff --git a/src/ModuleLoader.ts b/src/ModuleLoader.ts
@@ -132,13 +132,7 @@ export class ModuleLoader {
 			unresolvedEntryModules.map(({ fileName, id, name }) =>
 				this.loadEntryModule(id, true).then(module => {
 					if (fileName !== null) {
-						if (module.chunkFileName !== null) {
-							// TODO Lukas test or create facades and make chunkFileName a Set
-							throw new Error('Cannot reassign chunk filename');
-						}
-						module.chunkFileName = fileName;
-						// TODO Lukas can we make sure a precise filename does not override a user-defined one?
-						// should we create facades if the user provides several names for a chunk?
+						module.chunkFileNames.add(fileName);
 					} else if (name !== null) {
 						if (module.chunkName === null) {
 							module.chunkName = name;

diff --git a/src/utils/assignChunkIds.ts b/src/utils/assignChunkIds.ts
@@ -12,29 +12,25 @@ export function assignChunkIds(
 	addons: Addons,
 	bundle: OutputBundleWithPlaceholders
 ) {
-	const [entryChunks, otherChunks] = chunks.reduce<[Chunk[], Chunk[]]>(
-		([entryChunks, otherChunks], chunk) => {
-			(chunk.facadeModule && chunk.facadeModule.isUserDefinedEntryPoint
-				? entryChunks
-				: otherChunks
-			).push(chunk);
-			return [entryChunks, otherChunks];
-		},
-		[[], []]
-	);
+	// TODO Lukas test conflict between prenamed chunk and entry chunk
+	// TODO Lukas do not filter out prenamed chunks but use new property
+	const entryChunks: Chunk[] = [];
+	const otherChunks: Chunk[] = [];
+	for (const chunk of chunks) {
+		(chunk.facadeModule && chunk.facadeModule.isUserDefinedEntryPoint
+			? entryChunks
+			: otherChunks
+		).push(chunk);
+	}
 
 	// make sure entry chunk names take precedence with regard to deconflicting
 	const chunksForNaming: Chunk[] = entryChunks.concat(otherChunks);
-	for (let i = 0; i < chunksForNaming.length; i++) {
-		const chunk = chunksForNaming[i];
+	for (const chunk of chunksForNaming) {
 		const facadeModule = chunk.facadeModule;
-
 		if (outputOptions.file) {
 			chunk.id = basename(outputOptions.file);
 		} else if (inputOptions.preserveModules) {
 			chunk.id = chunk.generateIdPreserveModules(inputBase, bundle);
-		} else if (facadeModule && facadeModule.chunkFileName) {
-			chunk.id = facadeModule.chunkFileName;
 		} else {
 			let pattern, patternName;
 			if (facadeModule && facadeModule.isUserDefinedEntryPoint) {

diff --git a/test/chunking-form/samples/emit-file/named-user-chunks/_config.js b/test/chunking-form/samples/emit-file/named-user-chunks/_config.js
@@ -1,17 +1,31 @@
 const assert = require('assert');
 
-let referenceId;
+let referenceIdName;
+let referenceIdFileName1;
+let referenceIdFileName2;
 
 module.exports = {
 	description: 'deduplicates with named chunks defined by the user',
 	options: {
 		input: { mainChunk: 'main', mainChunkFacade: 'main' },
 		plugins: {
 			buildStart() {
-				referenceId = this.emitFile({ type: 'chunk', id: 'main', name: 'ignored' });
+				referenceIdName = this.emitFile({ type: 'chunk', id: 'main', name: 'ignored' });
+				referenceIdFileName1 = this.emitFile({
+					type: 'chunk',
+					id: 'main',
+					fileName: 'explicit-name1.js'
+				});
+				referenceIdFileName2 = this.emitFile({
+					type: 'chunk',
+					id: 'main',
+					fileName: 'explicit-name2.js'
+				});
 			},
 			generateBundle() {
-				assert.strictEqual(this.getFileName(referenceId), 'mainChunk.js');
+				assert.strictEqual(this.getFileName(referenceIdName), 'mainChunk.js');
+				assert.strictEqual(this.getFileName(referenceIdFileName1), 'explicit-name1.js');
+				assert.strictEqual(this.getFileName(referenceIdFileName2), 'explicit-name2.js');
 			}
 		}
 	}

diff --git a/test/chunking-form/samples/emit-file/named-user-chunks/_expected/amd/explicit-name1.js b/test/chunking-form/samples/emit-file/named-user-chunks/_expected/amd/explicit-name1.js
@@ -0,0 +1,9 @@
+define(['exports', './mainChunk'], function (exports, mainChunk) { 'use strict';
+
+
+
+	exports.value = mainChunk.value;
+
+	Object.defineProperty(exports, '__esModule', { value: true });
+
+});
diff --git a/test/chunking-form/samples/emit-file/named-user-chunks/_expected/amd/explicit-name2.js b/test/chunking-form/samples/emit-file/named-user-chunks/_expected/amd/explicit-name2.js
@@ -0,0 +1,9 @@
+define(['exports', './mainChunk'], function (exports, mainChunk) { 'use strict';
+
+
+
+	exports.value = mainChunk.value;
+
+	Object.defineProperty(exports, '__esModule', { value: true });
+
+});
diff --git a/test/chunking-form/samples/emit-file/named-user-chunks/_expected/cjs/explicit-name1.js b/test/chunking-form/samples/emit-file/named-user-chunks/_expected/cjs/explicit-name1.js
@@ -0,0 +1,9 @@
+'use strict';
+
+Object.defineProperty(exports, '__esModule', { value: true });
+
+var mainChunk = require('./mainChunk.js');
+
+
+
+exports.value = mainChunk.value;
diff --git a/test/chunking-form/samples/emit-file/named-user-chunks/_expected/cjs/explicit-name2.js b/test/chunking-form/samples/emit-file/named-user-chunks/_expected/cjs/explicit-name2.js
@@ -0,0 +1,9 @@
+'use strict';
+
+Object.defineProperty(exports, '__esModule', { value: true });
+
+var mainChunk = require('./mainChunk.js');
+
+
+
+exports.value = mainChunk.value;
diff --git a/test/chunking-form/samples/emit-file/named-user-chunks/_expected/es/explicit-name1.js b/test/chunking-form/samples/emit-file/named-user-chunks/_expected/es/explicit-name1.js
@@ -0,0 +1 @@
+export { value } from './mainChunk.js';
diff --git a/test/chunking-form/samples/emit-file/named-user-chunks/_expected/es/explicit-name2.js b/test/chunking-form/samples/emit-file/named-user-chunks/_expected/es/explicit-name2.js
@@ -0,0 +1 @@
+export { value } from './mainChunk.js';
diff --git a/test/chunking-form/samples/emit-file/named-user-chunks/_expected/system/explicit-name1.js b/test/chunking-form/samples/emit-file/named-user-chunks/_expected/system/explicit-name1.js
@@ -0,0 +1,13 @@
+System.register(['./mainChunk.js'], function (exports) {
+	'use strict';
+	return {
+		setters: [function (module) {
+			exports('value', module.value);
+		}],
+		execute: function () {
+
+
+
+		}
+	};
+});
diff --git a/test/chunking-form/samples/emit-file/named-user-chunks/_expected/system/explicit-name2.js b/test/chunking-form/samples/emit-file/named-user-chunks/_expected/system/explicit-name2.js
@@ -0,0 +1,13 @@
+System.register(['./mainChunk.js'], function (exports) {
+	'use strict';
+	return {
+		setters: [function (module) {
+			exports('value', module.value);
+		}],
+		execute: function () {
+
+
+
+		}
+	};
+});