Skip to content

Commit

Permalink
fix: tighten up rollup bundles (#450)
Browse files Browse the repository at this point in the history 
BREAKING CHANGE: Common chunks will only be created if necessary now.
  • Loading branch information
@tivac
tivac committed Jul 13, 2018
1 parent 1d83e3c commit e7170d1
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 72 deletions.
60 changes: 33 additions & 27 deletions packages/rollup/rollup.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,9 @@ module.exports = function(opts) {
Object.keys(bundles).forEach((entry) => {
const file = {
entry,
base : extensionless(entry),

css : [ ],
base : extensionless(entry),
css : [],
};

// Get CSS files being used by each entry point
Expand All @@ -153,45 +153,51 @@ module.exports = function(opts) {
start,
];

file.css = file.css.concat(used);
file.css.push(...used);

used.forEach((dep) => {
usage.set(dep, usage.has(dep) ? usage.get(dep) + 1 : 1);
usage.set(dep, (usage.get(dep) || 0) + 1);
});
});

files.push(file);
});

// Second pass removes any dependencies appearing in multiple bundles
files.forEach((file) => {
const { css } = file;
if(files.length === 1) {
// Only one entry file means we only need one bundle.
files[0].css = processor.dependencies();
} else {
// Multiple bundles means ref-counting to find the shared deps
// Second pass removes any dependencies appearing in multiple bundles
files.forEach((file) => {
const { css } = file;

file.css = css.filter((dep) => {
if(usage.get(dep) > 1) {
common.set(dep, true);
file.css = css.filter((dep) => {
if(usage.get(dep) > 1) {
common.set(dep, true);

return false;
}
return false;
}

return true;
return true;
});
});
});

// Add any other files that weren't part of a bundle to the common chunk
Object.keys(processor.files).forEach((file) => {
if(!usage.has(file)) {
common.set(file, true);
}
});

// Common chunk only emitted if necessary
if(common.size) {
files.push({
entry : options.common,
base : extensionless(options.common),
css : [ ...common.keys() ],
// Add any other files that weren't part of a bundle to the common chunk
Object.keys(processor.files).forEach((file) => {
if(!usage.has(file)) {
common.set(file, true);
}
});

// Common chunk only emitted if necessary
if(common.size) {
files.push({
entry : options.common,
base : extensionless(options.common),
css : [ ...common.keys() ],
});
}
}

await Promise.all(
Expand Down
24 changes: 9 additions & 15 deletions packages/rollup/test/__snapshots__/rollup-watch.test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,11 @@ exports[`/rollup.js watch mode should update when a dependency changes 2`] = `
`;

exports[`/rollup.js watch mode should update when a shared dependency changes 1`] = `
"/* packages/rollup/test/output/watch/shared-deps/one.css */
"/* packages/rollup/test/output/watch/shared-deps/two.css */
.mc5067d789_two {
color: green;
}
/* packages/rollup/test/output/watch/shared-deps/one.css */
.mca79168b9_one {
color: red;
}
Expand All @@ -102,28 +106,18 @@ exports[`/rollup.js watch mode should update when a shared dependency changes 1`
exports[`/rollup.js watch mode should update when a shared dependency changes 2`] = `
"/* packages/rollup/test/output/watch/shared-deps/two.css */
.mc5067d789_two {
color: green;
}"
`;

exports[`/rollup.js watch mode should update when a shared dependency changes 3`] = `
"/* packages/rollup/test/output/watch/shared-deps/one.css */
color: yellow;
}
/* packages/rollup/test/output/watch/shared-deps/one.css */
.mca79168b9_one {
color: blue;
color: red;
}
/* packages/rollup/test/output/watch/shared-deps/three.css */
.mc4ec9318b_three {
color: teal;
}"
`;

exports[`/rollup.js watch mode should update when a shared dependency changes 4`] = `
"/* packages/rollup/test/output/watch/shared-deps/two.css */
.mc5067d789_two {
color: green;
}"
`;

exports[`/rollup.js watch mode should update when adding new css files 1`] = `
"/* packages/rollup/test/output/watch/new-file/one.css */
.mc54f12712_one {
Expand Down
36 changes: 14 additions & 22 deletions packages/rollup/test/__snapshots__/rollup.test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,17 +80,13 @@ exports[`/rollup.js code splitting should support splitting up CSS files 2`] = `
`;

exports[`/rollup.js should accept an existing processor instance 1`] = `
"/* packages/rollup/test/specimens/simple.css */
.fooga {
color: red;
}
"
`;

exports[`/rollup.js should accept an existing processor instance 2`] = `
"/* packages/rollup/test/specimens/fake.css */
.fake {
color: yellow;
}
/* packages/rollup/test/specimens/simple.css */
.fooga {
color: red;
}"
`;

Expand Down Expand Up @@ -269,20 +265,6 @@ console.log(css, css$1);
`;
exports[`/rollup.js shouldn't over-remove files from an existing processor instance 2`] = `
"/* packages/rollup/test/specimens/repeated-references/a.css */
.a {
color: red;
}
/* packages/rollup/test/specimens/repeated-references/b.css */
.b {
color: blue;
}
"
`;
exports[`/rollup.js shouldn't over-remove files from an existing processor instance 3`] = `
"/* packages/rollup/test/specimens/repeated-references/d.css */
.d {
color: darkblue;
Expand All @@ -291,5 +273,15 @@ exports[`/rollup.js shouldn't over-remove files from an existing processor insta
.c {
color: cadetblue;
}
/* packages/rollup/test/specimens/repeated-references/b.css */
.b {
color: blue;
}
/* packages/rollup/test/specimens/repeated-references/a.css */
.a {
color: red;
}
"
`;
9 changes: 3 additions & 6 deletions packages/rollup/test/rollup-watch.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,24 +298,21 @@ describe("/rollup.js", () => {
});

// Create v2 of the file after a bit
setTimeout(() => write(`./watch/shared-deps/one.css`, dedent(`
.one {
composes: two from "./two.css";
color: blue;
setTimeout(() => write(`./watch/shared-deps/two.css`, dedent(`
.two {
color: yellow;
}
`)), 200);

watcher.on("event", watching((builds) => {
if(builds === 1) {
expect(read("./watch/shared-deps/assets/watch-output.css")).toMatchSnapshot();
expect(read("./watch/shared-deps/assets/common.css")).toMatchSnapshot();

// continue watching
return;
}

expect(read("./watch/shared-deps/assets/watch-output.css")).toMatchSnapshot();
expect(read("./watch/shared-deps/assets/common.css")).toMatchSnapshot();

return done();
}));
Expand Down
2 changes: 0 additions & 2 deletions packages/rollup/test/rollup.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,6 @@ describe("/rollup.js", () => {
});

expect(read("./rollup/existing-processor/assets/existing-processor.css")).toMatchSnapshot();
expect(read("./rollup/existing-processor/assets/common.css")).toMatchSnapshot();
});

it("shouldn't over-remove files from an existing processor instance", async () => {
Expand Down Expand Up @@ -376,7 +375,6 @@ describe("/rollup.js", () => {

expect(read("./rollup/repeated-references/repeated-references.js")).toMatchSnapshot();
expect(read("./rollup/repeated-references/assets/repeated-references.css")).toMatchSnapshot();
expect(read("./rollup/repeated-references/assets/common.css")).toMatchSnapshot();
});

describe("errors", () => {
Expand Down

0 comments on commit e7170d1

Please sign in to comment.