Skip to content

Commit

Permalink
refactor: logic of sass resolution
Browse files Browse the repository at this point in the history
  • Loading branch information
alexander-akait committed Jun 1, 2021
1 parent 6641a16 commit a248836
Show file tree
Hide file tree
Showing 9 changed files with 97 additions and 12 deletions.
60 changes: 48 additions & 12 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -73,6 +71,10 @@ function getSassImplementation(loaderContext, implementation) {
);
}

/**
* @param {any} loaderContext
* @returns {boolean}
*/
function isProductionLikeMode(loaderContext) {
return loaderContext.mode === "production" || !loaderContext.mode;
}
Expand Down Expand Up @@ -236,13 +238,14 @@ const IS_MODULE_IMPORT =
*
* @param {string} url
* @param {boolean} forWebpackResolver
* @param {string} rootContext
* @param {boolean} fromImport
* @returns {Array<string>}
*/
function getPossibleRequests(
// eslint-disable-next-line no-shadow
url,
forWebpackResolver = false
forWebpackResolver = false,
fromImport = false
) {
let request = url;

Expand All @@ -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:
Expand All @@ -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] : [])
),
];
}
Expand Down Expand Up @@ -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: [],
Expand Down Expand Up @@ -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) {
Expand All @@ -478,7 +506,11 @@ function getWebpackResolver(
);
}

const webpackPossibleRequests = getPossibleRequests(request, true);
const webpackPossibleRequests = getPossibleRequests(
request,
true,
fromImport
);

resolutionMap = resolutionMap.concat({
resolve: fromImport ? webpackImportResolve : webpackModuleResolve,
Expand Down Expand Up @@ -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] === "/") {
Expand Down
20 changes: 20 additions & 0 deletions test/__snapshots__/loader.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
16 changes: 16 additions & 0 deletions test/loader.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
2 changes: 2 additions & 0 deletions test/sass/dir-with-css/_name.sass
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
a
color: green
3 changes: 3 additions & 0 deletions test/sass/dir-with-css/name.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
a {
color: blue;
}
1 change: 1 addition & 0 deletions test/sass/use-dir-with-css.sass
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@use './dir-with-css/name'
3 changes: 3 additions & 0 deletions test/scss/dir-with-css/_name.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
a {
color: green;
}
3 changes: 3 additions & 0 deletions test/scss/dir-with-css/name.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
a {
color: blue;
}
1 change: 1 addition & 0 deletions test/scss/use-dir-with-css.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@use './dir-with-css/name';

0 comments on commit a248836

Please sign in to comment.