Skip to content

Commit

Permalink
fix: handle dynamic imports removed by tree-shaking (#538)
Browse files Browse the repository at this point in the history
Work around rollup/rollup#2659
  • Loading branch information
tivac authored Jan 16, 2019
1 parent 7557b82 commit c2a09e1
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 6 deletions.
15 changes: 10 additions & 5 deletions packages/rollup/rollup.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,12 @@ module.exports = (opts) => {

// Really wish rollup would provide this default...
assetFileNames = outputOptions.assetFileNames || "assets/[name]-[hash][extname]";

const {
chunkFileNames = "[name]-[hash].js",
entryFileNames = "[name].js",
} = outputOptions;

// Determine the correct to option for PostCSS by doing a bit of a dance
let to;

Expand All @@ -178,6 +178,11 @@ module.exports = (opts) => {
usage.addNode(entry, true);

[ ...imports, ...dynamicImports ].forEach((dep) => {
// Need to filter out invalid deps, see rollup/rollup#2659
if(!dep) {
return;
}

usage.addNode(dep, true);
usage.addDependency(entry, dep);
});
Expand Down Expand Up @@ -215,7 +220,7 @@ module.exports = (opts) => {
// based on the module's template (either entry or chunk)
let dest;
const template = isEntry ? entryFileNames : chunkFileNames;

if(template.includes("[hash]")) {
const parts = parse(template, fileName);

Expand Down Expand Up @@ -326,11 +331,11 @@ module.exports = (opts) => {
path.dirname(to),
path.basename(target)
);

log("map output", target);

fs.writeFileSync(dest, content.toString(), "utf8");

if(!assetFileNames.includes("hash")) {
return;
}
Expand Down
2 changes: 2 additions & 0 deletions packages/rollup/test/__snapshots__/splitting.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,8 @@ Array [
]
`;

exports[`/rollup.js code splitting shouldn't break when dynamic imports are tree-shaken away (rollup/rollup#2659) 1`] = `Array []`;

exports[`/rollup.js code splitting shouldn't put bundle-specific CSS in common.css 1`] = `
Array [
Object {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default false ? import('./b.js') : null;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default "hi";
29 changes: 28 additions & 1 deletion packages/rollup/test/splitting.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,33 @@ describe("/rollup.js", () => {
expect(dir("./dynamic-imports/assets/")).toMatchSnapshot();
});

it("shouldn't break when dynamic imports are tree-shaken away (rollup/rollup#2659)", async () => {
const bundle = await rollup({
input : [
require.resolve("./specimens/stripped-dynamic-imports/a.js"),
],

plugins : [
plugin({
namer,
map,
}),
],
});

await bundle.write({
format,
sourcemap,

assetFileNames,
chunkFileNames,

dir : prefix(`./output/stripped-dynamic-imports`),
});

expect(dir("./stripped-dynamic-imports/assets/")).toMatchSnapshot();
});

it("should ouput only 1 JSON file", async () => {
const bundle = await rollup({
input : [
Expand Down Expand Up @@ -260,7 +287,7 @@ describe("/rollup.js", () => {

expect(dir("./multiple-chunks/assets")).toMatchSnapshot();
});

it("should dedupe chunk names using rollup's incrementing counter logic (hashed)", async () => {
const bundle = await rollup({
input : [
Expand Down

0 comments on commit c2a09e1

Please sign in to comment.