Skip to content

Commit e913cb1

Browse files
alexander-akaitalexander-akait
authored
fix: do not crash on 'false' aliases (#1292)
1 parent 2388439 commit e913cb1

19 files changed

+312
-1182
lines changed

‎package-lock.json

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

‎package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@
8989
"stylus": "^0.54.8",
9090
"stylus-loader": "^4.3.0",
9191
"url-loader": "^4.1.1",
92-
"webpack": "^5.33.2"
92+
"webpack": "^5.34.0"
9393
},
9494
"keywords": [
9595
"webpack",

‎src/plugins/postcss-icss-parser.js

+17-4
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,11 @@ const plugin = (options = {}) => {
4040
...new Set([normalizedUrl, request]),
4141
]);
4242

43+
if (!resolvedUrl) {
44+
return;
45+
}
46+
47+
// eslint-disable-next-line consistent-return
4348
return { url: resolvedUrl, prefix, tokens };
4449
};
4550

@@ -49,8 +54,14 @@ const plugin = (options = {}) => {
4954
const results = await Promise.all(tasks);
5055

5156
for (let index = 0; index <= results.length - 1; index++) {
52-
const { url, prefix, tokens } = results[index];
53-
const newUrl = prefix ? `${prefix}!${url}` : url;
57+
const item = results[index];
58+
59+
if (!item) {
60+
// eslint-disable-next-line no-continue
61+
continue;
62+
}
63+
64+
const newUrl = item.prefix ? `${item.prefix}!${item.url}` : item.url;
5465
const importKey = newUrl;
5566
let importName = imports.get(importKey);
5667

@@ -68,9 +79,11 @@ const plugin = (options = {}) => {
6879
options.api.push({ importName, dedupe: true, index });
6980
}
7081

71-
for (const [replacementIndex, token] of Object.keys(tokens).entries()) {
82+
for (const [replacementIndex, token] of Object.keys(
83+
item.tokens
84+
).entries()) {
7285
const replacementName = `___CSS_LOADER_ICSS_IMPORT_${index}_REPLACEMENT_${replacementIndex}___`;
73-
const localName = tokens[token];
86+
const localName = item.tokens[token];
7487

7588
importReplacements[token] = replacementName;
7689

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

+11-3
Original file line numberDiff line numberDiff line change
@@ -166,12 +166,10 @@ const plugin = (options = {}) => {
166166
const needKeep = await options.filter(url, media);
167167

168168
if (!needKeep) {
169-
return null;
169+
return;
170170
}
171171
}
172172

173-
atRule.remove();
174-
175173
if (isRequestable) {
176174
const request = requestify(url, options.rootContext);
177175

@@ -180,9 +178,19 @@ const plugin = (options = {}) => {
180178
...new Set([request, url]),
181179
]);
182180

181+
if (!resolvedUrl) {
182+
return;
183+
}
184+
185+
atRule.remove();
186+
187+
// eslint-disable-next-line consistent-return
183188
return { url: resolvedUrl, media, prefix, isRequestable };
184189
}
185190

191+
atRule.remove();
192+
193+
// eslint-disable-next-line consistent-return
186194
return { url, media, prefix, isRequestable };
187195
})
188196
);

‎src/plugins/postcss-url-parser.js

+12-5
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ const plugin = (options = {}) => {
292292
const needKeep = await options.filter(url);
293293

294294
if (!needKeep) {
295-
return null;
295+
return;
296296
}
297297
}
298298

@@ -309,19 +309,26 @@ const plugin = (options = {}) => {
309309
...new Set([request, url]),
310310
]);
311311

312+
if (!resolvedUrl) {
313+
return;
314+
}
315+
316+
// eslint-disable-next-line consistent-return
312317
return { ...parsedDeclaration, url: resolvedUrl, hash };
313318
})
314319
);
315320

316-
const results = await Promise.all(resolvedDeclarations);
317-
318321
const urlToNameMap = new Map();
319322
const urlToReplacementMap = new Map();
320323

321324
let hasUrlImportHelper = false;
322325

323-
for (let index = 0; index <= results.length - 1; index++) {
324-
const item = results[index];
326+
for (
327+
let index = 0;
328+
index <= resolvedDeclarations.length - 1;
329+
index++
330+
) {
331+
const item = resolvedDeclarations[index];
325332

326333
if (!item) {
327334
// eslint-disable-next-line no-continue

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

+4
Original file line numberDiff line numberDiff line change
@@ -1865,6 +1865,10 @@ Warning
18651865
]
18661866
`;
18671867

1868+
exports[`"import" option should work with 'false' aliases: errors 1`] = `Array []`;
1869+
1870+
exports[`"import" option should work with 'false' aliases: warnings 1`] = `Array []`;
1871+
18681872
exports[`"import" option should work with a value equal to "false": errors 1`] = `Array []`;
18691873

18701874
exports[`"import" option should work with a value equal to "false": module 1`] = `

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

+4
Original file line numberDiff line numberDiff line change
@@ -4239,6 +4239,10 @@ Object {
42394239

42404240
exports[`"modules" option should work with "exportOnlyLocals" and "namedExport" option: warnings 1`] = `Array []`;
42414241

4242+
exports[`"modules" option should work with "false" alises: errors 1`] = `Array []`;
4243+
4244+
exports[`"modules" option should work with "false" alises: warnings 1`] = `Array []`;
4245+
42424246
exports[`"modules" option should work with "url" and "namedExport": errors 1`] = `Array []`;
42434247

42444248
exports[`"modules" option should work with "url" and "namedExport": module 1`] = `

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

+4
Original file line numberDiff line numberDiff line change
@@ -655,6 +655,10 @@ Warning
655655
]
656656
`;
657657

658+
exports[`"url" option should work with 'false' aliases: errors 1`] = `Array []`;
659+
660+
exports[`"url" option should work with 'false' aliases: warnings 1`] = `Array []`;
661+
658662
exports[`"url" option should work with a value equal to "Function": errors 1`] = `Array []`;
659663

660664
exports[`"url" option should work with a value equal to "Function": module 1`] = `

‎test/fixtures/import/false-alias.css

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
@import "/style.css";
2+
3+
.class {
4+
color: red;
5+
}

‎test/fixtures/import/false-alias.js

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import css from './false-alias.css';
2+
3+
__export__ = css;
4+
5+
export default css;

‎test/fixtures/modules/icss-false-alias/icss.js

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import relative from './relative.icss.css';
2+
3+
__export__ = relative;
4+
5+
export default relative;

‎test/fixtures/modules/icss-false-alias/relative.icss.css

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
:import("./unknown.css") {
2+
IMPORTED_NAME: primary-color;
3+
}
4+
5+
.className {
6+
color: IMPORTED_NAME;
7+
}
8+
9+
:export {
10+
primary-color: IMPORTED_NAME
11+
}

‎test/fixtures/modules/icss-false-alias/unknown.css

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
:export {
2+
primary-color: red;
3+
}

‎test/fixtures/url/false-alias.css

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
.class {
2+
background-image: url(/logo.png);
3+
}

‎test/fixtures/url/false-alias.js

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import css from './false-alias.css';
2+
3+
__export__ = css;
4+
5+
export default css;

‎test/fixtures/url/logo.png

76.3 KB
Loading

‎test/import-option.test.js

+39
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import fs from "fs";
22
import path from "path";
33

4+
import webpack from "webpack";
5+
46
import {
57
compile,
68
getCompiler,
@@ -10,6 +12,8 @@ import {
1012
getWarnings,
1113
} from "./helpers/index";
1214

15+
const isWebpack5 = webpack.version.startsWith(5);
16+
1317
describe('"import" option', () => {
1418
it("should work when not specified", async () => {
1519
const compiler = getCompiler("./import/import.js");
@@ -196,6 +200,41 @@ describe('"import" option', () => {
196200
expect(getErrors(stats)).toMatchSnapshot("errors");
197201
});
198202

203+
it("should work with 'false' aliases", async () => {
204+
const compiler = getCompiler(
205+
"./import/false-alias.js",
206+
{},
207+
{
208+
module: {
209+
rules: [
210+
{
211+
test: /\.css$/i,
212+
loader: path.resolve(__dirname, "../src"),
213+
},
214+
],
215+
},
216+
resolve: {
217+
alias: {
218+
"/style.css": isWebpack5
219+
? false
220+
: path.resolve(__dirname, "./fixtures/import/alias.css"),
221+
},
222+
},
223+
}
224+
);
225+
const stats = await compile(compiler);
226+
227+
// TODO uncomment after drop webpack v4
228+
// expect(getModuleSource("./import/false-alias.css", stats)).toMatchSnapshot(
229+
// "module"
230+
// );
231+
// expect(getExecutedCode("main.bundle.js", compiler, stats)).toMatchSnapshot(
232+
// "result"
233+
// );
234+
expect(getWarnings(stats)).toMatchSnapshot("warnings");
235+
expect(getErrors(stats)).toMatchSnapshot("errors");
236+
});
237+
199238
it("should throw an error on unresolved import", async () => {
200239
const compiler = getCompiler("./import/unresolved.js");
201240
const stats = await compile(compiler);

‎test/modules-option.test.js

+34
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import path from "path";
22
import fs from "fs";
33

4+
import webpack from "webpack";
5+
46
import {
57
compile,
68
getCompiler,
@@ -15,6 +17,8 @@ const testCases = fs.readdirSync(testCasesPath);
1517

1618
jest.setTimeout(60000);
1719

20+
const isWebpack5 = webpack.version.startsWith(5);
21+
1822
describe('"modules" option', () => {
1923
[
2024
true,
@@ -843,6 +847,36 @@ describe('"modules" option', () => {
843847
expect(getErrors(stats)).toMatchSnapshot("errors");
844848
});
845849

850+
it('should work with "false" alises', async () => {
851+
const compiler = getCompiler(
852+
"./modules/icss-false-alias/icss.js",
853+
{},
854+
{
855+
resolve: {
856+
alias: {
857+
"./unknown.css": isWebpack5
858+
? false
859+
: path.resolve(
860+
__dirname,
861+
"./fixtures/modules/icss-false-alias/unknown.css"
862+
),
863+
},
864+
},
865+
}
866+
);
867+
const stats = await compile(compiler);
868+
869+
// TODO uncomment after drop webpack v4
870+
// expect(
871+
// getModuleSource("./modules/icss-false-alias/relative.icss.css", stats)
872+
// ).toMatchSnapshot("module");
873+
// expect(getExecutedCode("main.bundle.js", compiler, stats)).toMatchSnapshot(
874+
// "result"
875+
// );
876+
expect(getWarnings(stats)).toMatchSnapshot("warnings");
877+
expect(getErrors(stats)).toMatchSnapshot("errors");
878+
});
879+
846880
it('should work with the "auto" when it is "false"', async () => {
847881
const compiler = getCompiler("./modules/mode/modules.js", {
848882
modules: {

‎test/url-option.test.js

+47
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,53 @@ describe('"url" option', () => {
279279
expect(getErrors(stats)).toMatchSnapshot("errors");
280280
});
281281

282+
it("should work with 'false' aliases", async () => {
283+
const compiler = getCompiler(
284+
"./url/false-alias.js",
285+
{},
286+
{
287+
module: {
288+
rules: [
289+
{
290+
test: /\.css$/i,
291+
loader: path.resolve(__dirname, "../src"),
292+
},
293+
isWebpack5
294+
? {
295+
test: /\.(png|jpg|gif|svg|eot|ttf|woff|woff2)$/i,
296+
type: "asset/resource",
297+
}
298+
: {
299+
test: /\.(png|jpg|gif|svg|eot|ttf|woff|woff2)$/i,
300+
loader: "file-loader",
301+
options: {
302+
name: "[name].[ext]",
303+
},
304+
},
305+
],
306+
},
307+
resolve: {
308+
alias: {
309+
"/logo.png": isWebpack5
310+
? false
311+
: path.resolve(__dirname, "./fixtures/url/logo.png"),
312+
},
313+
},
314+
}
315+
);
316+
const stats = await compile(compiler);
317+
318+
// TODO uncomment after drop webpack v4
319+
// expect(getModuleSource("./url/false-alias.css", stats)).toMatchSnapshot(
320+
// "module"
321+
// );
322+
// expect(getExecutedCode("main.bundle.js", compiler, stats)).toMatchSnapshot(
323+
// "result"
324+
// );
325+
expect(getWarnings(stats)).toMatchSnapshot("warnings");
326+
expect(getErrors(stats)).toMatchSnapshot("errors");
327+
});
328+
282329
it("should throw an error on unresolved import", async () => {
283330
const compiler = getCompiler("./url/url-unresolved.js");
284331
const stats = await compile(compiler);

0 commit comments

Comments
 (0)