Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clearer empty chunk warning #3244

Merged
merged 7 commits into from Nov 20, 2019
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
50 changes: 29 additions & 21 deletions cli/run/batchWarnings.ts
Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


count += 1;
},
Expand Down Expand Up @@ -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`
);
Expand All @@ -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`);
}
};

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)`
);
});
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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
tjenkinson marked this conversation as resolved.
Show resolved Hide resolved
}
};

Expand Down Expand Up @@ -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'}`);
Expand Down
4 changes: 3 additions & 1 deletion src/Chunk.ts
Expand Up @@ -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}"`
});
}

Expand Down
1 change: 1 addition & 0 deletions src/rollup/types.d.ts
Expand Up @@ -10,6 +10,7 @@ export interface RollupError extends RollupLogProps {
}

export interface RollupWarning extends RollupLogProps {
chunkName?: string;
exporter?: string;
exportName?: string;
guess?: string;
Expand Down
10 changes: 10 additions & 0 deletions 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'));
}
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Empty file.
Empty file.
2 changes: 2 additions & 0 deletions test/cli/samples/empty-chunk-multiple/main.js
@@ -0,0 +1,2 @@
import('./a.js');
import('./b.js');
6 changes: 6 additions & 0 deletions test/cli/samples/empty-chunk-multiple/rollup.config.js
@@ -0,0 +1,6 @@
module.exports = {
input: 'main.js',
output: {
format: 'cjs'
}
};
10 changes: 10 additions & 0 deletions 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'));
}
};
Empty file.
6 changes: 6 additions & 0 deletions test/cli/samples/empty-chunk/rollup.config.js
@@ -0,0 +1,6 @@
module.exports = {
input: 'main.js',
output: {
format: 'cjs'
}
};
3 changes: 2 additions & 1 deletion test/function/samples/assign-namespace-to-var/_config.js
Expand Up @@ -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"'
}
]
};
3 changes: 2 additions & 1 deletion test/function/samples/can-import-self-treeshake/_config.js
Expand Up @@ -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"`
}
]
};
3 changes: 2 additions & 1 deletion test/function/samples/module-tree/_config.js
Expand Up @@ -34,8 +34,9 @@ module.exports = {
},
warnings: [
{
chunkName: 'main',
code: 'EMPTY_BUNDLE',
message: 'Generated an empty bundle'
message: 'Generated an empty chunk: "main"'
}
]
};
Expand Up @@ -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"'
}
]
};
3 changes: 2 additions & 1 deletion test/function/samples/warn-on-empty-bundle/_config.js
Expand Up @@ -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"'
}
]
};
2 changes: 1 addition & 1 deletion test/function/samples/warnings-to-string/_config.js
Expand Up @@ -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"'
]);
}
};