Skip to content

Commit 9f66e33

Browse files
evilebottnawievilebottnawi
authored
fix: don't duplicate import with same media in different case (#819)
1 parent 3ebdcd5 commit 9f66e33

12 files changed

+88
-51
lines changed

‎lib/loader.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ module.exports = function loader(content, map) {
5151
const alreadyImported = {};
5252
const importJs = result.importItems
5353
.filter((imp) => {
54-
if (!imp.mediaQuery) {
54+
if (!imp.media) {
5555
if (alreadyImported[imp.url]) {
5656
return false;
5757
}
@@ -63,13 +63,13 @@ module.exports = function loader(content, map) {
6363
if (!loaderUtils.isUrlRequest(imp.url)) {
6464
return `exports.push([module.id, ${JSON.stringify(
6565
`@import url(${imp.url});`
66-
)}, ${JSON.stringify(imp.mediaQuery)}]);`;
66+
)}, ${JSON.stringify(imp.media)}]);`;
6767
}
6868
const importUrl = importUrlPrefix + imp.url;
6969
return `exports.i(require(${loaderUtils.stringifyRequest(
7070
this,
7171
importUrl
72-
)}), ${JSON.stringify(imp.mediaQuery)});`;
72+
)}), ${JSON.stringify(imp.media)});`;
7373
}, this)
7474
.join('\n');
7575

‎lib/plugins/postcss-import-parser.js

+15-4
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,10 @@ function parseImport(params) {
3737

3838
return {
3939
url,
40-
media: valueParser.stringify(nodes.slice(1)).trim(),
40+
media: valueParser
41+
.stringify(nodes.slice(1))
42+
.trim()
43+
.toLowerCase(),
4144
};
4245
}
4346

@@ -71,15 +74,23 @@ module.exports = postcss.plugin(
7174
});
7275
}
7376

77+
atrule.remove();
78+
79+
const { media } = parsed;
7480
let { url } = parsed;
81+
const isUrlRequest = loaderUtils.isUrlRequest(url);
7582

76-
if (loaderUtils.isUrlRequest(url)) {
83+
if (isUrlRequest) {
7784
url = loaderUtils.urlToRequest(url);
7885
}
7986

80-
importItems.push({ url, mediaQuery: parsed.media });
87+
const includes = importItems.find(
88+
(el) => el.url === url && el.media === media
89+
);
8190

82-
atrule.remove();
91+
if (!includes) {
92+
importItems.push({ url, media, isUrlRequest });
93+
}
8394
});
8495

8596
// eslint-disable-next-line no-param-reassign

‎lib/runtime/api.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ module.exports = function(useSourceMap) {
3434
var item = modules[i];
3535
// skip already imported module
3636
// this implementation is not 100% perfect for weird media query combinations
37-
// when a module is imported multiple times with different media queries.
38-
// I hope this will never occur (Hey this way we have smaller bundles)
37+
// when a module is imported multiple times with different media queries.
38+
// I hope this will never occur (Hey this way we have smaller bundles)
3939
if (item[0] == null || !alreadyImportedModules[item[0]]) {
4040
if (mediaQuery && !item[2]) {
4141
item[2] = mediaQuery;

‎test/__snapshots__/import-option.test.js.snap

+48-32
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,16 @@ Array [
3333
@import url(test.css) screen and print;
3434
@import url(test.css) SCREEN AND PRINT;
3535
@import url(test.css)screen and print;
36+
@import url(test.css) screen and print;
3637
@import url(test-media.css) screen and print;
3738
@import url(test-other.css) (min-width: 100px);
3839
@import url(http://example.com/style.css);
40+
@import url(http://example.com/style.css);
3941
@import url(http://example.com/style.css#hash);
4042
@import url(http://example.com/style.css?#hash);
4143
@import url(http://example.com/style.css?foo=bar#hash);
4244
@import url(http://example.com/other-style.css) screen and print;
45+
@import url(http://example.com/other-style.css) screen and print;
4346
@import url(\\"//example.com/style.css\\");
4447
@import url(~package/test.css);
4548
@import ;
@@ -48,6 +51,10 @@ Array [
4851
@import url('http://') :root {}
4952
@import url('query.css?foo=1&bar=1');
5053
@import url('other-query.css?foo=1&bar=1#hash');
54+
@import url('other-query.css?foo=1&bar=1#hash') screen and print;
55+
@import url('https://fonts.googleapis.com/css?family=Roboto');
56+
@import url('https://fonts.googleapis.com/css?family=Noto+Sans+TC');
57+
@import url('https://fonts.googleapis.com/css?family=Noto+Sans+TC|Roboto');
5158
5259
.class {
5360
a: b c d;
@@ -68,7 +75,7 @@ exports[`import option false: module 1`] = `
6875
6976
7077
// module
71-
exports.push([module.id, \\"@import url(test.css);\\\\n@import url('test.css');\\\\n@import url(\\\\\\"test.css\\\\\\");\\\\n@IMPORT url(test.css);\\\\n@import URL(test.css);\\\\n@import url(test.css );\\\\n@import url( test.css);\\\\n@import url( test.css );\\\\n@import url(\\\\n test.css\\\\n);\\\\n@import url();\\\\n@import url('');\\\\n@import url(\\\\\\"\\\\\\");\\\\n@import \\\\\\"test.css\\\\\\";\\\\n@import 'test.css';\\\\n@import '';\\\\n@import \\\\\\"\\\\\\";\\\\n@import \\\\\\" \\\\\\";\\\\n@import \\\\\\"\\\\n\\\\\\";\\\\n@import url();\\\\n@import url('');\\\\n@import url(\\\\\\"\\\\\\");\\\\n@import url(test.css) screen and print;\\\\n@import url(test.css) SCREEN AND PRINT;\\\\n@import url(test.css)screen and print;\\\\n@import url(test-media.css) screen and print;\\\\n@import url(test-other.css) (min-width: 100px);\\\\n@import url(http://example.com/style.css);\\\\n@import url(http://example.com/style.css#hash);\\\\n@import url(http://example.com/style.css?#hash);\\\\n@import url(http://example.com/style.css?foo=bar#hash);\\\\n@import url(http://example.com/other-style.css) screen and print;\\\\n@import url(\\\\\\"//example.com/style.css\\\\\\");\\\\n@import url(~package/test.css);\\\\n@import ;\\\\n@import foo-bar;\\\\n@import-normalize;\\\\n@import url('http://') :root {}\\\\n@import url('query.css?foo=1&bar=1');\\\\n@import url('other-query.css?foo=1&bar=1#hash');\\\\n\\\\n.class {\\\\n a: b c d;\\\\n}\\\\n\\\\n.foo {\\\\n @import 'path.css';\\\\n}\\\\n\\", \\"\\"]);
78+
exports.push([module.id, \\"@import url(test.css);\\\\n@import url('test.css');\\\\n@import url(\\\\\\"test.css\\\\\\");\\\\n@IMPORT url(test.css);\\\\n@import URL(test.css);\\\\n@import url(test.css );\\\\n@import url( test.css);\\\\n@import url( test.css );\\\\n@import url(\\\\n test.css\\\\n);\\\\n@import url();\\\\n@import url('');\\\\n@import url(\\\\\\"\\\\\\");\\\\n@import \\\\\\"test.css\\\\\\";\\\\n@import 'test.css';\\\\n@import '';\\\\n@import \\\\\\"\\\\\\";\\\\n@import \\\\\\" \\\\\\";\\\\n@import \\\\\\"\\\\n\\\\\\";\\\\n@import url();\\\\n@import url('');\\\\n@import url(\\\\\\"\\\\\\");\\\\n@import url(test.css) screen and print;\\\\n@import url(test.css) SCREEN AND PRINT;\\\\n@import url(test.css)screen and print;\\\\n@import url(test.css) screen and print;\\\\n@import url(test-media.css) screen and print;\\\\n@import url(test-other.css) (min-width: 100px);\\\\n@import url(http://example.com/style.css);\\\\n@import url(http://example.com/style.css);\\\\n@import url(http://example.com/style.css#hash);\\\\n@import url(http://example.com/style.css?#hash);\\\\n@import url(http://example.com/style.css?foo=bar#hash);\\\\n@import url(http://example.com/other-style.css) screen and print;\\\\n@import url(http://example.com/other-style.css) screen and print;\\\\n@import url(\\\\\\"//example.com/style.css\\\\\\");\\\\n@import url(~package/test.css);\\\\n@import ;\\\\n@import foo-bar;\\\\n@import-normalize;\\\\n@import url('http://') :root {}\\\\n@import url('query.css?foo=1&bar=1');\\\\n@import url('other-query.css?foo=1&bar=1#hash');\\\\n@import url('other-query.css?foo=1&bar=1#hash') screen and print;\\\\n@import url('https://fonts.googleapis.com/css?family=Roboto');\\\\n@import url('https://fonts.googleapis.com/css?family=Noto+Sans+TC');\\\\n@import url('https://fonts.googleapis.com/css?family=Noto+Sans+TC|Roboto');\\\\n\\\\n.class {\\\\n a: b c d;\\\\n}\\\\n\\\\n.foo {\\\\n @import 'path.css';\\\\n}\\\\n\\", \\"\\"]);
7279
7380
// exports
7481
"
@@ -96,34 +103,18 @@ Array [
96103
",
97104
"screen and print",
98105
],
99-
Array [
100-
4,
101-
".test {
102-
a: a;
103-
}
104-
",
105-
"SCREEN AND PRINT",
106-
],
107106
Array [
108107
5,
109-
".test {
110-
a: a;
111-
}
112-
",
113-
"screen and print",
114-
],
115-
Array [
116-
7,
117-
".test {
118-
a: b;
108+
"a {
109+
b: b;
119110
}
120111
",
121112
"((min-width: 100px)) and (screen and print)",
122113
],
123114
Array [
124-
6,
115+
4,
125116
".test {
126-
c: d;
117+
c: c;
127118
}
128119
",
129120
"screen and print",
@@ -159,29 +150,52 @@ Array [
159150
"",
160151
],
161152
Array [
162-
8,
153+
6,
163154
".test {
164-
a: b
155+
d: d
165156
}
166157
",
167158
"",
168159
],
169160
Array [
170-
9,
161+
7,
171162
".query {
172-
color: green;
163+
e: e;
173164
}
174165
",
175166
"",
176167
],
177168
Array [
178-
10,
169+
8,
179170
".other-query {
180-
color: green;
171+
f: f;
181172
}
182173
",
183174
"",
184175
],
176+
Array [
177+
9,
178+
".other-query {
179+
f: f;
180+
}
181+
",
182+
"screen and print",
183+
],
184+
Array [
185+
1,
186+
"@import url(https://fonts.googleapis.com/css?family=Roboto);",
187+
"",
188+
],
189+
Array [
190+
1,
191+
"@import url(https://fonts.googleapis.com/css?family=Noto+Sans+TC);",
192+
"",
193+
],
194+
Array [
195+
1,
196+
"@import url(https://fonts.googleapis.com/css?family=Noto+Sans+TC|Roboto);",
197+
"",
198+
],
185199
Array [
186200
1,
187201
"@import url();
@@ -218,8 +232,6 @@ exports[`import option true: module 1`] = `
218232
// imports
219233
exports.i(require(\\"-!../../../index.js??ref--4-0!./test.css\\"), \\"\\");
220234
exports.i(require(\\"-!../../../index.js??ref--4-0!./test.css\\"), \\"screen and print\\");
221-
exports.i(require(\\"-!../../../index.js??ref--4-0!./test.css\\"), \\"SCREEN AND PRINT\\");
222-
exports.i(require(\\"-!../../../index.js??ref--4-0!./test.css\\"), \\"screen and print\\");
223235
exports.i(require(\\"-!../../../index.js??ref--4-0!./test-media.css\\"), \\"screen and print\\");
224236
exports.i(require(\\"-!../../../index.js??ref--4-0!./test-other.css\\"), \\"(min-width: 100px)\\");
225237
exports.push([module.id, \\"@import url(http://example.com/style.css);\\", \\"\\"]);
@@ -231,6 +243,10 @@ exports.push([module.id, \\"@import url(//example.com/style.css);\\", \\"\\"]);
231243
exports.i(require(\\"-!../../../index.js??ref--4-0!package/test.css\\"), \\"\\");
232244
exports.i(require(\\"-!../../../index.js??ref--4-0!./query.css?foo=1&bar=1\\"), \\"\\");
233245
exports.i(require(\\"-!../../../index.js??ref--4-0!./other-query.css?foo=1&bar=1#hash\\"), \\"\\");
246+
exports.i(require(\\"-!../../../index.js??ref--4-0!./other-query.css?foo=1&bar=1#hash\\"), \\"screen and print\\");
247+
exports.push([module.id, \\"@import url(https://fonts.googleapis.com/css?family=Roboto);\\", \\"\\"]);
248+
exports.push([module.id, \\"@import url(https://fonts.googleapis.com/css?family=Noto+Sans+TC);\\", \\"\\"]);
249+
exports.push([module.id, \\"@import url(https://fonts.googleapis.com/css?family=Noto+Sans+TC|Roboto);\\", \\"\\"]);
234250
235251
// module
236252
exports.push([module.id, \\"@import url();\\\\n@import url('');\\\\n@import url(\\\\\\"\\\\\\");\\\\n@import '';\\\\n@import \\\\\\"\\\\\\";\\\\n@import \\\\\\" \\\\\\";\\\\n@import \\\\\\"\\\\n\\\\\\";\\\\n@import url();\\\\n@import url('');\\\\n@import url(\\\\\\"\\\\\\");\\\\n@import ;\\\\n@import foo-bar;\\\\n@import-normalize;\\\\n@import url('http://') :root {}\\\\n\\\\n.class {\\\\n a: b c d;\\\\n}\\\\n\\\\n.foo {\\\\n @import 'path.css';\\\\n}\\\\n\\", \\"\\"]);
@@ -285,14 +301,14 @@ Warning
285301
"ModuleWarning: Module Warning (from \`replaced original path\`):
286302
Warning
287303
288-
(37:1) Unable to find uri in '@import '",
304+
(40:1) Unable to find uri in '@import '",
289305
"ModuleWarning: Module Warning (from \`replaced original path\`):
290306
Warning
291307
292-
(38:1) Unable to find uri in '@import foo-bar'",
308+
(41:1) Unable to find uri in '@import foo-bar'",
293309
"ModuleWarning: Module Warning (from \`replaced original path\`):
294310
Warning
295311
296-
(40:1) It looks like you didn't end your @import statement correctly. Child nodes are attached to it.",
312+
(43:1) It looks like you didn't end your @import statement correctly. Child nodes are attached to it.",
297313
]
298314
`;

‎test/__snapshots__/loader.test.js.snap

+4-4
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ module.exports = function(useSourceMap) {
3737
var item = modules[i];
3838
// skip already imported module
3939
// this implementation is not 100% perfect for weird media query combinations
40-
// when a module is imported multiple times with different media queries.
41-
// I hope this will never occur (Hey this way we have smaller bundles)
40+
// when a module is imported multiple times with different media queries.
41+
// I hope this will never occur (Hey this way we have smaller bundles)
4242
if (item[0] == null || !alreadyImportedModules[item[0]]) {
4343
if (mediaQuery && !item[2]) {
4444
item[2] = mediaQuery;
@@ -219,8 +219,8 @@ module.exports = function(useSourceMap) {
219219
var item = modules[i];
220220
// skip already imported module
221221
// this implementation is not 100% perfect for weird media query combinations
222-
// when a module is imported multiple times with different media queries.
223-
// I hope this will never occur (Hey this way we have smaller bundles)
222+
// when a module is imported multiple times with different media queries.
223+
// I hope this will never occur (Hey this way we have smaller bundles)
224224
if (item[0] == null || !alreadyImportedModules[item[0]]) {
225225
if (mediaQuery && !item[2]) {
226226
item[2] = mediaQuery;

‎test/fixtures/import/import.css

+7
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,16 @@
2525
@import url(test.css) screen and print;
2626
@import url(test.css) SCREEN AND PRINT;
2727
@import url(test.css)screen and print;
28+
@import url(test.css) screen and print;
2829
@import url(test-media.css) screen and print;
2930
@import url(test-other.css) (min-width: 100px);
3031
@import url(http://example.com/style.css);
32+
@import url(http://example.com/style.css);
3133
@import url(http://example.com/style.css#hash);
3234
@import url(http://example.com/style.css?#hash);
3335
@import url(http://example.com/style.css?foo=bar#hash);
3436
@import url(http://example.com/other-style.css) screen and print;
37+
@import url(http://example.com/other-style.css) screen and print;
3538
@import url("//example.com/style.css");
3639
@import url(~package/test.css);
3740
@import ;
@@ -40,6 +43,10 @@
4043
@import url('http://') :root {}
4144
@import url('query.css?foo=1&bar=1');
4245
@import url('other-query.css?foo=1&bar=1#hash');
46+
@import url('other-query.css?foo=1&bar=1#hash') screen and print;
47+
@import url('https://fonts.googleapis.com/css?family=Roboto');
48+
@import url('https://fonts.googleapis.com/css?family=Noto+Sans+TC');
49+
@import url('https://fonts.googleapis.com/css?family=Noto+Sans+TC|Roboto');
4350

4451
.class {
4552
a: b c d;

‎test/fixtures/import/node_modules/package/test.css

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/fixtures/import/other-query.css

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
.other-query {
2-
color: green;
2+
f: f;
33
}

‎test/fixtures/import/query.css

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
.query {
2-
color: green;
2+
e: e;
33
}

‎test/fixtures/import/test-media.css

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
@import url('test-other.css') (min-width: 100px);
1+
@import url('test-nested-media.css') (min-width: 100px);
22

33
.test {
4-
c: d;
4+
c: c;
55
}

‎test/fixtures/import/test-nested-media.css

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
a {
2+
b: b;
3+
}

‎test/fixtures/import/test-other.css

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
.test {
2-
a: b;
2+
d: d;
33
}

0 commit comments

Comments
 (0)