From a2488364cacd68fbd2425f80790afd7613e2f866 Mon Sep 17 00:00:00 2001 From: Alexander Akait <4567934+alexander-akait@users.noreply.github.com> Date: Tue, 1 Jun 2021 17:43:39 +0300 Subject: [PATCH] refactor: logic of sass resolution --- src/utils.js | 60 ++++++++++++++++++++------ test/__snapshots__/loader.test.js.snap | 20 +++++++++ test/loader.test.js | 16 +++++++ test/sass/dir-with-css/_name.sass | 2 + test/sass/dir-with-css/name.css | 3 ++ test/sass/use-dir-with-css.sass | 1 + test/scss/dir-with-css/_name.scss | 3 ++ test/scss/dir-with-css/name.css | 3 ++ test/scss/use-dir-with-css.scss | 1 + 9 files changed, 97 insertions(+), 12 deletions(-) create mode 100644 test/sass/dir-with-css/_name.sass create mode 100644 test/sass/dir-with-css/name.css create mode 100644 test/sass/use-dir-with-css.sass create mode 100644 test/scss/dir-with-css/_name.scss create mode 100644 test/scss/dir-with-css/name.css create mode 100644 test/scss/use-dir-with-css.scss diff --git a/src/utils.js b/src/utils.js index c6bbfb2a..627d9b56 100644 --- a/src/utils.js +++ b/src/utils.js @@ -23,9 +23,7 @@ function getDefaultSassImplementation() { } /** - * @public - * This function is not Webpack-specific and can be used by tools wishing to - * mimic `sass-loader`'s behaviour, so its signature should not be changed. + * This function is not Webpack-specific and can be used by tools wishing to mimic `sass-loader`'s behaviour, so its signature should not be changed. */ function getSassImplementation(loaderContext, implementation) { let resolvedImplementation = implementation; @@ -73,6 +71,10 @@ function getSassImplementation(loaderContext, implementation) { ); } +/** + * @param {any} loaderContext + * @returns {boolean} + */ function isProductionLikeMode(loaderContext) { return loaderContext.mode === "production" || !loaderContext.mode; } @@ -236,13 +238,14 @@ const IS_MODULE_IMPORT = * * @param {string} url * @param {boolean} forWebpackResolver - * @param {string} rootContext + * @param {boolean} fromImport * @returns {Array} */ function getPossibleRequests( // eslint-disable-next-line no-shadow url, - forWebpackResolver = false + forWebpackResolver = false, + fromImport = false ) { let request = url; @@ -261,7 +264,7 @@ function getPossibleRequests( // 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).toLowerCase(); + const extension = path.extname(request).toLowerCase(); // Because @import is also defined in CSS, Sass needs a way of compiling plain CSS @imports without trying to import the files at compile time. // To accomplish this, and to ensure SCSS is as much of a superset of CSS as possible, Sass will compile any @imports with the following characteristics to plain CSS imports: @@ -271,18 +274,31 @@ function getPossibleRequests( // - imports that have media queries. // // The `node-sass` package sends `@import` ending on `.css` to importer, it is bug, so we skip resolve - if (ext === ".css") { + if (extension === ".css") { return []; } const dirname = path.dirname(request); + const normalizedDirname = dirname === "." ? "" : `${dirname}/`; const basename = path.basename(request); + const basenameWithoutExtension = path.basename(request, extension); return [ ...new Set( - [`${dirname}/_${basename}`, request].concat( - forWebpackResolver ? [`${path.dirname(url)}/_${basename}`, url] : [] - ) + [] + .concat( + fromImport + ? [ + `${normalizedDirname}_${basenameWithoutExtension}.import${extension}`, + `${normalizedDirname}${basenameWithoutExtension}.import${extension}`, + ] + : [] + ) + .concat([ + `${normalizedDirname}_${basename}`, + `${normalizedDirname}${basename}`, + ]) + .concat(forWebpackResolver ? [url] : []) ), ]; } @@ -358,6 +374,14 @@ function getWebpackResolver( } const isDartSass = implementation.info.includes("dart-sass"); + // We only have one difference with the built-in sass resolution logic and out resolution logic: + // First, we look at the files starting with `_`, then without `_` (i.e. `_name.sass`, `_name.scss`, `_name.css`, `name.sass`, `name.scss`, `name.css`), + // although `sass` look together by extensions (i.e. `_name.sass`/`name.sass`/`_name.scss`/`name.scss`/`_name.css`/`name.css`). + // It shouldn't be a problem because `sass` throw errors: + // - on having `_name.sass` and `name.sass` (extension can be `sass`, `scss` or `css`) in the same directory + // - on having `_name.sass` and `_name.scss` in the same directory + // + // Also `sass` prefer `sass`/`scss` over `css`. const sassModuleResolve = promiseResolve( resolverFactory({ alias: [], @@ -455,7 +479,11 @@ function getWebpackResolver( // 5. Filesystem imports relative to a `SASS_PATH` path. // // `sass` run custom importers before `3`, `4` and `5` points, we need to emulate this behavior to avoid wrong resolution. - const sassPossibleRequests = getPossibleRequests(request); + const sassPossibleRequests = getPossibleRequests( + request, + false, + fromImport + ); // `node-sass` calls our importer before `1. Filesystem imports relative to the base file.`, so we need emulate this too if (!isDartSass) { @@ -478,7 +506,11 @@ function getWebpackResolver( ); } - const webpackPossibleRequests = getPossibleRequests(request, true); + const webpackPossibleRequests = getPossibleRequests( + request, + true, + fromImport + ); resolutionMap = resolutionMap.concat({ resolve: fromImport ? webpackImportResolve : webpackModuleResolve, @@ -551,6 +583,10 @@ function getRenderFunctionFromSassImplementation(implementation) { const ABSOLUTE_SCHEME = /^[A-Za-z0-9+\-.]+:/; +/** + * @param {string} source + * @returns {"absolute" | "scheme-relative" | "path-absolute" | "path-absolute"} + */ function getURLType(source) { if (source[0] === "/") { if (source[1] === "/") { diff --git a/test/__snapshots__/loader.test.js.snap b/test/__snapshots__/loader.test.js.snap index ef894527..c9e8a577 100644 --- a/test/__snapshots__/loader.test.js.snap +++ b/test/__snapshots__/loader.test.js.snap @@ -404,6 +404,26 @@ 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 "sass)" over CSS (dart-sass) (sass): css 1`] = ` +"a { + color: green; +}" +`; + +exports[`loader should prefer "sass)" over CSS (dart-sass) (sass): errors 1`] = `Array []`; + +exports[`loader should prefer "sass)" over CSS (dart-sass) (sass): warnings 1`] = `Array []`; + +exports[`loader should prefer "scss)" over CSS (dart-sass) (scss): css 1`] = ` +"a { + color: green; +}" +`; + +exports[`loader should prefer "scss)" over CSS (dart-sass) (scss): errors 1`] = `Array []`; + +exports[`loader should prefer "scss)" over CSS (dart-sass) (scss): warnings 1`] = `Array []`; + exports[`loader should prefer relative import (dart-sass) (sass): css 1`] = ` ".class { color: blue; diff --git a/test/loader.test.js b/test/loader.test.js index 91713da2..9e04fec2 100644 --- a/test/loader.test.js +++ b/test/loader.test.js @@ -1643,6 +1643,22 @@ describe("loader", () => { expect(getErrors(stats)).toMatchSnapshot("errors"); }); + it(`should prefer "${syntax})" over CSS (${implementationName}) (${syntax})`, async () => { + const testId = getTestId("use-dir-with-css", 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"); + }); + it(`should work and output deprecation message (${implementationName})`, async () => { const testId = getTestId("deprecation", syntax); const options = { diff --git a/test/sass/dir-with-css/_name.sass b/test/sass/dir-with-css/_name.sass new file mode 100644 index 00000000..8c8e41b6 --- /dev/null +++ b/test/sass/dir-with-css/_name.sass @@ -0,0 +1,2 @@ +a + color: green diff --git a/test/sass/dir-with-css/name.css b/test/sass/dir-with-css/name.css new file mode 100644 index 00000000..23d68a8a --- /dev/null +++ b/test/sass/dir-with-css/name.css @@ -0,0 +1,3 @@ +a { + color: blue; +} \ No newline at end of file diff --git a/test/sass/use-dir-with-css.sass b/test/sass/use-dir-with-css.sass new file mode 100644 index 00000000..1b0285de --- /dev/null +++ b/test/sass/use-dir-with-css.sass @@ -0,0 +1 @@ +@use './dir-with-css/name' diff --git a/test/scss/dir-with-css/_name.scss b/test/scss/dir-with-css/_name.scss new file mode 100644 index 00000000..03597d36 --- /dev/null +++ b/test/scss/dir-with-css/_name.scss @@ -0,0 +1,3 @@ +a { + color: green; +} diff --git a/test/scss/dir-with-css/name.css b/test/scss/dir-with-css/name.css new file mode 100644 index 00000000..23d68a8a --- /dev/null +++ b/test/scss/dir-with-css/name.css @@ -0,0 +1,3 @@ +a { + color: blue; +} \ No newline at end of file diff --git a/test/scss/use-dir-with-css.scss b/test/scss/use-dir-with-css.scss new file mode 100644 index 00000000..5f59a758 --- /dev/null +++ b/test/scss/use-dir-with-css.scss @@ -0,0 +1 @@ +@use './dir-with-css/name'; \ No newline at end of file