feat: import option doesn't affect on composes (#822)

BREAKING CHANGE: `import` option only affect on `@import` at-rules and doesn't affect on `composes` declarations
webpack-contrib · Nov 29, 2018 · f9aa73c · f9aa73c
1 parent 3b573a9
commit f9aa73c
Show file tree

Hide file tree

Showing 7 changed files with 137 additions and 29 deletions.
diff --git a/README.md b/README.md
@@ -116,20 +116,16 @@ url(~module/image.png) => require('module/image.png')
 
 ### `import`
 
-To import styles from a node module path, prefix it with a `~`:
+To disable `@import` resolving by `css-loader` set the option to `false`.
 
-```css
-@import '~module/styles.css';
-```
+Absolute urls are not resolving.
 
-To disable `@import` resolving by `css-loader` set the option to `false`
+To import styles from a node module path, prefix it with a `~`:
 
 ```css
-@import url('https://fonts.googleapis.com/css?family=Roboto');
+@import '~module/styles.css';
 ```
 
-> _⚠️ Use with caution, since this disables resolving for **all** `@import`s, including css modules `composes: xxx from 'path/to/file.css'` feature._
-
 ### [`modules`](https://github.com/css-modules/css-modules)
 
 The query parameter `modules` enables the **CSS Modules** spec.

diff --git a/lib/loader.js b/lib/loader.js
@@ -60,16 +60,21 @@ module.exports = function loader(content, map) {
           return true;
         })
         .map((imp) => {
-          if (!loaderUtils.isUrlRequest(imp.url)) {
+          const { url } = imp;
+          const media = imp.media || '';
+
+          if (!loaderUtils.isUrlRequest(url)) {
             return `exports.push([module.id, ${JSON.stringify(
-              `@import url(${imp.url});`
-            )}, ${JSON.stringify(imp.media)}]);`;
+              `@import url(${url});`
+            )}, ${JSON.stringify(media)}]);`;
           }
-          const importUrl = importUrlPrefix + imp.url;
+
+          const importUrl = importUrlPrefix + url;
+
           return `exports.i(require(${loaderUtils.stringifyRequest(
             this,
             importUrl
-          )}), ${JSON.stringify(imp.media)});`;
+          )}), ${JSON.stringify(media)});`;
         }, this)
         .join('\n');
 

diff --git a/lib/plugins/postcss-icss-parser.js b/lib/plugins/postcss-icss-parser.js
@@ -28,26 +28,23 @@ module.exports = postcss.plugin(
       });
 
       function replaceImportsInString(str) {
-        if (options.import) {
-          const tokens = valueParser(str);
+        const tokens = valueParser(str);
 
-          tokens.walk((node) => {
-            if (node.type !== 'word') {
-              return;
-            }
+        tokens.walk((node) => {
+          if (node.type !== 'word') {
+            return;
+          }
 
-            const token = node.value;
-            const importIndex = imports[`$${token}`];
+          const token = node.value;
+          const importIndex = imports[`$${token}`];
 
-            if (typeof importIndex === 'number') {
-              // eslint-disable-next-line no-param-reassign
-              node.value = `___CSS_LOADER_IMPORT___${importIndex}___`;
-            }
-          });
+          if (typeof importIndex === 'number') {
+            // eslint-disable-next-line no-param-reassign
+            node.value = `___CSS_LOADER_IMPORT___${importIndex}___`;
+          }
+        });
 
-          return tokens.toString();
-        }
-        return str;
+        return tokens.toString();
       }
 
       // Replace tokens in declarations

diff --git a/test/__snapshots__/import-option.test.js.snap b/test/__snapshots__/import-option.test.js.snap
@@ -1,5 +1,84 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`import option false and modules false: errors 1`] = `Array []`;
+
+exports[`import option false and modules false: module (evaluated) 1`] = `
+Array [
+  Array [
+    2,
+    "
+",
+    "",
+  ],
+  Array [
+    1,
+    "@import url(test-other.css) (min-width: 100px);
+
+.ghi {
+  color: red;
+}
+",
+    "",
+  ],
+]
+`;
+
+exports[`import option false and modules false: module 1`] = `
+"exports = module.exports = require(\\"../../../lib/runtime/api.js\\")(false);
+// imports
+exports.i(require(\\"-!../../../index.js??ref--4-0!./values.css\\"), \\"\\");
+
+// module
+exports.push([module.id, \\"@import url(test-other.css) (min-width: 100px);\\\\n\\\\n.ghi {\\\\n  color: \\" + require(\\"-!../../../index.js??ref--4-0!./values.css\\").locals[\\"def\\"] + \\";\\\\n}\\\\n\\", \\"\\"]);
+
+// exports
+exports.locals = {
+	\\"def\\": \\"\\" + require(\\"-!../../../index.js??ref--4-0!./values.css\\").locals[\\"def\\"] + \\"\\"
+};"
+`;
+
+exports[`import option false and modules false: warnings 1`] = `Array []`;
+
+exports[`import option false and modules true: errors 1`] = `Array []`;
+
+exports[`import option false and modules true: module (evaluated) 1`] = `
+Array [
+  Array [
+    2,
+    "
+",
+    "",
+  ],
+  Array [
+    1,
+    "@import url(test-other.css) (min-width: 100px);
+
+._3r49KZIIAltPknAjdNVZ-7 {
+  color: red;
+}
+",
+    "",
+  ],
+]
+`;
+
+exports[`import option false and modules true: module 1`] = `
+"exports = module.exports = require(\\"../../../lib/runtime/api.js\\")(false);
+// imports
+exports.i(require(\\"-!../../../index.js??ref--4-0!./values.css\\"), \\"\\");
+
+// module
+exports.push([module.id, \\"@import url(test-other.css) (min-width: 100px);\\\\n\\\\n._3r49KZIIAltPknAjdNVZ-7 {\\\\n  color: \\" + require(\\"-!../../../index.js??ref--4-0!./values.css\\").locals[\\"def\\"] + \\";\\\\n}\\\\n\\", \\"\\"]);
+
+// exports
+exports.locals = {
+	\\"def\\": \\"\\" + require(\\"-!../../../index.js??ref--4-0!./values.css\\").locals[\\"def\\"] + \\"\\",
+	\\"ghi\\": \\"_3r49KZIIAltPknAjdNVZ-7\\"
+};"
+`;
+
+exports[`import option false and modules true: warnings 1`] = `Array []`;
+
 exports[`import option false: errors 1`] = `Array []`;
 
 exports[`import option false: module (evaluated) 1`] = `

diff --git a/test/fixtures/import/css-modules.css b/test/fixtures/import/css-modules.css
@@ -0,0 +1,7 @@
+@import url(test-other.css) (min-width: 100px);
+
+@value def from './values.css';
+
+.ghi {
+  color: def;
+}
diff --git a/test/fixtures/import/values.css b/test/fixtures/import/values.css
@@ -0,0 +1 @@
+@value def: red;
diff --git a/test/import-option.test.js b/test/import-option.test.js
@@ -31,4 +31,27 @@ describe('import option', () => {
     expect(stats.compilation.warnings).toMatchSnapshot('warnings');
     expect(stats.compilation.errors).toMatchSnapshot('errors');
   });
+
+  [true, false].forEach((modulesValue) => {
+    it(`false and modules ${modulesValue}`, async () => {
+      const config = {
+        loader: { options: { import: false, modules: modulesValue } },
+      };
+      const testId = './import/css-modules.css';
+      const stats = await webpack(testId, config);
+      const { modules } = stats.toJson();
+      const module = modules.find((m) => m.id === testId);
+
+      expect(module.source).toMatchSnapshot('module');
+      expect(evaluated(module.source, modules)).toMatchSnapshot(
+        'module (evaluated)'
+      );
+      expect(normalizeErrors(stats.compilation.warnings)).toMatchSnapshot(
+        'warnings'
+      );
+      expect(normalizeErrors(stats.compilation.errors)).toMatchSnapshot(
+        'errors'
+      );
+    });
+  });
 });