feat: Refactor resolving and simplify webpack config aliases (#479)

evilebottnawi · jhnns · commit e0fde1a45640 · 2018-04-13T02:28:51.000+02:00
- Make it easier to alias modules via the webpack config
- Make importsToResolve algorithm more readable

BREAKING CHANGE: This slightly changes the resolving algorithm. Should not break in normal usage, but might break in complex configurations.
diff --git a/lib/importsToResolve.js b/lib/importsToResolve.js
@@ -1,58 +1,60 @@
 "use strict";
 
 const path = require("path");
+const utils = require("loader-utils");
 
-// libsass uses this precedence when importing files without extension
-const extPrecedence = [".scss", ".sass", ".css"];
+const matchModuleImport = /^~([^\/]+|@[^\/]+[\/][^\/]+)$/g;
 
 /**
  * When libsass tries to resolve an import, it uses a special algorithm.
  * Since the sass-loader uses webpack to resolve the modules, we need to simulate that algorithm. This function
- * returns an array of import paths to try.
+ * returns an array of import paths to try. The first entry in the array is always the original url
+ * to enable straight-forward webpack.config aliases.
  *
- * @param {string} request
+ * @param {string} url
  * @returns {Array<string>}
  */
-function importsToResolve(request) {
+function importsToResolve(url) {
+    const request = utils.urlToRequest(url);
+    // Keep in mind: ext can also be something like '.datepicker' when the true extension is omitted and the filename contains a dot.
+    // @see https://github.com/webpack-contrib/sass-loader/issues/167
+    const ext = path.extname(request);
+
+    if (matchModuleImport.test(url)) {
+        return [url, request];
+    }
+
     // libsass' import algorithm works like this:
-    // In case there is no file extension...
-    //   - Prefer modules starting with '_'.
-    //   - File extension precedence: .scss, .sass, .css.
+
     // In case there is a file extension...
     //   - If the file is a CSS-file, do not include it all, but just link it via @import url().
     //   - The exact file name must match (no auto-resolving of '_'-modules).
+    if (ext === ".css") {
+        return [];
+    }
+    if (ext === ".scss" || ext === ".sass") {
+        return [url, request];
+    }
 
-    // Keep in mind: ext can also be something like '.datepicker' when the true extension is omitted and the filename contains a dot.
-    // @see https://github.com/webpack-contrib/sass-loader/issues/167
-    const ext = path.extname(request);
+    // In case there is no file extension...
+    //   - Prefer modules starting with '_'.
+    //   - File extension precedence: .scss, .sass, .css.
     const basename = path.basename(request);
-    const dirname = path.dirname(request);
-    const startsWithUnderscore = basename.charAt(0) === "_";
-    const hasCssExt = ext === ".css";
-    const hasSassExt = ext === ".scss" || ext === ".sass";
-
-    // a module import is an identifier like 'bootstrap-sass'
-    // We also need to check for dirname since it might also be a deep import like 'bootstrap-sass/something'
-    let isModuleImport = request.charAt(0) !== "." && dirname === ".";
-
-    if (dirname.charAt(0) === "@") {
-        // Check whether it is a deep import from scoped npm package
-        // (i.e. @pkg/foo/file), if so, process import as file import;
-        // otherwise, if we import from root npm scoped package (i.e. @pkg/foo)
-        // process import as a module import.
-        isModuleImport = !(dirname.indexOf("/") > -1);
+
+    if (basename.charAt(0) === "_") {
+        return [
+            url,
+            `${ request }.scss`, `${ request }.sass`, `${ request }.css`
+        ];
     }
 
-    return (isModuleImport && [request]) || // Do not modify module imports
-        (hasCssExt && []) || // Do not import css files
-        (hasSassExt && [request]) || // Do not modify imports with explicit extensions
-        (startsWithUnderscore ? [] : extPrecedence) // Do not add underscore imports if there is already an underscore
-            .map(ext => "_" + basename + ext)
-            .concat(
-                extPrecedence.map(ext => basename + ext)
-            ).map(
-                file => dirname + "/" + file // No path.sep required here, because imports inside SASS are usually with /
-            );
+    const dirname = path.dirname(request);
+
+    return [
+        url,
+        `${ dirname }/_${ basename }.scss`, `${ dirname }/_${ basename }.sass`, `${ dirname }/_${ basename }.css`,
+        `${ request }.scss`, `${ request }.sass`, `${ request }.css`
+    ];
 }
 
 module.exports = importsToResolve;
diff --git a/lib/webpackImporter.js b/lib/webpackImporter.js
@@ -17,7 +17,6 @@
  */
 
 const path = require("path");
-const utils = require("loader-utils");
 const tail = require("lodash.tail");
 const importsToResolve = require("./importsToResolve");
 
@@ -63,7 +62,7 @@ function webpackImporter(resourcePath, resolve, addNormalizedDependency) {
     return (url, prev, done) => {
         startResolving(
             dirContextFrom(prev),
-            importsToResolve(utils.urlToRequest(url))
+            importsToResolve(url)
         ) // Catch all resolving errors, return the original file and pass responsibility back to other custom importers
             .catch(() => ({ file: url }))
             .then(done);
diff --git a/test/index.test.js b/test/index.test.js
@@ -26,14 +26,14 @@ const loaderContextMock = {
 };
 
 Object.defineProperty(loaderContextMock, "options", {
-    set() {},
+    set() { },
     get() {
         throw new Error("webpack options are not allowed to be accessed anymore.");
     }
 });
 
 syntaxStyles.forEach(ext => {
-    function execTest(testId, options) {
+    function execTest(testId, loaderOptions, webpackOptions) {
         return new Promise((resolve, reject) => {
             const baseConfig = merge({
                 entry: path.join(__dirname, ext, testId + "." + ext),
@@ -45,11 +45,11 @@ syntaxStyles.forEach(ext => {
                         test: new RegExp(`\\.${ ext }$`),
                         use: [
                             { loader: "raw-loader" },
-                            { loader: pathToSassLoader, options }
+                            { loader: pathToSassLoader, options: loaderOptions }
                         ]
                     }]
                 }
-            });
+            }, webpackOptions);
 
             runWebpack(baseConfig, (err) => err ? reject(err) : resolve());
         }).then(() => {
@@ -79,6 +79,13 @@ syntaxStyles.forEach(ext => {
             it("should not resolve CSS imports", () => execTest("import-css"));
             it("should compile bootstrap-sass without errors", () => execTest("bootstrap-sass"));
             it("should correctly import scoped npm packages", () => execTest("import-from-npm-org-pkg"));
+            it("should resolve aliases", () => execTest("import-alias", {}, {
+                resolve: {
+                    alias: {
+                        "path-to-alias": path.join(__dirname, ext, "alias." + ext)
+                    }
+                }
+            }));
         });
         describe("custom importers", () => {
             it("should use custom importer", () => execTest("custom-importer", {
@@ -170,9 +177,11 @@ describe("sass-loader", () => {
                             test: /\.scss$/,
                             use: [
                                 { loader: testLoader.filename },
-                                { loader: pathToSassLoader, options: {
-                                    sourceMap: true
-                                } }
+                                {
+                                    loader: pathToSassLoader, options: {
+                                        sourceMap: true
+                                    }
+                                }
                             ]
                         }]
                     }
@@ -196,7 +205,7 @@ describe("sass-loader", () => {
                     sourceMap.should.not.have.property("file");
                     sourceMap.should.have.property("sourceRoot", fakeCwd);
                     // This number needs to be updated if imports.scss or any dependency of that changes
-                    sourceMap.sources.should.have.length(8);
+                    sourceMap.sources.should.have.length(9);
                     sourceMap.sources.forEach(sourcePath =>
                         fs.existsSync(path.resolve(sourceMap.sourceRoot, sourcePath))
                     );
diff --git a/test/sass/alias.sass b/test/sass/alias.sass
@@ -0,0 +1,3 @@
+a
+  color: red
+
diff --git a/test/sass/import-alias.sass b/test/sass/import-alias.sass
@@ -0,0 +1 @@
+@import path-to-alias
diff --git a/test/sass/import-css.sass b/test/sass/import-css.sass
@@ -1,7 +1,7 @@
 // Special behavior of node-sass/libsass with CSS-files
 // 1. CSS-files are not included, but linked with @import url(path/to/css)
-@import ../node_modules/css/some-css-module.css
+@import ~css/some-css-module.css
 // 2. It does not matter whether the CSS-file exists or not, the file is just linked
 @import ./does/not/exist.css
 // 3. When the .css extension is missing, the file is included just like scss
-@import ../node_modules/css/some-css-module
+@import ~css/some-css-module
diff --git a/test/sass/imports.sass b/test/sass/imports.sass
@@ -2,6 +2,8 @@
 @import another/module
 /* @import another/underscore */
 @import another/underscore
+/* @import another/_underscore */
+@import another/_underscore
 /* @import ~sass/underscore */
 @import ~sass/underscore
 // Import a module with a dot in its name
diff --git a/test/scss/alias.scss b/test/scss/alias.scss
@@ -0,0 +1,3 @@
+a {
+    color: red;
+}
diff --git a/test/scss/import-alias.scss b/test/scss/import-alias.scss
@@ -0,0 +1 @@
+@import 'path-to-alias';
diff --git a/test/scss/imports.scss b/test/scss/imports.scss
@@ -2,6 +2,8 @@
 @import "another/module";
 /* @import "another/underscore"; */
 @import "another/underscore";
+/* @import "another/underscore"; */
+@import "another/_underscore";
 /* @import "~scss/underscore"; */
 @import "~scss/underscore";
 // Import a module with a dot in its name
diff --git a/test/tools/createSpec.js b/test/tools/createSpec.js
@@ -15,6 +15,7 @@ function createSpec(ext) {
     const testNodeModules = path.relative(basePath, path.join(testFolder, "node_modules")) + path.sep;
     const pathToBootstrap = path.relative(basePath, path.resolve(testFolder, "..", "node_modules", "bootstrap-sass"));
     const pathToScopedNpmPkg = path.relative(basePath, path.resolve(testFolder, "node_modules", "@org", "pkg", "./index.scss"));
+    const pathToFooAlias = path.relative(basePath, path.resolve(testFolder, ext, "./alias." + ext));
 
     fs.readdirSync(path.join(testFolder, ext))
         .filter((file) => {
@@ -32,7 +33,8 @@ function createSpec(ext) {
                         url = url
                             .replace(/^~bootstrap-sass/, pathToBootstrap)
                             .replace(/^~@org\/pkg/, pathToScopedNpmPkg)
-                            .replace(/^~/, testNodeModules);
+                            .replace(/^~/, testNodeModules)
+                            .replace(/^path-to-alias/, pathToFooAlias);
                     }
                     return {
                         file: url
