fix: resolution algorithm for node-sass (#866)

Author: evilebottnawi

src/index.js

@@ -37,7 +37,9 @@ function loader(content) {
   if (shouldUseWebpackImporter) {
     const { includePaths } = sassOptions;
 
-    sassOptions.importer.push(getWebpackImporter(this, includePaths));
+    sassOptions.importer.push(
+      getWebpackImporter(this, implementation, includePaths)
+    );
   }
 
   const callback = this.async();

src/utils.js

@@ -267,7 +267,7 @@ const isSpecialModuleImport = /^~[^/]+$/;
 // `[drive_letter]:\` + `\\[server]\[sharename]\`
 const isNativeWin32Path = /^[a-zA-Z]:[/\\]|^\\\\/i;
 
-function getWebpackImporter(loaderContext, includePaths) {
+function getWebpackImporter(loaderContext, implementation, includePaths) {
   function startResolving(resolutionMap) {
     if (resolutionMap.length === 0) {
       return Promise.reject();
@@ -359,6 +359,16 @@ function getWebpackImporter(loaderContext, includePaths) {
       //
       // Because `sass`/`node-sass` run custom importers before `3`, `4` and `5` points, we need to emulate this behavior to avoid wrong resolution.
       const sassPossibleRequests = getPossibleRequests(loaderContext, request);
+      const isDartSass = implementation.info.includes('dart-sass');
+
+      // `node-sass` calls our importer before `1. Filesystem imports relative to the base file.`, so we need emulate this too
+      if (!isDartSass) {
+        resolutionMap = resolutionMap.concat({
+          resolve: sassResolve,
+          context: path.dirname(prev),
+          possibleRequests: sassPossibleRequests,
+        });
+      }
 
       resolutionMap = resolutionMap.concat(
         includePaths.map((context) => ({

test/__snapshots__/loader.test.js.snap

@@ -346,6 +346,60 @@ exports[`loader should prefer "mainFiles" with extension over without (node-sass
 
 exports[`loader should prefer "mainFiles" with extension over without (node-sass) (scss): warnings 1`] = `Array []`;
 
+exports[`loader should prefer relative import (dart-sass) (sass): css 1`] = `
+".class {
+  color: blue;
+}
+
+a {
+  color: red;
+}"
+`;
+
+exports[`loader should prefer relative import (dart-sass) (sass): errors 1`] = `Array []`;
+
+exports[`loader should prefer relative import (dart-sass) (sass): warnings 1`] = `Array []`;
+
+exports[`loader should prefer relative import (dart-sass) (scss): css 1`] = `
+".class {
+  color: blue;
+}
+
+a {
+  color: red;
+}"
+`;
+
+exports[`loader should prefer relative import (dart-sass) (scss): errors 1`] = `Array []`;
+
+exports[`loader should prefer relative import (dart-sass) (scss): warnings 1`] = `Array []`;
+
+exports[`loader should prefer relative import (node-sass) (sass): css 1`] = `
+".class {
+  color: blue; }
+
+a {
+  color: red; }
+"
+`;
+
+exports[`loader should prefer relative import (node-sass) (sass): errors 1`] = `Array []`;
+
+exports[`loader should prefer relative import (node-sass) (sass): warnings 1`] = `Array []`;
+
+exports[`loader should prefer relative import (node-sass) (scss): css 1`] = `
+".class {
+  color: blue; }
+
+a {
+  color: red; }
+"
+`;
+
+exports[`loader should prefer relative import (node-sass) (scss): errors 1`] = `Array []`;
+
+exports[`loader should prefer relative import (node-sass) (scss): warnings 1`] = `Array []`;
+
 exports[`loader should resolve absolute paths (dart-sass) (sass): css 1`] = `
 "@charset \\"UTF-8\\";
 body {

test/helpers/getCodeFromSass.js

@@ -267,6 +267,10 @@ function getCodeFromSass(testId, options) {
   const pathToLanguage = isSass
     ? path.resolve(testFolder, 'sass/language.sass')
     : path.resolve(testFolder, 'scss/language.scss');
+  const pathToPackageWithSameImport = path.resolve(
+    testFolder,
+    'node_modules/package-with-same-import/style.scss'
+  );
 
   // Pseudo importer for tests
   function testImporter(url) {
@@ -733,6 +737,10 @@ function getCodeFromSass(testId, options) {
         .replace(/^\/scss\/language.scss/, pathToLanguage)
         .replace(/^file:\/\/\/scss\/language.scss/, pathToLanguage)
         .replace(/^file:\/\/\/sass\/language.sass/, pathToLanguage)
+        .replace(
+          /^package-with-same-import\/style/,
+          pathToPackageWithSameImport
+        )
         .replace(/^~/, testNodeModules);
     }
 

test/loader.test.js

@@ -884,6 +884,22 @@ describe('loader', () => {
         expect(getErrors(stats)).toMatchSnapshot('errors');
       });
 
+      it(`should prefer relative import (${implementationName}) (${syntax})`, async () => {
+        const testId = getTestId('package-with-same-import', syntax);
+        const options = {
+          implementation: getImplementationByName(implementationName),
+        };
+        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');
+      });
+
       if (implementation === dartSass) {
         it(`should output an understandable error with a problem in "@use" (${implementationName}) (${syntax})`, async () => {
           const testId = getTestId('error-use', syntax);

test/node_modules/package-with-same-import/package.json

@@ -0,0 +1 @@
+@import "package-with-same-import/style"

test/node_modules/package-with-same-import/style.scss

@@ -0,0 +1,4 @@
+@import "package-with-package"
+
+a
+  color: red