Skip to content

Commit

Permalink
fix: compatibility with custom importers for node-sass (#927)
Browse files Browse the repository at this point in the history
  • Loading branch information
alexander-akait authored Feb 8, 2021
1 parent 0db6fb6 commit af5a072
Show file tree
Hide file tree
Showing 11 changed files with 532 additions and 69 deletions.
476 changes: 425 additions & 51 deletions package-lock.json

Large diffs are not rendered by default.

9 changes: 5 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@
"dist"
],
"peerDependencies": {
"webpack": "^5.0.0",
"fibers": ">= 3.1.0",
"node-sass": "^4.0.0 || ^5.0.0",
"sass": "^1.3.0",
"fibers": ">= 3.1.0"
"webpack": "^5.0.0"
},
"peerDependenciesMeta": {
"node-sass": {
Expand Down Expand Up @@ -83,17 +83,18 @@
"foundation-sites": "^6.6.3",
"husky": "^4.3.6",
"jest": "^26.6.3",
"lint-staged": "^10.5.3",
"lint-staged": "^10.5.4",
"material-components-web": "^8.0.0",
"memfs": "^3.2.0",
"node-sass": "^5.0.0",
"node-sass-glob-importer": "^5.3.2",
"npm-run-all": "^4.1.5",
"prettier": "^2.2.1",
"sass": "^1.32.0",
"semver": "^7.3.4",
"standard-version": "^9.1.0",
"style-loader": "^2.0.0",
"webpack": "^5.20.2"
"webpack": "^5.21.2"
},
"keywords": [
"sass",
Expand Down
30 changes: 16 additions & 14 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,15 +207,16 @@ async function getSassOptions(
return options;
}

const MODULE_REQUEST_REGEX = /^[^?]*~/;

// Examples:
// - ~package
// - ~package/
// - ~@org
// - ~@org/
// - ~@org/package
// - ~@org/package/
const isModuleImport = /^~([^/]+|[^/]+\/|@[^/]+[/][^/]+|@[^/]+\/?|@[^/]+[/][^/]+\/)$/;
const MODULE_REQUEST_REGEX = /^[^?]*~/;
const IS_MODULE_IMPORT = /^~([^/]+|[^/]+\/|@[^/]+[/][^/]+|@[^/]+\/?|@[^/]+[/][^/]+\/)$/;

/**
* When `sass`/`node-sass` tries to resolve an import, it uses a special algorithm.
Expand Down Expand Up @@ -244,7 +245,7 @@ function getPossibleRequests(
request = request.replace(MODULE_REQUEST_REGEX, "");
}

if (isModuleImport.test(url)) {
if (IS_MODULE_IMPORT.test(url)) {
request = request[request.length - 1] === "/" ? request : `${request}/`;

return [...new Set([request, url])];
Expand Down Expand Up @@ -310,15 +311,13 @@ const IS_NATIVE_WIN32_PATH = /^[a-z]:[/\\]|^\\\\/i;
* @param {Object} implementation - The imported Sass implementation, both
* `sass` (Dart Sass) and `node-sass` are supported.
* @param {string[]} [includePaths] - The list of include paths passed to Sass.
* @param {boolean} [rootContext] - The configured Webpack root context.
*
* @throws If a compatible Sass implementation cannot be found.
*/
function getWebpackResolver(
resolverFactory,
implementation,
includePaths = [],
rootContext = false
includePaths = []
) {
async function startResolving(resolutionMap) {
if (resolutionMap.length === 0) {
Expand Down Expand Up @@ -380,6 +379,13 @@ function getWebpackResolver(
);

return (context, request) => {
// See https://github.com/webpack/webpack/issues/12340
// Because `node-sass` calls our importer before `1. Filesystem imports relative to the base file.`
// custom importer may not return `{ file: '/path/to/name.ext' }` and therefore our `context` will be relative
if (!isDartSass && !path.isAbsolute(context)) {
return Promise.reject();
}

const originalRequest = request;
const isFileScheme = originalRequest.slice(0, 5).toLowerCase() === "file:";

Expand Down Expand Up @@ -415,7 +421,7 @@ function getWebpackResolver(
// 4. Filesystem imports relative to an `includePaths` path.
// 5. Filesystem imports relative to a `SASS_PATH` path.
//
// Because `sass`/`node-sass` run custom importers before `3`, `4` and `5` points, we need to emulate this behavior to avoid wrong resolution.
// `sass` run custom importers before `3`, `4` and `5` points, we need to emulate this behavior to avoid wrong resolution.
const sassPossibleRequests = getPossibleRequests(request);

// `node-sass` calls our importer before `1. Filesystem imports relative to the base file.`, so we need emulate this too
Expand All @@ -439,11 +445,7 @@ function getWebpackResolver(
);
}

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

resolutionMap = resolutionMap.concat({
resolve: webpackResolve,
Expand All @@ -461,8 +463,7 @@ function getWebpackImporter(loaderContext, implementation, includePaths) {
const resolve = getWebpackResolver(
loaderContext.getResolve,
implementation,
includePaths,
loaderContext.rootContext
includePaths
);

return (originalUrl, prev, done) => {
Expand All @@ -472,6 +473,7 @@ function getWebpackImporter(loaderContext, implementation, includePaths) {
// Although we're also using stats.includedFiles, this might come in handy when an error occurs.
// In this case, we don't get stats.includedFiles from node-sass/sass.
loaderContext.addDependency(path.normalize(result));

// By removing the CSS file extension, we trigger node-sass to include the CSS file instead of just linking it.
done({ file: result.replace(matchCss, "") });
})
Expand Down
54 changes: 54 additions & 0 deletions test/__snapshots__/sassOptions-option.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2010,28 +2010,82 @@ exports[`sassOptions option should work with the "functions" option (node-sass)

exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (sass): css 1`] = `""`;

exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (sass): css 2`] = `
".a {
color: red;
}
.b {
color: blue;
}"
`;

exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (sass): errors 1`] = `Array []`;

exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (sass): errors 2`] = `Array []`;

exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (sass): warnings 1`] = `Array []`;

exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (sass): warnings 2`] = `Array []`;

exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (scss): css 1`] = `""`;

exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (scss): css 2`] = `
".a {
color: red;
}
.b {
color: blue;
}"
`;

exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (scss): errors 1`] = `Array []`;

exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (scss): errors 2`] = `Array []`;

exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (scss): warnings 1`] = `Array []`;

exports[`sassOptions option should work with the "importer" as a array of functions option (dart-sass) (scss): warnings 2`] = `Array []`;

exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (sass): css 1`] = `""`;

exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (sass): css 2`] = `
".a {
color: red; }
.b {
color: blue; }
"
`;

exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (sass): errors 1`] = `Array []`;

exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (sass): errors 2`] = `Array []`;

exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (sass): warnings 1`] = `Array []`;

exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (sass): warnings 2`] = `Array []`;

exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (scss): css 1`] = `""`;

exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (scss): css 2`] = `
".a {
color: red; }
.b {
color: blue; }
"
`;

exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (scss): errors 1`] = `Array []`;

exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (scss): errors 2`] = `Array []`;

exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (scss): warnings 1`] = `Array []`;

exports[`sassOptions option should work with the "importer" as a array of functions option (node-sass) (scss): warnings 2`] = `Array []`;

exports[`sassOptions option should work with the "importer" as a single function option (dart-sass) (sass): css 1`] = `""`;

exports[`sassOptions option should work with the "importer" as a single function option (dart-sass) (sass): css 2`] = `
Expand Down
2 changes: 2 additions & 0 deletions test/sass/glob-dir/a.sass
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
.a
color: red
2 changes: 2 additions & 0 deletions test/sass/glob-dir/b.sass
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
.b
color: blue
1 change: 1 addition & 0 deletions test/sass/glob-importer.sass
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import "glob-dir/*"
20 changes: 20 additions & 0 deletions test/sassOptions-option.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import path from "path";

import globImporter from "node-sass-glob-importer";
import semver from "semver";
import nodeSass from "node-sass";
import dartSass from "sass";
Expand Down Expand Up @@ -233,6 +234,25 @@ describe("sassOptions option", () => {
expect(getErrors(stats)).toMatchSnapshot("errors");
});

it(`should work with the "importer" as a array of functions option (${implementationName}) (${syntax})`, async () => {
const testId = getTestId("glob-importer", syntax);
const options = {
implementation: getImplementationByName(implementationName),
sassOptions: {
importer: [globImporter()],
},
};
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 with the "includePaths" option (${implementationName}) (${syntax})`, async () => {
const testId = getTestId("import-include-paths", syntax);
const options = {
Expand Down
3 changes: 3 additions & 0 deletions test/scss/glob-dir/a.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.a {
color: red;
}
3 changes: 3 additions & 0 deletions test/scss/glob-dir/b.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.b {
color: blue;
}
1 change: 1 addition & 0 deletions test/scss/glob-importer.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import "glob-dir/*";

0 comments on commit af5a072

Please sign in to comment.