From 4584c9054befbc56661e2781a55df96fb9f94673 Mon Sep 17 00:00:00 2001 From: Evilebot Tnawi Date: Tue, 7 Jul 2020 18:28:09 +0300 Subject: [PATCH] fix: resolution algorithm for `node-sass` (#866) --- src/index.js | 4 +- src/utils.js | 12 ++++- test/__snapshots__/loader.test.js.snap | 54 +++++++++++++++++++ test/helpers/getCodeFromSass.js | 8 +++ test/loader.test.js | 16 ++++++ .../package-with-same-import/package.json | 11 ++++ .../package-with-same-import/style.scss | 1 + .../test/scss/package-with-package.scss | 3 ++ test/sass/package-with-package.sass | 1 + test/sass/package-with-same-import.sass | 4 ++ test/scss/package-with-package.scss | 1 + test/scss/package-with-same-import.scss | 5 ++ 12 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 test/node_modules/package-with-same-import/package.json create mode 100644 test/node_modules/package-with-same-import/style.scss create mode 100644 test/node_modules/package-with-same-import/test/scss/package-with-package.scss create mode 100644 test/sass/package-with-package.sass create mode 100644 test/sass/package-with-same-import.sass create mode 100644 test/scss/package-with-package.scss create mode 100644 test/scss/package-with-same-import.scss diff --git a/src/index.js b/src/index.js index ba597bb4..17435c4b 100644 --- a/src/index.js +++ b/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(); diff --git a/src/utils.js b/src/utils.js index 658090c8..2cf720d8 100644 --- a/src/utils.js +++ b/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) => ({ diff --git a/test/__snapshots__/loader.test.js.snap b/test/__snapshots__/loader.test.js.snap index 1dd10cf4..222db7d9 100644 --- a/test/__snapshots__/loader.test.js.snap +++ b/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 { diff --git a/test/helpers/getCodeFromSass.js b/test/helpers/getCodeFromSass.js index 0b7cc01f..e538bff9 100644 --- a/test/helpers/getCodeFromSass.js +++ b/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); } diff --git a/test/loader.test.js b/test/loader.test.js index 6ac7df8f..859ba7bf 100644 --- a/test/loader.test.js +++ b/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); diff --git a/test/node_modules/package-with-same-import/package.json b/test/node_modules/package-with-same-import/package.json new file mode 100644 index 00000000..eabfa2d3 --- /dev/null +++ b/test/node_modules/package-with-same-import/package.json @@ -0,0 +1,11 @@ +{ + "name": "package-with-same-import", + "version": "1.0.0", + "description": "test", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "author": "", + "license": "MIT" +} diff --git a/test/node_modules/package-with-same-import/style.scss b/test/node_modules/package-with-same-import/style.scss new file mode 100644 index 00000000..3f962d16 --- /dev/null +++ b/test/node_modules/package-with-same-import/style.scss @@ -0,0 +1 @@ +@import "test/scss/package-with-package"; diff --git a/test/node_modules/package-with-same-import/test/scss/package-with-package.scss b/test/node_modules/package-with-same-import/test/scss/package-with-package.scss new file mode 100644 index 00000000..e229134e --- /dev/null +++ b/test/node_modules/package-with-same-import/test/scss/package-with-package.scss @@ -0,0 +1,3 @@ +.class { + color: blue; +} diff --git a/test/sass/package-with-package.sass b/test/sass/package-with-package.sass new file mode 100644 index 00000000..69bce7ec --- /dev/null +++ b/test/sass/package-with-package.sass @@ -0,0 +1 @@ +@import "package-with-same-import/style" diff --git a/test/sass/package-with-same-import.sass b/test/sass/package-with-same-import.sass new file mode 100644 index 00000000..a9dff7d1 --- /dev/null +++ b/test/sass/package-with-same-import.sass @@ -0,0 +1,4 @@ +@import "package-with-package" + +a + color: red diff --git a/test/scss/package-with-package.scss b/test/scss/package-with-package.scss new file mode 100644 index 00000000..2b2c4b49 --- /dev/null +++ b/test/scss/package-with-package.scss @@ -0,0 +1 @@ +@import "package-with-same-import/style"; diff --git a/test/scss/package-with-same-import.scss b/test/scss/package-with-same-import.scss new file mode 100644 index 00000000..67cfdc7e --- /dev/null +++ b/test/scss/package-with-same-import.scss @@ -0,0 +1,5 @@ +@import "package-with-package"; + +a { + color: red; +}