fix: do not crash on 'false' aliases (#1292)

webpack-contrib · Apr 19, 2021 · e913cb1 · e913cb1
1 parent 2388439
commit e913cb1
Show file tree

Hide file tree

Showing 19 changed files with 312 additions and 1,182 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -89,7 +89,7 @@
     "stylus": "^0.54.8",
     "stylus-loader": "^4.3.0",
     "url-loader": "^4.1.1",
-    "webpack": "^5.33.2"
+    "webpack": "^5.34.0"
   },
   "keywords": [
     "webpack",

diff --git a/src/plugins/postcss-icss-parser.js b/src/plugins/postcss-icss-parser.js
@@ -40,6 +40,11 @@ const plugin = (options = {}) => {
             ...new Set([normalizedUrl, request]),
           ]);
 
+          if (!resolvedUrl) {
+            return;
+          }
+
+          // eslint-disable-next-line consistent-return
           return { url: resolvedUrl, prefix, tokens };
         };
 
@@ -49,8 +54,14 @@ const plugin = (options = {}) => {
       const results = await Promise.all(tasks);
 
       for (let index = 0; index <= results.length - 1; index++) {
-        const { url, prefix, tokens } = results[index];
-        const newUrl = prefix ? `${prefix}!${url}` : url;
+        const item = results[index];
+
+        if (!item) {
+          // eslint-disable-next-line no-continue
+          continue;
+        }
+
+        const newUrl = item.prefix ? `${item.prefix}!${item.url}` : item.url;
         const importKey = newUrl;
         let importName = imports.get(importKey);
 
@@ -68,9 +79,11 @@ const plugin = (options = {}) => {
           options.api.push({ importName, dedupe: true, index });
         }
 
-        for (const [replacementIndex, token] of Object.keys(tokens).entries()) {
+        for (const [replacementIndex, token] of Object.keys(
+          item.tokens
+        ).entries()) {
           const replacementName = `___CSS_LOADER_ICSS_IMPORT_${index}_REPLACEMENT_${replacementIndex}___`;
-          const localName = tokens[token];
+          const localName = item.tokens[token];
 
           importReplacements[token] = replacementName;
 

diff --git a/src/plugins/postcss-import-parser.js b/src/plugins/postcss-import-parser.js
@@ -166,12 +166,10 @@ const plugin = (options = {}) => {
                 const needKeep = await options.filter(url, media);
 
                 if (!needKeep) {
-                  return null;
+                  return;
                 }
               }
 
-              atRule.remove();
-
               if (isRequestable) {
                 const request = requestify(url, options.rootContext);
 
@@ -180,9 +178,19 @@ const plugin = (options = {}) => {
                   ...new Set([request, url]),
                 ]);
 
+                if (!resolvedUrl) {
+                  return;
+                }
+
+                atRule.remove();
+
+                // eslint-disable-next-line consistent-return
                 return { url: resolvedUrl, media, prefix, isRequestable };
               }
 
+              atRule.remove();
+
+              // eslint-disable-next-line consistent-return
               return { url, media, prefix, isRequestable };
             })
           );

diff --git a/src/plugins/postcss-url-parser.js b/src/plugins/postcss-url-parser.js
@@ -292,7 +292,7 @@ const plugin = (options = {}) => {
                 const needKeep = await options.filter(url);
 
                 if (!needKeep) {
-                  return null;
+                  return;
                 }
               }
 
@@ -309,19 +309,26 @@ const plugin = (options = {}) => {
                 ...new Set([request, url]),
               ]);
 
+              if (!resolvedUrl) {
+                return;
+              }
+
+              // eslint-disable-next-line consistent-return
               return { ...parsedDeclaration, url: resolvedUrl, hash };
             })
           );
 
-          const results = await Promise.all(resolvedDeclarations);
-
           const urlToNameMap = new Map();
           const urlToReplacementMap = new Map();
 
           let hasUrlImportHelper = false;
 
-          for (let index = 0; index <= results.length - 1; index++) {
-            const item = results[index];
+          for (
+            let index = 0;
+            index <= resolvedDeclarations.length - 1;
+            index++
+          ) {
+            const item = resolvedDeclarations[index];
 
             if (!item) {
               // eslint-disable-next-line no-continue

diff --git a/test/__snapshots__/import-option.test.js.snap b/test/__snapshots__/import-option.test.js.snap
@@ -1865,6 +1865,10 @@ Warning
 ]
 `;
 
+exports[`"import" option should work with 'false' aliases: errors 1`] = `Array []`;
+
+exports[`"import" option should work with 'false' aliases: warnings 1`] = `Array []`;
+
 exports[`"import" option should work with a value equal to "false": errors 1`] = `Array []`;
 
 exports[`"import" option should work with a value equal to "false": module 1`] = `

diff --git a/test/__snapshots__/modules-option.test.js.snap b/test/__snapshots__/modules-option.test.js.snap
@@ -4239,6 +4239,10 @@ Object {
 
 exports[`"modules" option should work with "exportOnlyLocals" and "namedExport" option: warnings 1`] = `Array []`;
 
+exports[`"modules" option should work with "false" alises: errors 1`] = `Array []`;
+
+exports[`"modules" option should work with "false" alises: warnings 1`] = `Array []`;
+
 exports[`"modules" option should work with "url" and "namedExport": errors 1`] = `Array []`;
 
 exports[`"modules" option should work with "url" and "namedExport": module 1`] = `

diff --git a/test/__snapshots__/url-option.test.js.snap b/test/__snapshots__/url-option.test.js.snap
@@ -655,6 +655,10 @@ Warning
 ]
 `;
 
+exports[`"url" option should work with 'false' aliases: errors 1`] = `Array []`;
+
+exports[`"url" option should work with 'false' aliases: warnings 1`] = `Array []`;
+
 exports[`"url" option should work with a value equal to "Function": errors 1`] = `Array []`;
 
 exports[`"url" option should work with a value equal to "Function": module 1`] = `

diff --git a/test/fixtures/import/false-alias.css b/test/fixtures/import/false-alias.css
@@ -0,0 +1,5 @@
+@import "/style.css";
+
+.class {
+    color: red;
+}
diff --git a/test/fixtures/import/false-alias.js b/test/fixtures/import/false-alias.js
@@ -0,0 +1,5 @@
+import css from './false-alias.css';
+
+__export__ = css;
+
+export default css;
diff --git a/test/fixtures/modules/icss-false-alias/icss.js b/test/fixtures/modules/icss-false-alias/icss.js
@@ -0,0 +1,5 @@
+import relative from './relative.icss.css';
+
+__export__ = relative;
+
+export default relative;
diff --git a/test/fixtures/modules/icss-false-alias/relative.icss.css b/test/fixtures/modules/icss-false-alias/relative.icss.css
@@ -0,0 +1,11 @@
+:import("./unknown.css") {
+  IMPORTED_NAME: primary-color;
+}
+
+.className {
+  color: IMPORTED_NAME;
+}
+
+:export {
+  primary-color: IMPORTED_NAME
+}
diff --git a/test/fixtures/modules/icss-false-alias/unknown.css b/test/fixtures/modules/icss-false-alias/unknown.css
@@ -0,0 +1,3 @@
+:export {
+    primary-color: red;
+}
diff --git a/test/fixtures/url/false-alias.css b/test/fixtures/url/false-alias.css
@@ -0,0 +1,3 @@
+.class {
+    background-image: url(/logo.png);
+}
diff --git a/test/fixtures/url/false-alias.js b/test/fixtures/url/false-alias.js
@@ -0,0 +1,5 @@
+import css from './false-alias.css';
+
+__export__ = css;
+
+export default css;
diff --git a/test/fixtures/url/logo.png b/test/fixtures/url/logo.png
diff --git a/test/import-option.test.js b/test/import-option.test.js
@@ -1,6 +1,8 @@
 import fs from "fs";
 import path from "path";
 
+import webpack from "webpack";
+
 import {
   compile,
   getCompiler,
@@ -10,6 +12,8 @@ import {
   getWarnings,
 } from "./helpers/index";
 
+const isWebpack5 = webpack.version.startsWith(5);
+
 describe('"import" option', () => {
   it("should work when not specified", async () => {
     const compiler = getCompiler("./import/import.js");
@@ -196,6 +200,41 @@ describe('"import" option', () => {
     expect(getErrors(stats)).toMatchSnapshot("errors");
   });
 
+  it("should work with 'false' aliases", async () => {
+    const compiler = getCompiler(
+      "./import/false-alias.js",
+      {},
+      {
+        module: {
+          rules: [
+            {
+              test: /\.css$/i,
+              loader: path.resolve(__dirname, "../src"),
+            },
+          ],
+        },
+        resolve: {
+          alias: {
+            "/style.css": isWebpack5
+              ? false
+              : path.resolve(__dirname, "./fixtures/import/alias.css"),
+          },
+        },
+      }
+    );
+    const stats = await compile(compiler);
+
+    // TODO uncomment after drop webpack v4
+    // expect(getModuleSource("./import/false-alias.css", stats)).toMatchSnapshot(
+    //   "module"
+    // );
+    // expect(getExecutedCode("main.bundle.js", compiler, stats)).toMatchSnapshot(
+    //   "result"
+    // );
+    expect(getWarnings(stats)).toMatchSnapshot("warnings");
+    expect(getErrors(stats)).toMatchSnapshot("errors");
+  });
+
   it("should throw an error on unresolved import", async () => {
     const compiler = getCompiler("./import/unresolved.js");
     const stats = await compile(compiler);

diff --git a/test/modules-option.test.js b/test/modules-option.test.js
@@ -1,6 +1,8 @@
 import path from "path";
 import fs from "fs";
 
+import webpack from "webpack";
+
 import {
   compile,
   getCompiler,
@@ -15,6 +17,8 @@ const testCases = fs.readdirSync(testCasesPath);
 
 jest.setTimeout(60000);
 
+const isWebpack5 = webpack.version.startsWith(5);
+
 describe('"modules" option', () => {
   [
     true,
@@ -843,6 +847,36 @@ describe('"modules" option', () => {
     expect(getErrors(stats)).toMatchSnapshot("errors");
   });
 
+  it('should work with "false" alises', async () => {
+    const compiler = getCompiler(
+      "./modules/icss-false-alias/icss.js",
+      {},
+      {
+        resolve: {
+          alias: {
+            "./unknown.css": isWebpack5
+              ? false
+              : path.resolve(
+                  __dirname,
+                  "./fixtures/modules/icss-false-alias/unknown.css"
+                ),
+          },
+        },
+      }
+    );
+    const stats = await compile(compiler);
+
+    // TODO uncomment after drop webpack v4
+    // expect(
+    //     getModuleSource("./modules/icss-false-alias/relative.icss.css", stats)
+    // ).toMatchSnapshot("module");
+    // expect(getExecutedCode("main.bundle.js", compiler, stats)).toMatchSnapshot(
+    //     "result"
+    // );
+    expect(getWarnings(stats)).toMatchSnapshot("warnings");
+    expect(getErrors(stats)).toMatchSnapshot("errors");
+  });
+
   it('should work with the "auto" when it is "false"', async () => {
     const compiler = getCompiler("./modules/mode/modules.js", {
       modules: {

diff --git a/test/url-option.test.js b/test/url-option.test.js
@@ -279,6 +279,53 @@ describe('"url" option', () => {
     expect(getErrors(stats)).toMatchSnapshot("errors");
   });
 
+  it("should work with 'false' aliases", async () => {
+    const compiler = getCompiler(
+      "./url/false-alias.js",
+      {},
+      {
+        module: {
+          rules: [
+            {
+              test: /\.css$/i,
+              loader: path.resolve(__dirname, "../src"),
+            },
+            isWebpack5
+              ? {
+                  test: /\.(png|jpg|gif|svg|eot|ttf|woff|woff2)$/i,
+                  type: "asset/resource",
+                }
+              : {
+                  test: /\.(png|jpg|gif|svg|eot|ttf|woff|woff2)$/i,
+                  loader: "file-loader",
+                  options: {
+                    name: "[name].[ext]",
+                  },
+                },
+          ],
+        },
+        resolve: {
+          alias: {
+            "/logo.png": isWebpack5
+              ? false
+              : path.resolve(__dirname, "./fixtures/url/logo.png"),
+          },
+        },
+      }
+    );
+    const stats = await compile(compiler);
+
+    // TODO uncomment after drop webpack v4
+    // expect(getModuleSource("./url/false-alias.css", stats)).toMatchSnapshot(
+    //   "module"
+    // );
+    // expect(getExecutedCode("main.bundle.js", compiler, stats)).toMatchSnapshot(
+    //   "result"
+    // );
+    expect(getWarnings(stats)).toMatchSnapshot("warnings");
+    expect(getErrors(stats)).toMatchSnapshot("errors");
+  });
+
   it("should throw an error on unresolved import", async () => {
     const compiler = getCompiler("./url/url-unresolved.js");
     const stats = await compile(compiler);