Skip to content

Commit ca499c6

Browse files
tivactivac
authored
fix(rollup-rewriter): include static dependencies (#577)
* test: add failing test for rewriter bug It's not including dependencies of any static imports for the dynamic modules being found, which means that depending on your flow you can end up with incomplete styling. * fix: include static imports of dynamic deps So every CSS file is included before the dynamic chunk is loaded
1 parent ec746a9 commit ca499c6

File tree

11 files changed

+432
-3
lines changed

11 files changed

+432
-3
lines changed

‎packages/rollup-rewriter/rewriter.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,13 @@ module.exports = (opts) => {
5757
}
5858

5959
graph.addNode(entry);
60-
60+
6161
imported.forEach((file) => {
6262
graph.addNode(file);
6363
graph.addDependency(entry, file);
6464
});
6565
});
66-
66+
6767
entries.forEach((deps, entry) => {
6868
const { code } = chunks[entry];
6969

@@ -88,7 +88,11 @@ module.exports = (opts) => {
8888
const { index } = result;
8989

9090
// eslint-disable-next-line no-loop-func
91-
const css = [ ...graph.dependenciesOf(file), file ].reduce((out, curr) => {
91+
const css = [
92+
...graph.dependenciesOf(file),
93+
...(file in chunks ? chunks[file].imports : []),
94+
file,
95+
].reduce((out, curr) => {
9296
const { assets = [] } = chunks[curr];
9397

9498
assets.forEach((asset) => out.add(asset));

‎packages/rollup-rewriter/test/__snapshots__/rewriter.test.js.snap

Lines changed: 361 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,367 @@
22

33
exports[`rollup-rewriter should error on unsupported formats ("cjs") 1`] = `"Unsupported format: cjs. Supported formats are [\\"amd\\",\\"es\\",\\"esm\\",\\"system\\"]"`;
44

5+
exports[`rollup-rewriter should include css for static imports used by a dynamic import ("amd") 1`] = `
6+
Object {
7+
"assets/dynamic1.css": "
8+
/* packages/rollup-rewriter/test/specimens/dynamic-shared-imports/dynamic1.css */
9+
.dynamic1 {
10+
color: dynamic1;
11+
}
12+
",
13+
"assets/entry1.css": "
14+
/* packages/rollup-rewriter/test/specimens/dynamic-shared-imports/entry1.css */
15+
.entry1 {
16+
color: entry1;
17+
}
18+
",
19+
"assets/entry2.css": "
20+
/* packages/rollup-rewriter/test/specimens/dynamic-shared-imports/entry2.css */
21+
.entry2 {
22+
color: entry2;
23+
}
24+
",
25+
"assets/static1.css": "
26+
/* packages/rollup-rewriter/test/specimens/dynamic-shared-imports/static1.css */
27+
.static1 {
28+
color: static1;
29+
}
30+
",
31+
"chunk.js": "
32+
define(['exports'], function (exports) { 'use strict';
33+
34+
var css = {
35+
\\"static1\\": \\"static1\\"
36+
};
37+
38+
console.log(css);
39+
40+
var static1 = \\"static1.js\\";
41+
42+
exports.static1 = static1;
43+
44+
});
45+
",
46+
"chunk2.js": "
47+
define(['exports', './chunk.js'], function (exports, __chunk_1) { 'use strict';
48+
49+
var css = {
50+
\\"dynamic1\\": \\"dynamic1\\"
51+
};
52+
53+
console.log(__chunk_1.static1, css);
54+
55+
var dynamic1 = \\"dynamic1.js\\";
56+
57+
exports.default = dynamic1;
58+
59+
});
60+
",
61+
"entry1.js": "
62+
define(['./chunk.js'], function (__chunk_1) { 'use strict';
63+
64+
var css = {
65+
\\"entry1\\": \\"entry1\\"
66+
};
67+
68+
console.log(__chunk_1.static1, css);
69+
70+
});
71+
",
72+
"entry2.js": "
73+
define(['require'], function (require) { 'use strict';
74+
75+
var css = {
76+
\\"entry2\\": \\"entry2\\"
77+
};
78+
79+
console.log(css);
80+
81+
(function() {
82+
new Promise(function (resolve, reject) { Promise.all([
83+
lazyload(\\"./assets/static1.css\\"),
84+
lazyload(\\"./assets/dynamic1.css\\"),
85+
new Promise(function (resolve, reject) { require(['./chunk2.js'], resolve, reject) })
86+
])
87+
.then((results) => resolve(results[results.length - 1]))
88+
.catch(reject) }).then(console.log);
89+
}());
90+
91+
});
92+
",
93+
}
94+
`;
95+
96+
exports[`rollup-rewriter should include css for static imports used by a dynamic import ("es") 1`] = `
97+
Object {
98+
"assets/dynamic1.css": "
99+
/* packages/rollup-rewriter/test/specimens/dynamic-shared-imports/dynamic1.css */
100+
.dynamic1 {
101+
color: dynamic1;
102+
}
103+
",
104+
"assets/entry1.css": "
105+
/* packages/rollup-rewriter/test/specimens/dynamic-shared-imports/entry1.css */
106+
.entry1 {
107+
color: entry1;
108+
}
109+
",
110+
"assets/entry2.css": "
111+
/* packages/rollup-rewriter/test/specimens/dynamic-shared-imports/entry2.css */
112+
.entry2 {
113+
color: entry2;
114+
}
115+
",
116+
"assets/static1.css": "
117+
/* packages/rollup-rewriter/test/specimens/dynamic-shared-imports/static1.css */
118+
.static1 {
119+
color: static1;
120+
}
121+
",
122+
"chunk.js": "
123+
var css = {
124+
\\"static1\\": \\"static1\\"
125+
};
126+
127+
console.log(css);
128+
129+
var static1 = \\"static1.js\\";
130+
131+
export { static1 as a };
132+
",
133+
"chunk2.js": "
134+
import { a as static1 } from './chunk.js';
135+
136+
var css = {
137+
\\"dynamic1\\": \\"dynamic1\\"
138+
};
139+
140+
console.log(static1, css);
141+
142+
var dynamic1 = \\"dynamic1.js\\";
143+
144+
export default dynamic1;
145+
",
146+
"entry1.js": "
147+
import { a as static1 } from './chunk.js';
148+
149+
var css = {
150+
\\"entry1\\": \\"entry1\\"
151+
};
152+
153+
console.log(static1, css);
154+
",
155+
"entry2.js": "
156+
var css = {
157+
\\"entry2\\": \\"entry2\\"
158+
};
159+
160+
console.log(css);
161+
162+
(function() {
163+
Promise.all([
164+
lazyload(\\"./assets/static1.css\\"),
165+
lazyload(\\"./assets/dynamic1.css\\"),
166+
import('./chunk2.js')
167+
])
168+
.then((results) => results[results.length - 1]).then(console.log);
169+
}());
170+
",
171+
}
172+
`;
173+
174+
exports[`rollup-rewriter should include css for static imports used by a dynamic import ("esm") 1`] = `
175+
Object {
176+
"assets/dynamic1.css": "
177+
/* packages/rollup-rewriter/test/specimens/dynamic-shared-imports/dynamic1.css */
178+
.dynamic1 {
179+
color: dynamic1;
180+
}
181+
",
182+
"assets/entry1.css": "
183+
/* packages/rollup-rewriter/test/specimens/dynamic-shared-imports/entry1.css */
184+
.entry1 {
185+
color: entry1;
186+
}
187+
",
188+
"assets/entry2.css": "
189+
/* packages/rollup-rewriter/test/specimens/dynamic-shared-imports/entry2.css */
190+
.entry2 {
191+
color: entry2;
192+
}
193+
",
194+
"assets/static1.css": "
195+
/* packages/rollup-rewriter/test/specimens/dynamic-shared-imports/static1.css */
196+
.static1 {
197+
color: static1;
198+
}
199+
",
200+
"chunk.js": "
201+
var css = {
202+
\\"static1\\": \\"static1\\"
203+
};
204+
205+
console.log(css);
206+
207+
var static1 = \\"static1.js\\";
208+
209+
export { static1 as a };
210+
",
211+
"chunk2.js": "
212+
import { a as static1 } from './chunk.js';
213+
214+
var css = {
215+
\\"dynamic1\\": \\"dynamic1\\"
216+
};
217+
218+
console.log(static1, css);
219+
220+
var dynamic1 = \\"dynamic1.js\\";
221+
222+
export default dynamic1;
223+
",
224+
"entry1.js": "
225+
import { a as static1 } from './chunk.js';
226+
227+
var css = {
228+
\\"entry1\\": \\"entry1\\"
229+
};
230+
231+
console.log(static1, css);
232+
",
233+
"entry2.js": "
234+
var css = {
235+
\\"entry2\\": \\"entry2\\"
236+
};
237+
238+
console.log(css);
239+
240+
(function() {
241+
Promise.all([
242+
lazyload(\\"./assets/static1.css\\"),
243+
lazyload(\\"./assets/dynamic1.css\\"),
244+
import('./chunk2.js')
245+
])
246+
.then((results) => results[results.length - 1]).then(console.log);
247+
}());
248+
",
249+
}
250+
`;
251+
252+
exports[`rollup-rewriter should include css for static imports used by a dynamic import ("system") 1`] = `
253+
Object {
254+
"assets/dynamic1.css": "
255+
/* packages/rollup-rewriter/test/specimens/dynamic-shared-imports/dynamic1.css */
256+
.dynamic1 {
257+
color: dynamic1;
258+
}
259+
",
260+
"assets/entry1.css": "
261+
/* packages/rollup-rewriter/test/specimens/dynamic-shared-imports/entry1.css */
262+
.entry1 {
263+
color: entry1;
264+
}
265+
",
266+
"assets/entry2.css": "
267+
/* packages/rollup-rewriter/test/specimens/dynamic-shared-imports/entry2.css */
268+
.entry2 {
269+
color: entry2;
270+
}
271+
",
272+
"assets/static1.css": "
273+
/* packages/rollup-rewriter/test/specimens/dynamic-shared-imports/static1.css */
274+
.static1 {
275+
color: static1;
276+
}
277+
",
278+
"chunk.js": "
279+
System.register([], function (exports, module) {
280+
'use strict';
281+
return {
282+
execute: function () {
283+
284+
var css = {
285+
\\"static1\\": \\"static1\\"
286+
};
287+
288+
console.log(css);
289+
290+
var static1 = exports('a', \\"static1.js\\");
291+
292+
}
293+
};
294+
});
295+
",
296+
"chunk2.js": "
297+
System.register(['./chunk.js'], function (exports, module) {
298+
'use strict';
299+
var static1;
300+
return {
301+
setters: [function (module) {
302+
static1 = module.a;
303+
}],
304+
execute: function () {
305+
306+
var css = {
307+
\\"dynamic1\\": \\"dynamic1\\"
308+
};
309+
310+
console.log(static1, css);
311+
312+
var dynamic1 = exports('default', \\"dynamic1.js\\");
313+
314+
}
315+
};
316+
});
317+
",
318+
"entry1.js": "
319+
System.register(['./chunk.js'], function (exports, module) {
320+
'use strict';
321+
var static1;
322+
return {
323+
setters: [function (module) {
324+
static1 = module.a;
325+
}],
326+
execute: function () {
327+
328+
var css = {
329+
\\"entry1\\": \\"entry1\\"
330+
};
331+
332+
console.log(static1, css);
333+
334+
}
335+
};
336+
});
337+
",
338+
"entry2.js": "
339+
System.register([], function (exports, module) {
340+
'use strict';
341+
return {
342+
execute: function () {
343+
344+
var css = {
345+
\\"entry2\\": \\"entry2\\"
346+
};
347+
348+
console.log(css);
349+
350+
(function() {
351+
Promise.all([
352+
lazyload(\\"./assets/static1.css\\"),
353+
lazyload(\\"./assets/dynamic1.css\\"),
354+
module.import('./chunk2.js')
355+
])
356+
.then((results) => results[results.length - 1]).then(console.log);
357+
}());
358+
359+
}
360+
};
361+
});
362+
",
363+
}
364+
`;
365+
5366
exports[`rollup-rewriter should log details in verbose mode 1`] = `
6367
Array [
7368
Array [

0 commit comments

Comments
 (0)