fix: url resolving logic (#843)

BREAKING CHANGE: resolving logic works the same everywhere (it does not matter whether css modules are enabled or not or you setup `global` or `local` mode). Examples - `url('image.png')` as `require('./image.png')`, `url('./image.png')` as `require('./image.png')`, `url('~module/image.png')` as `require('module/image.png')`.

Author: evilebottnawi

README.md

@@ -55,12 +55,18 @@ You can also use the css-loader results directly as a string, such as in Angular
 
 **webpack.config.js**
 ```js
-{
-   test: /\.css$/,
-   use: [
-     'to-string-loader',
-     'css-loader'
-   ]
+module.exports = {
+  module: {
+    rules: [
+      {
+        test: /\.css$/,
+        use: [
+          'to-string-loader',
+          'css-loader'
+        ]
+      }
+    ]
+  }
 }
 ```
 
@@ -106,13 +112,20 @@ It's useful when you, for instance, need to post process the CSS as a string.
 
 ### `url`
 
-To disable `url()` resolving by `css-loader` set the option to `false`.
+Enable/disable `url()` resolving. Absolute `urls` are not resolving by default.
 
-To be compatible with existing css files (if not in CSS Module mode).
+Examples resolutions:
 
 ```
 url(image.png) => require('./image.png')
+url(./image.png) => require('./image.png')
+```
+
+To import styles from a `node_modules` path (include `resolve.modules`) and for `alias`, prefix it with a `~`:
+
+```
 url(~module/image.png) => require('module/image.png')
+url(~aliasDirectory/image.png) => require('otherDirectory/image.png')
 ```
 
 ### `import`
@@ -184,13 +197,6 @@ exports.locals = {
 
 CamelCase is recommended for local selectors. They are easier to use within the imported JS module.
 
-`url()` URLs in block scoped (`:local .abc`) rules behave like requests in modules.
-
-```
-file.png => ./file.png
-~module/file.png => module/file.png
-```
-
 You can use `:local(#someId)`, but this is not recommended. Use classes instead of ids.
 
 ##### `Composing`

lib/loader.js

@@ -72,25 +72,7 @@ module.exports = function loader(content, map, meta) {
 
     plugins.push(
       modulesValues,
-      localByDefault({
-        mode,
-        rewriteUrl(global, url) {
-          if (resolveUrl) {
-            // eslint-disable-next-line no-param-reassign
-            url = url.trim();
-
-            if (!url.replace(/\s/g, '').length || !isUrlRequest(url)) {
-              return url;
-            }
-
-            if (global) {
-              return urlToRequest(url);
-            }
-          }
-
-          return url;
-        },
-      }),
+      localByDefault({ mode }),
       extractImports(),
       modulesScope({
         generateScopedName: function generateScopedName(exportName) {
@@ -117,7 +99,11 @@ module.exports = function loader(content, map, meta) {
   }
 
   if (resolveUrl) {
-    plugins.push(urlParser());
+    plugins.push(
+      urlParser({
+        filter: (value) => isUrlRequest(value),
+      })
+    );
   }
 
   plugins.push(icssParser());
@@ -251,7 +237,7 @@ module.exports = function loader(content, map, meta) {
 
               return `" + escape(require(${stringifyRequest(
                 this,
-                normalizedUrl
+                urlToRequest(normalizedUrl)
               )})${hash ? ` + ${hash}` : ''}) + "`;
             }
           );

lib/plugins/postcss-url-parser.js

@@ -1,6 +1,5 @@
 const postcss = require('postcss');
 const valueParser = require('postcss-value-parser');
-const { isUrlRequest } = require('loader-utils');
 
 const pluginName = 'postcss-url-parser';
 
@@ -97,11 +96,9 @@ function uniq(array) {
 
 module.exports = postcss.plugin(
   pluginName,
-  () =>
+  (options) =>
     function process(css, result) {
-      const traversed = walkDeclsWithUrl(css, result, (value) =>
-        isUrlRequest(value)
-      );
+      const traversed = walkDeclsWithUrl(css, result, options.filter);
       const paths = uniq(flatten(traversed.map((item) => item.values)));
 
       if (paths.length === 0) {

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

@@ -35,7 +35,7 @@
 }
 
 .class {
-  background: green /* url(~package/img.png) */ url(./other-img.png) xyz;
+  background: green url(~package/img.png) url(./other-img.png) xyz;
 }
 
 .class {
@@ -82,7 +82,7 @@
   src: url(./font.woff) format('woff'),
     url('./font.woff2') format('woff2'),
     url("./font.eot") format('eot'),
-    /*url(~package/font.ttf) format('truetype'), */
+    url(~package/font.ttf) format('truetype'),
     url("./font with spaces.eot") format("embedded-opentype"),
     url('./font.svg#svgFontName') format('svg'),
     url('./font.woff2?foo=bar') format('woff2'),
@@ -177,3 +177,23 @@ b {
   background: ___CSS_LOADER_IMPORT___INDEX___;
   background: ___CSS_LOADER_IMPORT___99999___;
 }
+
+.pure-url {
+  background: url('img-simple.png');
+}
+
+.not-resolved {
+  background: url('/img-simple.png');
+}
+
+.above-below {
+  background: url('../url/img-simple.png');
+}
+
+.tilde {
+  background: url('~package/img.png');
+}
+
+.aliases {
+  background: url('~aliasesImg/img.png') ;
+}

test/fixtures/url/img-simple.png

@@ -52,8 +52,13 @@ function evaluated(output, modules, moduleId = 1) {
             'modules/tests-cases/composes-with-importing',
             'modules/tests-cases/media-2',
           ].map((importedPath) =>
-            path.resolve(__dirname, `./fixtures/${importedPath}`, module)
+            path.resolve(
+              __dirname,
+              `./fixtures/${importedPath}`,
+              module.replace('aliasesImg/', '')
+            )
           );
+
           return importedPaths.includes(modulePath);
         });
 
@@ -165,6 +170,11 @@ function compile(fixture, config = {}, options = {}) {
     optimization: {
       runtimeChunk: true,
     },
+    resolve: {
+      alias: {
+        aliasesImg: path.resolve(__dirname, 'fixtures/url'),
+      },
+    },
   };
 
   // Compiler Options

test/fixtures/url/url.css

@@ -31,4 +31,27 @@ describe('url option', () => {
     expect(stats.compilation.warnings).toMatchSnapshot('warnings');
     expect(stats.compilation.errors).toMatchSnapshot('errors');
   });
+
+  [true, 'local', 'global', false].forEach((modulesValue) => {
+    it(`true and modules \`${modulesValue}\``, async () => {
+      const config = {
+        loader: { options: { modules: modulesValue } },
+      };
+      const testId = './url/url.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'
+      );
+    });
+  });
 });