fix: compatibility with custom importers for node-sass (#927)

Author: alexander-akait

package-lock.json

@@ -39,10 +39,10 @@
     "dist"
   ],
   "peerDependencies": {
-    "webpack": "^5.0.0",
+    "fibers": ">= 3.1.0",
     "node-sass": "^4.0.0 || ^5.0.0",
     "sass": "^1.3.0",
-    "fibers": ">= 3.1.0"
+    "webpack": "^5.0.0"
   },
   "peerDependenciesMeta": {
     "node-sass": {
@@ -83,17 +83,18 @@
     "foundation-sites": "^6.6.3",
     "husky": "^4.3.6",
     "jest": "^26.6.3",
-    "lint-staged": "^10.5.3",
+    "lint-staged": "^10.5.4",
     "material-components-web": "^8.0.0",
     "memfs": "^3.2.0",
     "node-sass": "^5.0.0",
+    "node-sass-glob-importer": "^5.3.2",
     "npm-run-all": "^4.1.5",
     "prettier": "^2.2.1",
     "sass": "^1.32.0",
     "semver": "^7.3.4",
     "standard-version": "^9.1.0",
     "style-loader": "^2.0.0",
-    "webpack": "^5.20.2"
+    "webpack": "^5.21.2"
   },
   "keywords": [
     "sass",

package.json

@@ -207,15 +207,16 @@ async function getSassOptions(
   return options;
 }
 
+const MODULE_REQUEST_REGEX = /^[^?]*~/;
+
 // Examples:
 // - ~package
 // - ~package/
 // - ~@org
 // - ~@org/
 // - ~@org/package
 // - ~@org/package/
-const isModuleImport = /^~([^/]+|[^/]+\/|@[^/]+[/][^/]+|@[^/]+\/?|@[^/]+[/][^/]+\/)$/;
-const MODULE_REQUEST_REGEX = /^[^?]*~/;
+const IS_MODULE_IMPORT = /^~([^/]+|[^/]+\/|@[^/]+[/][^/]+|@[^/]+\/?|@[^/]+[/][^/]+\/)$/;
 
 /**
  * When `sass`/`node-sass` tries to resolve an import, it uses a special algorithm.
@@ -244,7 +245,7 @@ function getPossibleRequests(
       request = request.replace(MODULE_REQUEST_REGEX, "");
     }
 
-    if (isModuleImport.test(url)) {
+    if (IS_MODULE_IMPORT.test(url)) {
       request = request[request.length - 1] === "/" ? request : `${request}/`;
 
       return [...new Set([request, url])];
@@ -310,15 +311,13 @@ const IS_NATIVE_WIN32_PATH = /^[a-z]:[/\\]|^\\\\/i;
  * @param {Object} implementation - The imported Sass implementation, both
  *   `sass` (Dart Sass) and `node-sass` are supported.
  * @param {string[]} [includePaths] - The list of include paths passed to Sass.
- * @param {boolean} [rootContext] - The configured Webpack root context.
  *
  * @throws If a compatible Sass implementation cannot be found.
  */
 function getWebpackResolver(
   resolverFactory,
   implementation,
-  includePaths = [],
-  rootContext = false
+  includePaths = []
 ) {
   async function startResolving(resolutionMap) {
     if (resolutionMap.length === 0) {
@@ -380,6 +379,13 @@ function getWebpackResolver(
   );
 
   return (context, request) => {
+    // See https://github.com/webpack/webpack/issues/12340
+    // Because `node-sass` calls our importer before `1. Filesystem imports relative to the base file.`
+    // custom importer may not return `{ file: '/path/to/name.ext' }` and therefore our `context` will be relative
+    if (!isDartSass && !path.isAbsolute(context)) {
+      return Promise.reject();
+    }
+
     const originalRequest = request;
     const isFileScheme = originalRequest.slice(0, 5).toLowerCase() === "file:";
 
@@ -415,7 +421,7 @@ function getWebpackResolver(
       // 4. Filesystem imports relative to an `includePaths` path.
       // 5. Filesystem imports relative to a `SASS_PATH` path.
       //
-      // Because `sass`/`node-sass` run custom importers before `3`, `4` and `5` points, we need to emulate this behavior to avoid wrong resolution.
+      // `sass` run custom importers before `3`, `4` and `5` points, we need to emulate this behavior to avoid wrong resolution.
       const sassPossibleRequests = getPossibleRequests(request);
 
       // `node-sass` calls our importer before `1. Filesystem imports relative to the base file.`, so we need emulate this too
@@ -439,11 +445,7 @@ function getWebpackResolver(
       );
     }
 
-    const webpackPossibleRequests = getPossibleRequests(
-      request,
-      true,
-      rootContext
-    );
+    const webpackPossibleRequests = getPossibleRequests(request, true);
 
     resolutionMap = resolutionMap.concat({
       resolve: webpackResolve,
@@ -461,8 +463,7 @@ function getWebpackImporter(loaderContext, implementation, includePaths) {
   const resolve = getWebpackResolver(
     loaderContext.getResolve,
     implementation,
-    includePaths,
-    loaderContext.rootContext
+    includePaths
   );
 
   return (originalUrl, prev, done) => {
@@ -472,6 +473,7 @@ function getWebpackImporter(loaderContext, implementation, includePaths) {
         // Although we're also using stats.includedFiles, this might come in handy when an error occurs.
         // In this case, we don't get stats.includedFiles from node-sass/sass.
         loaderContext.addDependency(path.normalize(result));
+
         // By removing the CSS file extension, we trigger node-sass to include the CSS file instead of just linking it.
         done({ file: result.replace(matchCss, "") });
       })

src/utils.js

@@ -2010,28 +2010,82 @@ exports[`sassOptions option should work with the "functions" option (node-sass)
 
 exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (sass): css 1`] = `""`;
 
+exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (sass): css 2`] = `
+".a {
+  color: red;
+}
+
+.b {
+  color: blue;
+}"
+`;
+
 exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (sass): errors 1`] = `Array []`;
 
+exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (sass): errors 2`] = `Array []`;
+
 exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (sass): warnings 1`] = `Array []`;
 
+exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (sass): warnings 2`] = `Array []`;
+
 exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (scss): css 1`] = `""`;
 
+exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (scss): css 2`] = `
+".a {
+  color: red;
+}
+
+.b {
+  color: blue;
+}"
+`;
+
 exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (scss): errors 1`] = `Array []`;
 
+exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (scss): errors 2`] = `Array []`;
+
 exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (scss): warnings 1`] = `Array []`;
 
+exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (scss): warnings 2`] = `Array []`;
+
 exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (sass): css 1`] = `""`;
 
+exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (sass): css 2`] = `
+".a {
+  color: red; }
+
+.b {
+  color: blue; }
+"
+`;
+
 exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (sass): errors 1`] = `Array []`;
 
+exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (sass): errors 2`] = `Array []`;
+
 exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (sass): warnings 1`] = `Array []`;
 
+exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (sass): warnings 2`] = `Array []`;
+
 exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (scss): css 1`] = `""`;
 
+exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (scss): css 2`] = `
+".a {
+  color: red; }
+
+.b {
+  color: blue; }
+"
+`;
+
 exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (scss): errors 1`] = `Array []`;
 
+exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (scss): errors 2`] = `Array []`;
+
 exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (scss): warnings 1`] = `Array []`;
 
+exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (scss): warnings 2`] = `Array []`;
+
 exports[`sassOptions option should work with the "importer" as a single function option (dart-sass) (sass): css 1`] = `""`;
 
 exports[`sassOptions option should work with the "importer" as a single function option (dart-sass) (sass): css 2`] = `

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

@@ -0,0 +1,2 @@
+.a
+  color: red

test/sass/glob-dir/a.sass

@@ -0,0 +1,2 @@
+.b
+  color: blue

test/sass/glob-dir/b.sass

@@ -0,0 +1 @@
+@import "glob-dir/*"

test/sass/glob-importer.sass

@@ -1,5 +1,6 @@
 import path from "path";
 
+import globImporter from "node-sass-glob-importer";
 import semver from "semver";
 import nodeSass from "node-sass";
 import dartSass from "sass";
@@ -233,6 +234,25 @@ describe("sassOptions option", () => {
         expect(getErrors(stats)).toMatchSnapshot("errors");
       });
 
+      it(`should work with the "importer" as a array of functions option (${implementationName}) (${syntax})`, async () => {
+        const testId = getTestId("glob-importer", syntax);
+        const options = {
+          implementation: getImplementationByName(implementationName),
+          sassOptions: {
+            importer: [globImporter()],
+          },
+        };
+        const compiler = getCompiler(testId, { loader: { options } });
+        const stats = await compile(compiler);
+        const codeFromBundle = getCodeFromBundle(stats, compiler);
+        const codeFromSass = getCodeFromSass(testId, options);
+
+        expect(codeFromBundle.css).toBe(codeFromSass.css);
+        expect(codeFromBundle.css).toMatchSnapshot("css");
+        expect(getWarnings(stats)).toMatchSnapshot("warnings");
+        expect(getErrors(stats)).toMatchSnapshot("errors");
+      });
+
       it(`should work with the "includePaths" option (${implementationName}) (${syntax})`, async () => {
         const testId = getTestId("import-include-paths", syntax);
         const options = {

test/sassOptions-option.test.js

@@ -0,0 +1,3 @@
+.a {
+  color: red;
+}

test/scss/glob-dir/a.scss

@@ -0,0 +1,3 @@
+.b {
+  color: blue;
+}

test/scss/glob-dir/b.scss

@@ -0,0 +1 @@
+@import "glob-dir/*";