fix: keep order of @imports with the webpackIgnore comment (#1600)

Author: alexander-akait

Diff for: src/plugins/postcss-import-parser.js

@@ -8,12 +8,7 @@ import {
   WEBPACK_IGNORE_COMMENT_REGEXP,
 } from "../utils";
 
-function parseNode(atRule, key, options) {
-  // Convert only top-level @import
-  if (atRule.parent.type !== "root") {
-    return;
-  }
-
+function isIgnoredAfterName(atRule) {
   if (
     atRule.raws &&
     atRule.raws.afterName &&
@@ -25,20 +20,35 @@ function parseNode(atRule, key, options) {
       .match(WEBPACK_IGNORE_COMMENT_REGEXP);
 
     if (matched && matched[2] === "true") {
-      return;
+      return true;
     }
   }
 
+  return false;
+}
+
+function isIgnoredPrevNode(atRule) {
   const prevNode = atRule.prev();
 
   if (prevNode && prevNode.type === "comment") {
     const matched = prevNode.text.match(WEBPACK_IGNORE_COMMENT_REGEXP);
 
     if (matched && matched[2] === "true") {
-      return;
+      return true;
     }
   }
 
+  return false;
+}
+
+function parseNode(atRule, key, options) {
+  // Convert only top-level @import
+  if (atRule.parent.type !== "root") {
+    return;
+  }
+
+  const isIgnored = isIgnoredAfterName(atRule) || isIgnoredPrevNode(atRule);
+
   // Nodes do not exists - `@import url('http://') :root {}`
   if (atRule.nodes) {
     const error = new Error(
@@ -97,7 +107,12 @@ function parseNode(atRule, key, options) {
 
   url = normalizeUrl(url, isStringValue);
 
-  const { requestable, needResolve } = isURLRequestable(url, options);
+  let requestable = false;
+  let needResolve = false;
+
+  if (!isIgnored) {
+    ({ requestable, needResolve } = isURLRequestable(url, options));
+  }
 
   let prefix;
 

Diff for: test/__snapshots__/import-option.test.js.snap

@@ -1,5 +1,41 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`"import" option should jeep order of imports with 'webpackIgnore': errors 1`] = `[]`;
+
+exports[`"import" option should jeep order of imports with 'webpackIgnore': module 1`] = `
+"// Imports
+import ___CSS_LOADER_API_NO_SOURCEMAP_IMPORT___ from "../../../src/runtime/noSourceMaps.js";
+import ___CSS_LOADER_API_IMPORT___ from "../../../src/runtime/api.js";
+import ___CSS_LOADER_AT_RULE_IMPORT_0___ from "-!../../../src/index.js??ruleSet[1].rules[0].use[0]!./test.css";
+var ___CSS_LOADER_EXPORT___ = ___CSS_LOADER_API_IMPORT___(___CSS_LOADER_API_NO_SOURCEMAP_IMPORT___);
+___CSS_LOADER_EXPORT___.push([module.id, "@import url(/assets/themes.css);"]);
+___CSS_LOADER_EXPORT___.i(___CSS_LOADER_AT_RULE_IMPORT_0___);
+// Module
+___CSS_LOADER_EXPORT___.push([module.id, \`/*! /* webpackIgnore: true */
+
+body {
+  background: red;
+}
+\`, ""]);
+// Exports
+export default ___CSS_LOADER_EXPORT___;
+"
+`;
+
+exports[`"import" option should jeep order of imports with 'webpackIgnore': result 1`] = `
+"@import url(/assets/themes.css);.test {
+  a: a;
+}
+/*! /* webpackIgnore: true */
+
+body {
+  background: red;
+}
+"
+`;
+
+exports[`"import" option should jeep order of imports with 'webpackIgnore': warnings 1`] = `[]`;
+
 exports[`"import" option should keep original order: errors 1`] = `[]`;
 
 exports[`"import" option should keep original order: module 1`] = `

Diff for: test/__snapshots__/loader.test.js.snap

@@ -2558,6 +2558,11 @@ var ___CSS_LOADER_URL_IMPORT_3___ = new URL("./fonts/Roboto-Regular.ttf", import
 var ___CSS_LOADER_URL_IMPORT_4___ = new URL("./fonts/Roboto-Regular.svg", import.meta.url);
 var ___CSS_LOADER_URL_IMPORT_5___ = new URL("./fonts/Roboto-Regular.eot", import.meta.url);
 var ___CSS_LOADER_EXPORT___ = ___CSS_LOADER_API_IMPORT___(___CSS_LOADER_API_NO_SOURCEMAP_IMPORT___);
+___CSS_LOADER_EXPORT___.push([module.id, "@import url(./basic.css);"]);
+___CSS_LOADER_EXPORT___.push([module.id, "@import url(./imported.css);"]);
+___CSS_LOADER_EXPORT___.push([module.id, "@import url(./simple.css);"]);
+___CSS_LOADER_EXPORT___.push([module.id, "@import url(./simple.css);"]);
+___CSS_LOADER_EXPORT___.push([module.id, "@import url(./simple.css);"]);
 ___CSS_LOADER_EXPORT___.i(___CSS_LOADER_AT_RULE_IMPORT_0___);
 ___CSS_LOADER_EXPORT___.i(___CSS_LOADER_AT_RULE_IMPORT_0___);
 ___CSS_LOADER_EXPORT___.i(___CSS_LOADER_AT_RULE_IMPORT_0___);
@@ -2572,11 +2577,6 @@ var ___CSS_LOADER_URL_REPLACEMENT_6___ = ___CSS_LOADER_GET_URL_IMPORT___(___CSS_
 var ___CSS_LOADER_URL_REPLACEMENT_7___ = ___CSS_LOADER_GET_URL_IMPORT___(___CSS_LOADER_URL_IMPORT_0___, { needQuotes: true });
 // Module
 ___CSS_LOADER_EXPORT___.push([module.id, \`/* webpackIgnore: true */
-@import url(./basic.css);
-@import /*    webpackIgnore:   true    */ url(./imported.css);
-@import /* webpackIgnore: false */ /* webpackIgnore: true */ url(./simple.css);
-@import /* webpackIgnore: false */ /* webpackIgnore: true */ /* webpackIgnore: true */ url(./simple.css);
-@import /* webpackIgnore: false */ /* webpackIgnore: false */ /* webpackIgnore: true */ url(./simple.css);
 
 /** Resolved **/
 /** Resolved **/
@@ -2830,6 +2830,26 @@ export default ___CSS_LOADER_EXPORT___;
 
 exports[`loader should work with webpackIgnore comment: result 1`] = `
 [
+  [
+    "./webpackIgnore.css",
+    "@import url(./basic.css);",
+  ],
+  [
+    "./webpackIgnore.css",
+    "@import url(./imported.css);",
+  ],
+  [
+    "./webpackIgnore.css",
+    "@import url(./simple.css);",
+  ],
+  [
+    "./webpackIgnore.css",
+    "@import url(./simple.css);",
+  ],
+  [
+    "./webpackIgnore.css",
+    "@import url(./simple.css);",
+  ],
   [
     "../../src/index.js??ruleSet[1].rules[0].use[0]!./simple.css",
     ".some-class {
@@ -2865,11 +2885,6 @@ exports[`loader should work with webpackIgnore comment: result 1`] = `
   [
     "./webpackIgnore.css",
     "/* webpackIgnore: true */
-@import url(./basic.css);
-@import /*    webpackIgnore:   true    */ url(./imported.css);
-@import /* webpackIgnore: false */ /* webpackIgnore: true */ url(./simple.css);
-@import /* webpackIgnore: false */ /* webpackIgnore: true */ /* webpackIgnore: true */ url(./simple.css);
-@import /* webpackIgnore: false */ /* webpackIgnore: false */ /* webpackIgnore: true */ url(./simple.css);
 
 /** Resolved **/
 /** Resolved **/

Diff for: test/fixtures/import/webpackIgnore-order.css

@@ -0,0 +1,7 @@
+/*! /* webpackIgnore: true */
+@import url("/assets/themes.css");
+@import "~test";
+
+body {
+  background: red;
+}

Diff for: test/fixtures/import/webpackIgnore-order.js

@@ -0,0 +1,5 @@
+import css from './webpackIgnore-order.css';
+
+__export__ = css.toString();
+
+export default css;

Diff for: test/import-option.test.js

@@ -588,4 +588,18 @@ describe('"import" option', () => {
     expect(getWarnings(stats)).toMatchSnapshot("warnings");
     expect(getErrors(stats)).toMatchSnapshot("errors");
   });
+
+  it("should jeep order of imports with 'webpackIgnore'", async () => {
+    const compiler = getCompiler("./import/webpackIgnore-order.js");
+    const stats = await compile(compiler);
+
+    expect(
+      getModuleSource("./import/webpackIgnore-order.css", stats),
+    ).toMatchSnapshot("module");
+    expect(getExecutedCode("main.bundle.js", compiler, stats)).toMatchSnapshot(
+      "result",
+    );
+    expect(getWarnings(stats)).toMatchSnapshot("warnings");
+    expect(getErrors(stats)).toMatchSnapshot("errors");
+  });
 });