Skip to content

Commit 58f65c4

Browse files
authored
fix: only include direct deps in compositions (#857)
Because of how the css-in-js package works the deps already contain all of *their* deps, so by concatenating all of the deps of the selector you actually end up with duplicated classes in the output and it looks a real mess.
1 parent 8dc538b commit 58f65c4

File tree

9 files changed

+79
-16
lines changed

9 files changed

+79
-16
lines changed

packages/css-to-js/css-to-js.js

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,10 @@ exports.transform = (file, processor, opts = {}) => {
6868

6969
// Only want direct dependencies and any first-level dependencies
7070
// of this file to be processed
71-
graph.outgoingEdges[Processor.fileKey(id)].forEach((dep) => {
71+
graph.directDependenciesOf(Processor.fileKey(id)).forEach((dep) => {
7272
dependencies.add(dep);
7373

74-
graph.outgoingEdges[dep].forEach((d) => {
74+
graph.directDependenciesOf(dep).forEach((d) => {
7575
dependencies.add(d);
7676
});
7777
});
@@ -215,15 +215,14 @@ exports.transform = (file, processor, opts = {}) => {
215215

216216
// Build the list of composed classes for this class
217217
if(graph.hasNode(sKey)) {
218-
graph.dependenciesOf(sKey).forEach((dep) => {
218+
graph.directDependenciesOf(sKey).forEach((dep) => {
219219
const { file: src, selector } = graph.getNodeData(dep);
220220

221221
// Get the value from the right place
222-
if(src !== id) {
223-
elements.push(externalsMap.get(dep));
224-
} else {
225-
elements.push(internalsMap.get(selector));
226-
}
222+
elements.push(src !== id ?
223+
externalsMap.get(dep) :
224+
internalsMap.get(selector)
225+
);
227226
});
228227
}
229228

packages/rollup/test/__snapshots__/rollup.test.js.snap

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -268,21 +268,51 @@ Object {
268268
}
269269
`;
270270
271+
exports[`/rollup.js should express layers of locally-composed classes correctly 1`] = `
272+
Object {
273+
"assets/composition-layers.css": "
274+
/* packages/rollup/test/specimens/composition-layers/composition-layers.css */
275+
.mc_a {
276+
color: red;
277+
}
278+
279+
.mc_b {
280+
color: blue;
281+
}
282+
283+
.mc_c {
284+
color: green;
285+
}",
286+
"composition-layers.js": "
287+
const a = \\"mc_a\\";
288+
const b = a + \\" \\" + \\"mc_b\\";
289+
const c = b + \\" \\" + \\"mc_c\\";
290+
var css = {
291+
a,
292+
b,
293+
c
294+
};
295+
296+
console.log(css);
297+
",
298+
}
299+
`;
300+
271301
exports[`/rollup.js should express local & remote-composed classes correctly 1`] = `
272302
Object {
273-
"assets/multiple-composition.css": "
274-
/* packages/rollup/test/specimens/multiple-composition/other.css */
303+
"assets/internal-external-composition.css": "
304+
/* packages/rollup/test/specimens/internal-external-composition/other.css */
275305
.mc_other {
276306
color: green;
277307
}
278-
/* packages/rollup/test/specimens/multiple-composition/multiple-composition.css */
308+
/* packages/rollup/test/specimens/internal-external-composition/internal-external-composition.css */
279309
.mc_one {
280310
color: red;
281311
}
282312
.mc_two {
283313
background: blue;
284314
}",
285-
"multiple-composition.js": "
315+
"internal-external-composition.js": "
286316
const other = \\"mc_other\\";
287317
288318
const one = \\"mc_one\\";

packages/rollup/test/rollup.test.js

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,25 @@ describe("/rollup.js", () => {
7272
).toMatchRollupSnapshot();
7373
});
7474

75+
it("should express layers of locally-composed classes correctly", async () => {
76+
const bundle = await rollup({
77+
input : require.resolve(`./specimens/composition-layers/composition-layers.js`),
78+
plugins : [
79+
createPlugin(),
80+
],
81+
});
82+
83+
expect(
84+
await bundle.generate({
85+
format,
86+
assetFileNames,
87+
})
88+
).toMatchRollupSnapshot();
89+
});
90+
7591
it("should express local & remote-composed classes correctly", async () => {
7692
const bundle = await rollup({
77-
input : require.resolve(`./specimens/multiple-composition/multiple-composition.js`),
93+
input : require.resolve(`./specimens/internal-external-composition/internal-external-composition.js`),
7894
plugins : [
7995
createPlugin(),
8096
],
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
.a {
2+
color: red;
3+
}
4+
5+
.b {
6+
composes: a;
7+
8+
color: blue;
9+
}
10+
11+
.c {
12+
composes: b;
13+
14+
color: green;
15+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import css from "./composition-layers.css";
2+
3+
console.log(css);

packages/rollup/test/specimens/multiple-composition/multiple-composition.css renamed to packages/rollup/test/specimens/internal-external-composition/internal-external-composition.css

File renamed without changes.
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import css from "./internal-external-composition.css";
2+
3+
console.log(css);

packages/rollup/test/specimens/multiple-composition/other.css renamed to packages/rollup/test/specimens/internal-external-composition/other.css

File renamed without changes.

packages/rollup/test/specimens/multiple-composition/multiple-composition.js

Lines changed: 0 additions & 3 deletions
This file was deleted.

0 commit comments

Comments
 (0)