Clearer empty chunk warning (#3244)

* include chunk in EMPTY_BUNDLE warning message * update batchWarnings to group multiple EMPTY_BUNDLE's together * put chunks on single line * use non-null assertion operator * add tests for cli EMPTY_BUNDLE * remove console.log
rollup · Nov 20, 2019 · 91010bf · 91010bf
1 parent edddb12
commit 91010bf
Show file tree

Hide file tree

Showing 17 changed files with 78 additions and 28 deletions.
diff --git a/cli/run/batchWarnings.ts b/cli/run/batchWarnings.ts
@@ -23,13 +23,13 @@ export default function batchWarnings() {
 				warning = { code: 'UNKNOWN', message: warning };
 			}
 
-			if ((warning.code as string) in immediateHandlers) {
-				immediateHandlers[warning.code as string](warning);
+			if (warning.code! in immediateHandlers) {
+				immediateHandlers[warning.code!](warning);
 				return;
 			}
 
-			if (!allWarnings.has(warning.code as string)) allWarnings.set(warning.code as string, []);
-			(allWarnings.get(warning.code as string) as RollupWarning[]).push(warning);
+			if (!allWarnings.has(warning.code!)) allWarnings.set(warning.code!, []);
+			(allWarnings.get(warning.code!) as RollupWarning[]).push(warning);
 
 			count += 1;
 		},
@@ -94,12 +94,12 @@ const immediateHandlers: {
 		title(`Missing shims for Node.js built-ins`);
 
 		const detail =
-			(warning.modules as string[]).length === 1
-				? `'${(warning.modules as string[])[0]}'`
-				: `${(warning.modules as string[])
-						.slice(0, -1)
+			warning.modules!.length === 1
+				? `'${warning.modules![0]}'`
+				: `${warning
+						.modules!.slice(0, -1)
 						.map((name: string) => `'${name}'`)
-						.join(', ')} and '${(warning.modules as string[]).slice(-1)}'`;
+						.join(', ')} and '${warning.modules!.slice(-1)}'`;
 		stderr(
 			`Creating a browser bundle that depends on ${detail}. You might need to include https://www.npmjs.com/package/rollup-plugin-node-builtins`
 		);
@@ -110,10 +110,6 @@ const immediateHandlers: {
 		stderr(
 			`Consumers of your bundle will have to use bundle['default'] to access the default export, which may not be what you want. Use \`output.exports: 'named'\` to disable this warning`
 		);
-	},
-
-	EMPTY_BUNDLE: () => {
-		title(`Generated an empty bundle`);
 	}
 };
 
@@ -159,9 +155,9 @@ const deferredHandlers: {
 			info('https://rollupjs.org/guide/en/#error-name-is-not-exported-by-module');
 
 			warnings.forEach(warning => {
-				stderr(tc.bold(warning.importer as string));
+				stderr(tc.bold(warning.importer!));
 				stderr(`${warning.missing} is not exported by ${warning.exporter}`);
-				stderr(tc.gray(warning.frame as string));
+				stderr(tc.gray(warning.frame!));
 			});
 		},
 		priority: 1
@@ -198,10 +194,10 @@ const deferredHandlers: {
 			title(`Conflicting re-exports`);
 			warnings.forEach(warning => {
 				stderr(
-					`${tc.bold(relativeId(warning.reexporter as string))} re-exports '${
+					`${tc.bold(relativeId(warning.reexporter!))} re-exports '${
 						warning.name
-					}' from both ${relativeId((warning.sources as string[])[0])} and ${relativeId(
-						(warning.sources as string[])[1]
+					}' from both ${relativeId(warning.sources![0])} and ${relativeId(
+						warning.sources![1]
 					)} (will be ignored)`
 				);
 			});
@@ -216,7 +212,7 @@ const deferredHandlers: {
 				`Use output.globals to specify browser global variable names corresponding to external modules`
 			);
 			warnings.forEach(warning => {
-				stderr(`${tc.bold(warning.source as string)} (guessing '${warning.guess}')`);
+				stderr(`${tc.bold(warning.source!)} (guessing '${warning.guess}')`);
 			});
 		},
 		priority: 1
@@ -255,7 +251,7 @@ const deferredHandlers: {
 				nestedByMessage.forEach(({ key: message, items }) => {
 					title(`Plugin ${plugin}: ${message}`);
 					items.forEach(warning => {
-						if (warning.url !== lastUrl) info((lastUrl = warning.url as string));
+						if (warning.url !== lastUrl) info((lastUrl = warning.url!));
 
 						if (warning.id) {
 							let loc = relativeId(warning.id);
@@ -270,6 +266,18 @@ const deferredHandlers: {
 			});
 		},
 		priority: 1
+	},
+
+	EMPTY_BUNDLE: {
+		fn: warnings => {
+			title(
+				`Generated${warnings.length === 1 ? ' an' : ''} empty ${
+					warnings.length > 1 ? 'chunks' : 'chunk'
+				}`
+			);
+			stderr(warnings.map(warning => warning.chunkName!).join(', '));
+		},
+		priority: 1
 	}
 };
 
@@ -308,7 +316,7 @@ function showTruncatedWarnings(warnings: RollupWarning[]) {
 	const sliced = nestedByModule.length > 5 ? nestedByModule.slice(0, 3) : nestedByModule;
 	sliced.forEach(({ key: id, items }) => {
 		stderr(tc.bold(relativeId(id)));
-		stderr(tc.gray(items[0].frame as string));
+		stderr(tc.gray(items[0].frame!));
 
 		if (items.length > 1) {
 			stderr(`...and ${items.length - 1} other ${items.length > 2 ? 'occurrences' : 'occurrence'}`);

diff --git a/src/Chunk.ts b/src/Chunk.ts
@@ -620,9 +620,11 @@ export default class Chunk {
 		this.renderedHash = undefined as any;
 
 		if (this.getExportNames().length === 0 && this.getImportIds().length === 0 && this.isEmpty) {
+			const chunkName = this.getChunkName();
 			this.graph.warn({
+				chunkName,
 				code: 'EMPTY_BUNDLE',
-				message: 'Generated an empty bundle'
+				message: `Generated an empty chunk: "${chunkName}"`
 			});
 		}
 

diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts
@@ -10,6 +10,7 @@ export interface RollupError extends RollupLogProps {
 }
 
 export interface RollupWarning extends RollupLogProps {
+	chunkName?: string;
 	exporter?: string;
 	exportName?: string;
 	guess?: string;

diff --git a/test/cli/samples/empty-chunk-multiple/_config.js b/test/cli/samples/empty-chunk-multiple/_config.js
@@ -0,0 +1,10 @@
+const assert = require('assert');
+
+module.exports = {
+	description: 'shows warning when multiple chunks empty',
+	command: 'rollup -c',
+	error: () => true,
+	stderr: stderr => {
+		assert.ok(stderr.includes('(!) Generated empty chunks\na, b'));
+	}
+};
diff --git a/test/cli/samples/empty-chunk-multiple/a.js b/test/cli/samples/empty-chunk-multiple/a.js
diff --git a/test/cli/samples/empty-chunk-multiple/b.js b/test/cli/samples/empty-chunk-multiple/b.js
diff --git a/test/cli/samples/empty-chunk-multiple/main.js b/test/cli/samples/empty-chunk-multiple/main.js
@@ -0,0 +1,2 @@
+import('./a.js');
+import('./b.js');
diff --git a/test/cli/samples/empty-chunk-multiple/rollup.config.js b/test/cli/samples/empty-chunk-multiple/rollup.config.js
@@ -0,0 +1,6 @@
+module.exports = {
+	input: 'main.js',
+	output: {
+		format: 'cjs'
+	}
+};
diff --git a/test/cli/samples/empty-chunk/_config.js b/test/cli/samples/empty-chunk/_config.js
@@ -0,0 +1,10 @@
+const assert = require('assert');
+
+module.exports = {
+	description: 'shows warning when chunk empty',
+	command: 'rollup -c',
+	error: () => true,
+	stderr: stderr => {
+		assert.ok(stderr.includes('(!) Generated an empty chunk\nmain'));
+	}
+};
diff --git a/test/cli/samples/empty-chunk/main.js b/test/cli/samples/empty-chunk/main.js
diff --git a/test/cli/samples/empty-chunk/rollup.config.js b/test/cli/samples/empty-chunk/rollup.config.js
@@ -0,0 +1,6 @@
+module.exports = {
+	input: 'main.js',
+	output: {
+		format: 'cjs'
+	}
+};
diff --git a/test/function/samples/assign-namespace-to-var/_config.js b/test/function/samples/assign-namespace-to-var/_config.js
@@ -2,8 +2,9 @@ module.exports = {
 	description: 'allows a namespace to be assigned to a variable',
 	warnings: [
 		{
+			chunkName: 'main',
 			code: 'EMPTY_BUNDLE',
-			message: 'Generated an empty bundle'
+			message: 'Generated an empty chunk: "main"'
 		}
 	]
 };
diff --git a/test/function/samples/can-import-self-treeshake/_config.js b/test/function/samples/can-import-self-treeshake/_config.js
@@ -7,8 +7,9 @@ module.exports = {
 			message: 'Circular dependency: lib.js -> lib.js'
 		},
 		{
+			chunkName: 'main',
 			code: 'EMPTY_BUNDLE',
-			message: `Generated an empty bundle`
+			message: `Generated an empty chunk: "main"`
 		}
 	]
 };
diff --git a/test/function/samples/module-tree/_config.js b/test/function/samples/module-tree/_config.js
@@ -34,8 +34,9 @@ module.exports = {
 	},
 	warnings: [
 		{
+			chunkName: 'main',
 			code: 'EMPTY_BUNDLE',
-			message: 'Generated an empty bundle'
+			message: 'Generated an empty chunk: "main"'
 		}
 	]
 };
diff --git a/test/function/samples/vars-with-init-in-dead-branch/_config.js b/test/function/samples/vars-with-init-in-dead-branch/_config.js
@@ -2,8 +2,9 @@ module.exports = {
 	description: 'handles vars with init in dead branch (#1198)',
 	warnings: [
 		{
+			chunkName: 'main',
 			code: 'EMPTY_BUNDLE',
-			message: 'Generated an empty bundle'
+			message: 'Generated an empty chunk: "main"'
 		}
 	]
 };
diff --git a/test/function/samples/warn-on-empty-bundle/_config.js b/test/function/samples/warn-on-empty-bundle/_config.js
@@ -2,8 +2,9 @@ module.exports = {
 	description: 'warns if empty bundle is generated  (#444)',
 	warnings: [
 		{
+			chunkName: 'main',
 			code: 'EMPTY_BUNDLE',
-			message: 'Generated an empty bundle'
+			message: 'Generated an empty chunk: "main"'
 		}
 	]
 };
diff --git a/test/function/samples/warnings-to-string/_config.js b/test/function/samples/warnings-to-string/_config.js
@@ -13,7 +13,7 @@ module.exports = {
 	warnings(warnings) {
 		assert.deepStrictEqual(warnings.map(String), [
 			'(test-plugin plugin) main.js (1:6) This might be removed',
-			'Generated an empty bundle'
+			'Generated an empty chunk: "main"'
 		]);
 	}
 };