fix: resolution logic (#839)

Author: evilebottnawi

src/getPossibleRequests.js

@@ -18,13 +18,14 @@ const matchModuleImport = /^~([^/]+|[^/]+\/|@[^/]+[/][^/]+|@[^/]+\/?|@[^/]+[/][^
  * to enable straight-forward webpack.config aliases.
  *
  * @param {string} url
+ * @param {boolean} forWebpackResolver
  * @returns {Array<string>}
  */
-export default function getPossibleRequests(url) {
+export default function getPossibleRequests(url, forWebpackResolver = false) {
   const request = utils.urlToRequest(url);
 
   // In case there is module request, send this to webpack resolver
-  if (matchModuleImport.test(url)) {
+  if (forWebpackResolver && matchModuleImport.test(url)) {
     return [request, url];
   }
 
@@ -51,23 +52,29 @@ export default function getPossibleRequests(url) {
   //
   // 1. Try to resolve `_` file.
   // 2. Try to resolve file without `_`.
-  // 3. Send a original url to webpack resolver, maybe it is alias.
-  if (['.scss', '.sass'].includes(ext)) {
-    return [`${dirname}/_${basename}`, `${dirname}/${basename}`, url];
+  // 3. Send a original url to webpack resolver, maybe it is alias for webpack resolving.
+  if (['.scss', '.sass', '.css'].includes(ext)) {
+    return [`${dirname}/_${basename}`, `${dirname}/${basename}`].concat(
+      forWebpackResolver ? [url] : []
+    );
   }
 
   // In case there is no file extension
   //
-  // 1. Try to resolve files starts with `_` and normal with order `sass`, `scss` and `css`
-  // 2. Send a original url to webpack resolver, maybe it is alias.
+  // 1. Try to resolve files starts with `_` and normal with order `sass`, `scss` and `css`.
+  // 2. Send a original url to webpack resolver, maybe it is alias for webpack resolving.
   return [
     `${dirname}/_${basename}.sass`,
     `${dirname}/${basename}.sass`,
     `${dirname}/_${basename}.scss`,
     `${dirname}/${basename}.scss`,
     `${dirname}/_${basename}.css`,
     `${dirname}/${basename}.css`,
-    request,
-    url,
-  ];
+    `${dirname}/${basename}/_index.sass`,
+    `${dirname}/${basename}/index.sass`,
+    `${dirname}/${basename}/_index.scss`,
+    `${dirname}/${basename}/index.scss`,
+    `${dirname}/${basename}/_index.css`,
+    `${dirname}/${basename}/index.css`,
+  ].concat(forWebpackResolver ? [request, url] : []);
 }

src/index.js

@@ -36,13 +36,10 @@ function loader(content) {
     const resolve = this.getResolve({
       mainFields: ['sass', 'style', 'main', '...'],
       mainFiles: ['_index', 'index', '...'],
-      extensions: ['.scss', '.sass', '.css'],
+      extensions: ['.sass', '.scss', '.css'],
     });
 
-    const includePaths =
-      options.sassOptions && options.sassOptions.includePaths
-        ? options.sassOptions.includePaths
-        : [];
+    const { includePaths } = sassOptions;
 
     sassOptions.importer.push(webpackImporter(this, resolve, includePaths));
   }

src/webpackImporter.js

@@ -1,11 +1,3 @@
-/**
- * @name PromisedResolve
- * @type {Function}
- * @param {string} dir
- * @param {string} request
- * @returns Promise
- */
-
 /**
  * @name Importer
  * @type {Function}
@@ -69,11 +61,28 @@ function webpackImporter(loaderContext, resolve, includePaths) {
   }
 
   return (url, prev, done) => {
-    const possibleRequests = getPossibleRequests(url);
+    // The order of import precedence is as follows:
+    //
+    // 1. Filesystem imports relative to the base file.
+    // 2. Custom importer imports.
+    // 3. Filesystem imports relative to the working directory.
+    // 4. Filesystem imports relative to an `includePaths` path.
+    // 5. Filesystem imports relative to a `SASS_PATH` path.
+    //
+    // Because `sass`/`node-sass` run custom importers after `3`, `4` and `5` points, we need to emulate this behavior to avoid wrong resolution.
+    const sassPossibleRequests = getPossibleRequests(url);
+    const webpackPossibleRequests = getPossibleRequests(url, true);
     const resolutionMap = []
-      .concat(includePaths)
-      .concat(dirContextFrom(prev))
-      .map((context) => ({ context, possibleRequests }));
+      .concat(
+        includePaths.map((context) => ({
+          context,
+          possibleRequests: sassPossibleRequests,
+        }))
+      )
+      .concat({
+        context: dirContextFrom(prev),
+        possibleRequests: webpackPossibleRequests,
+      });
 
     startResolving(resolutionMap)
       // Catch all resolving errors, return the original file and pass responsibility back to other custom importers

test/__snapshots__/loader.test.js.snap

@@ -224,6 +224,46 @@ SassError: expected \\"{\\".",
 
 exports[`loader should output an understandable error with a problem in "@use" (dart-sass) (scss): warnings 1`] = `Array []`;
 
+exports[`loader should respect resolving directory with the "index" file from "process.cwd()"  (dart-sass) (sass): css 1`] = `
+".dir-with-underscore-index {
+  color: red;
+}"
+`;
+
+exports[`loader should respect resolving directory with the "index" file from "process.cwd()"  (dart-sass) (sass): errors 1`] = `Array []`;
+
+exports[`loader should respect resolving directory with the "index" file from "process.cwd()"  (dart-sass) (sass): warnings 1`] = `Array []`;
+
+exports[`loader should respect resolving directory with the "index" file from "process.cwd()"  (dart-sass) (scss): css 1`] = `
+".dir-with-underscore-index {
+  color: red;
+}"
+`;
+
+exports[`loader should respect resolving directory with the "index" file from "process.cwd()"  (dart-sass) (scss): errors 1`] = `Array []`;
+
+exports[`loader should respect resolving directory with the "index" file from "process.cwd()"  (dart-sass) (scss): warnings 1`] = `Array []`;
+
+exports[`loader should respect resolving directory with the "index" file from "process.cwd()"  (node-sass) (sass): css 1`] = `
+".dir-with-underscore-index {
+  color: red; }
+"
+`;
+
+exports[`loader should respect resolving directory with the "index" file from "process.cwd()"  (node-sass) (sass): errors 1`] = `Array []`;
+
+exports[`loader should respect resolving directory with the "index" file from "process.cwd()"  (node-sass) (sass): warnings 1`] = `Array []`;
+
+exports[`loader should respect resolving directory with the "index" file from "process.cwd()"  (node-sass) (scss): css 1`] = `
+".dir-with-underscore-index {
+  color: red; }
+"
+`;
+
+exports[`loader should respect resolving directory with the "index" file from "process.cwd()"  (node-sass) (scss): errors 1`] = `Array []`;
+
+exports[`loader should respect resolving directory with the "index" file from "process.cwd()"  (node-sass) (scss): warnings 1`] = `Array []`;
+
 exports[`loader should respect resolving from "process.cwd()" (dart-sass) (sass): css 1`] = `
 "body {
   font: 100% Helvetica, sans-serif;

test/helpers/getCodeFromSass.js

@@ -259,6 +259,18 @@ function getCodeFromSass(testId, options) {
     if (/\.css$/i.test(url) === false) {
       // Polyfill for node-sass implementation
       if (isNodeSassImplementation) {
+        if (url === 'test/scss/dir-with-underscore-index') {
+          return {
+            file: pathToSCSSIndexAlias,
+          };
+        }
+
+        if (url === 'test/sass/dir-with-underscore-index') {
+          return {
+            file: pathToSassIndexAlias,
+          };
+        }
+
         if (url === '~sass-package-with-index') {
           return {
             file: pathToSassPackageWithIndexFile,

test/loader.test.js

@@ -656,8 +656,6 @@ describe('loader', () => {
       });
 
       it(`should respect resolving from the "SASS_PATH" environment variable (${implementationName}) (${syntax})`, async () => {
-        const OLD_SASS_PATH = process.env.SASS_PATH;
-
         process.env.SASS_PATH =
           process.platform === 'win32'
             ? `${path.resolve('test', syntax, 'sass_path')};${path.resolve(
@@ -685,7 +683,7 @@ describe('loader', () => {
         expect(getWarnings(stats)).toMatchSnapshot('warnings');
         expect(getErrors(stats)).toMatchSnapshot('errors');
 
-        process.env.SASS_PATH = OLD_SASS_PATH;
+        delete process.env.SASS_PATH;
       });
 
       it(`should respect resolving from "process.cwd()" (${implementationName}) (${syntax})`, async () => {
@@ -704,6 +702,25 @@ describe('loader', () => {
         expect(getErrors(stats)).toMatchSnapshot('errors');
       });
 
+      it(`should respect resolving directory with the "index" file from "process.cwd()"  (${implementationName}) (${syntax})`, async () => {
+        const testId = getTestId(
+          'process-cwd-with-index-file-inside-directory',
+          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/sass/process-cwd-with-index-file-inside-directory.sass

@@ -0,0 +1 @@
+@import 'test/sass/dir-with-underscore-index'

test/scss/process-cwd-with-index-file-inside-directory.scss

@@ -0,0 +1 @@
+@import 'test/scss/dir-with-underscore-index';