Repository: webpack-contrib/css-loader
Pull request #946 (2019-06-04 11:06:03)

refactor: postcss plugins
<!--
  HOLY CRAP a Pull Request. We ❤️ those!

  If you remove or skip this template, you'll make the 🐼 sad and the mighty god
  of Github will appear and pile-drive the close button from a great height
  while making animal noises.

  Please place an x (no spaces!) in all [ ] that apply
-->

This PR contains a:

- [ ] **bugfix**
- [ ] new **feature**
- [x] **code refactor**
- [ ] **test update** <!-- if bug or feature is checked, this should be too -->
- [ ] **typo fix**
- [ ] **metadata update**

### Motivation / Use-Case

refactor for future fixes

### Breaking Changes

No

### Additional Info

No

codecov[bot]: # [Codecov](https://codecov.io/gh/webpack-contrib/css-loader/pull/946?src=pr&el=h1) Report
> Merging [#946](https://codecov.io/gh/webpack-contrib/css-loader/pull/946?src=pr&el=desc) into [master](https://codecov.io/gh/webpack-contrib/css-loader/commit/6701cbd410d118de22953093669226789b5754c8?src=pr&el=desc) will **decrease** coverage by `0.22%`.
> The diff coverage is `95.83%`.

[![Impacted file tree graph](https://codecov.io/gh/webpack-contrib/css-loader/pull/946/graphs/tree.svg?width=650&token=DlN3OQ0JnO&height=150&src=pr)](https://codecov.io/gh/webpack-contrib/css-loader/pull/946?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #946      +/-   ##
==========================================
- Coverage   98.99%   98.76%   -0.23%     
==========================================
  Files          10       10              
  Lines         398      406       +8     
  Branches      119      124       +5     
==========================================
+ Hits          394      401       +7     
- Misses          4        5       +1
```


| [Impacted Files](https://codecov.io/gh/webpack-contrib/css-loader/pull/946?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/index.js](https://codecov.io/gh/webpack-contrib/css-loader/pull/946/diff?src=pr&el=tree#diff-c3JjL2luZGV4Lmpz) | `100% <100%> (ø)` | :arrow_up: |
| [src/plugins/postcss-url-parser.js](https://codecov.io/gh/webpack-contrib/css-loader/pull/946/diff?src=pr&el=tree#diff-c3JjL3BsdWdpbnMvcG9zdGNzcy11cmwtcGFyc2VyLmpz) | `97.53% <95.65%> (-1.1%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/webpack-contrib/css-loader/pull/946?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/webpack-contrib/css-loader/pull/946?src=pr&el=footer). Last update [6701cbd...1204069](https://codecov.io/gh/webpack-contrib/css-loader/pull/946?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/src/index.js b/src/index.js
index 5580999..3a8bbbf 100644
--- a/src/index.js
+++ b/src/index.js
@@ -62,6 +62,8 @@ export default function loader(content, map, meta) {
     plugins.push(...getModulesPlugins(options, this));
   }
 
+  plugins.push(icssParser());
+
   if (options.import !== false) {
     plugins.push(
       importParser({
@@ -80,8 +82,6 @@ export default function loader(content, map, meta) {
     );
   }
 
-  plugins.push(icssParser());
-
   postcss(plugins)
     .process(content, {
       from: getRemainingRequest(this)
diff --git a/src/plugins/postcss-url-parser.js b/src/plugins/postcss-url-parser.js
index 96fd718..b39cb0d 100644
--- a/src/plugins/postcss-url-parser.js
+++ b/src/plugins/postcss-url-parser.js
@@ -49,38 +49,54 @@ function walkUrls(parsed, callback) {
   });
 }
 
-function walkDeclsWithUrl(css, result, filter) {
-  const items = [];
+function getUrlsFromValue(value, result, filter, decl = null) {
+  if (!needParseDecl.test(value)) {
+    return;
+  }
+
+  const parsed = valueParser(value);
+  const urls = [];
+
+  walkUrls(parsed, (node, url, needQuotes) => {
+    if (url.trim().replace(/\\[\r\n]/g, '').length === 0) {
+      result.warn(
+        `Unable to find uri in '${decl ? decl.toString() : value}'`,
+        decl
+          ? {
+              node: decl,
+            }
+          : {}
+      );
 
-  css.walkDecls((decl) => {
-    if (!needParseDecl.test(decl.value)) {
       return;
     }
 
-    const parsed = valueParser(decl.value);
-    const urls = [];
+    if (filter && !filter(url)) {
+      return;
+    }
 
-    walkUrls(parsed, (node, url, needQuotes) => {
-      if (url.trim().replace(/\\[\r\n]/g, '').length === 0) {
-        result.warn(`Unable to find uri in '${decl.toString()}'`, {
-          node: decl,
-        });
+    urls.push({ url, needQuotes });
+  });
 
-        return;
-      }
+  // eslint-disable-next-line consistent-return
+  return { parsed, urls };
+}
 
-      if (filter && !filter(url)) {
-        return;
-      }
+function walkDeclsWithUrl(css, result, filter) {
+  const items = [];
 
-      urls.push({ url, needQuotes });
-    });
+  css.walkDecls((decl) => {
+    const item = getUrlsFromValue(decl.value, result, filter, decl);
+
+    if (!item) {
+      return;
+    }
 
-    if (urls.length === 0) {
+    if (item.urls.length === 0) {
       return;
     }
 
-    items.push({ decl, parsed, urls });
+    items.push({ decl, parsed: item.parsed, urls: item.urls });
   });
 
   return items;


Test patch:
diff --git a/test/__snapshots__/modules-option.test.js.snap b/test/__snapshots__/modules-option.test.js.snap
index 6a6eabf..8bd1f02 100644
--- a/test/__snapshots__/modules-option.test.js.snap
+++ b/test/__snapshots__/modules-option.test.js.snap
@@ -8396,26 +8396,18 @@ exports[`modules composes should supports resolving: module (evaluated) 1`] = `
 Array [
   Array [
     2,
-    "._2Nehoi0wFTN-8p0fEfvUYd {
-  d: d;
-}
-",
-    "(min-width: 100px)",
-  ],
-  Array [
-    3,
     "
 ",
     "",
   ],
   Array [
-    4,
+    3,
     "
 ",
     "",
   ],
   Array [
-    5,
+    4,
     "._22ETk3ZhhvjVe-4dAULcsF {
   display: block;
 }
@@ -8423,7 +8415,7 @@ Array [
     "",
   ],
   Array [
-    6,
+    5,
     ".dSGVakTZ8YY-XNW64tg9F {
   display: inline;
 }
@@ -8431,7 +8423,7 @@ Array [
     "",
   ],
   Array [
-    7,
+    6,
     "._1SUuyapRQ82dAPS8r7zmpY {
   display: flex;
 }
@@ -8439,7 +8431,7 @@ Array [
     "",
   ],
   Array [
-    8,
+    7,
     "._2n5szs-7uGKzfFTp9NTK64 {
   display: inline-block;
 }
@@ -8447,13 +8439,21 @@ Array [
     "",
   ],
   Array [
-    9,
+    8,
     "._37nzOIvIANGnEtP1lWjZDN {
   display: table;
 }
 ",
     "",
   ],
+  Array [
+    9,
+    "._2Nehoi0wFTN-8p0fEfvUYd {
+  d: d;
+}
+",
+    "(min-width: 100px)",
+  ],
   Array [
     1,
     "._14uFt0lIVKKAlKTTT29IIQ {
@@ -8533,7 +8533,6 @@ Array [
 exports[`modules composes should supports resolving: module 1`] = `
 "exports = module.exports = require(\\"../../../src/runtime/api.js\\")(false);
 // Imports
-exports.i(require(\\"-!../../../src/index.js??ref--4-0!./test-other.css\\"), \\"(min-width: 100px)\\");
 exports.i(require(\\"-!../../../src/index.js??ref--4-0!./values.css\\"), \\"\\");
 exports.i(require(\\"-!../../../src/index.js??ref--4-0!./something.css\\"), \\"\\");
 exports.i(require(\\"-!../../../src/index.js??ref--4-0!./imported-simple.css\\"), \\"\\");
@@ -8541,6 +8540,7 @@ exports.i(require(\\"-!../../../src/index.js??ref--4-0!./relative.css\\"), \\"\\
 exports.i(require(\\"-!../../../src/index.js??ref--4-0!../modules/top-relative.css\\"), \\"\\");
 exports.i(require(\\"-!../../../src/index.js??ref--4-0!package/style.css\\"), \\"\\");
 exports.i(require(\\"-!../../../src/index.js??ref--4-0!aliasesComposes/alias.css\\"), \\"\\");
+exports.i(require(\\"-!../../../src/index.js??ref--4-0!./test-other.css\\"), \\"(min-width: 100px)\\");
 
 // Module
 exports.push([module.id, \\"._14uFt0lIVKKAlKTTT29IIQ {\\\\n  color: \\" + require(\\"-!../../../src/index.js??ref--4-0!./values.css\\").locals[\\"v-def\\"] + \\";\\\\n}\\\\n\\\\n._3XDgIzfUIQkaUInpEdo7fN {\\\\n  color: blue;\\\\n}\\\\n\\\\n._1wABXM_RabWHj--wsPrhvM {\\\\n  display: block;\\\\n}\\\\n\\\\n._1DFEYnAfn9LZyk4fErI86e {\\\\n  width: \\" + require(\\"-!../../../src/index.js??ref--4-0!./something.css\\").locals[\\"v-something\\"] + \\";\\\\n}\\\\n\\\\n.Ywv5coVC2RU-pIFhN9O4w {\\\\n  color: \\" + require(\\"-!../../../src/index.js??ref--4-0!./values.css\\").locals[\\"v-other\\"] + \\";\\\\n}\\\\n\\\\n._1tAbIwITRWAdZZE6wKNk9O {\\\\n  prop: \\" + require(\\"-!../../../src/index.js??ref--4-0!./values.css\\").locals[\\"v-def\\"] + \\";\\\\n  duplicate: \\" + require(\\"-!../../../src/index.js??ref--4-0!./values.css\\").locals[\\"v-other\\"] + \\";\\\\n}\\\\n\\\\n.Q3SQ3BwtBwUFLlg6adzOI {\\\\n  color: red;\\\\n}\\\\n\\\\n._1n5XhXj4SFnYrwziC3un0d {\\\\n  color: yellow;\\\\n}\\\\n\\\\n._3dnFnGkAVAiMA6etF-naHc {\\\\n  color: gray;\\\\n}\\\\n\\\\n._1xUePnlnafMQ1cExy3PUWT {\\\\n  color: gray;\\\\n}\\\\n\\\\n._26Jdfenl9Xn8HXwb2jipvt {\\\\n  color: gainsboro;\\\\n}\\\\n\\\\n._1ya4VhsDkuPhQeVHQydw2Y {\\\\n  color: #BF4040;\\\\n}\\\\n\\\\n.sGE1Q_LliVEZU2Q4q9j4K {\\\\n  color: black;\\\\n}\\\\n\\\\n@media (min-width: 960px) {\\\\n  ._2zSMJ4hQh0FesbZjiKW_ya {\\\\n    padding: 0 20px;\\\\n  }\\\\n}\\\\n\\\\n.\\" + require(\\"-!../../../src/index.js??ref--4-0!./values.css\\").locals[\\"s-white\\"] + \\" {\\\\n  color: white;\\\\n}\\\\n\\\\n@media \\" + require(\\"-!../../../src/index.js??ref--4-0!./values.css\\").locals[\\"m-small\\"] + \\" {\\\\n  ._2zSMJ4hQh0FesbZjiKW_ya {\\\\n    padding: 20px 20px;\\\\n  }\\\\n}\\\\n\\", \\"\\"]);