Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: problem with resolving and the includePaths option #913

Merged
merged 4 commits into from Jan 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2,001 changes: 781 additions & 1,220 deletions package-lock.json

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions package.json
Expand Up @@ -77,9 +77,9 @@
"css-loader": "^5.0.1",
"del": "^6.0.0",
"del-cli": "^3.0.1",
"enhanced-resolve": "^5.3.1",
"enhanced-resolve": "^5.5.0",
"eslint": "^7.13.0",
"eslint-config-prettier": "^6.15.0",
"eslint-config-prettier": "^7.1.0",
"eslint-plugin-import": "^2.22.1",
"fibers": "^5.0.0",
"file-loader": "^6.2.0",
Expand All @@ -95,7 +95,7 @@
"sass": "^1.29.0",
"standard-version": "^9.0.0",
"style-loader": "^2.0.0",
"webpack": "^5.4.0"
"webpack": "^5.12.2"
},
"keywords": [
"sass",
Expand Down
7 changes: 6 additions & 1 deletion src/index.js
Expand Up @@ -81,7 +81,12 @@ async function loader(content) {
}

result.stats.includedFiles.forEach((includedFile) => {
this.addDependency(path.normalize(includedFile));
const normalizedIncludedFile = path.normalize(includedFile);

// Custom `importer` can return only `contents` so includedFile will be relative
if (path.isAbsolute(normalizedIncludedFile)) {
this.addDependency(normalizedIncludedFile);
}
});

callback(null, result.css.toString(), map);
Expand Down
34 changes: 22 additions & 12 deletions src/utils.js
Expand Up @@ -94,13 +94,14 @@ function isProductionLikeMode(loaderContext) {
}

function proxyCustomImporters(importers, loaderContext) {
return [].concat(importers).map((importer) => {
return function proxyImporter(...args) {
this.webpackLoaderContext = loaderContext;
return [].concat(importers).map(
(importer) =>
function proxyImporter(...args) {
this.webpackLoaderContext = loaderContext;

return importer.apply(this, args);
};
});
return importer.apply(this, args);
}
);
}

/**
Expand Down Expand Up @@ -205,7 +206,14 @@ async function getSassOptions(

options.includePaths = []
.concat(process.cwd())
.concat(options.includePaths || [])
.concat(
// We use `includePaths` in context for resolver, so it should be always absolute
(options.includePaths || []).map((includePath) =>
path.isAbsolute(includePath)
? includePath
: path.join(process.cwd(), includePath)
)
)
.concat(
process.env.SASS_PATH
? process.env.SASS_PATH.split(process.platform === "win32" ? ";" : ":")
Expand Down Expand Up @@ -430,11 +438,13 @@ function getWebpackResolver(

resolutionMap = resolutionMap.concat(
// eslint-disable-next-line no-shadow
includePaths.map((context) => ({
resolve: sassResolve,
context,
possibleRequests: sassPossibleRequests,
}))
includePaths.map((context) => {
return {
resolve: sassResolve,
context,
possibleRequests: sassPossibleRequests,
};
})
);
}

Expand Down
5 changes: 2 additions & 3 deletions test/helpers/compiler.js
@@ -1,5 +1,5 @@
export default (compiler) => {
return new Promise((resolve, reject) => {
export default (compiler) =>
new Promise((resolve, reject) => {
compiler.run((error, stats) => {
if (error) {
return reject(error);
Expand All @@ -8,4 +8,3 @@ export default (compiler) => {
return resolve(stats);
});
});
};
4 changes: 1 addition & 3 deletions test/helpers/getErrors.js
@@ -1,5 +1,3 @@
import normalizeErrors from "./normalizeErrors";

export default (stats) => {
return normalizeErrors(stats.compilation.errors.sort());
};
export default (stats) => normalizeErrors(stats.compilation.errors.sort());
4 changes: 1 addition & 3 deletions test/helpers/getWarnings.js
@@ -1,5 +1,3 @@
import normalizeErrors from "./normalizeErrors";

export default (stats) => {
return normalizeErrors(stats.compilation.warnings.sort());
};
export default (stats) => normalizeErrors(stats.compilation.warnings.sort());
5 changes: 2 additions & 3 deletions test/helpers/normalizeErrors.js
Expand Up @@ -12,8 +12,7 @@ function removeCWD(str) {
return str.replace(new RegExp(cwd, "g"), "");
}

export default (errors) => {
return errors.map((error) =>
export default (errors) =>
errors.map((error) =>
removeCWD(error.toString().split("\n").slice(0, 2).join("\n"))
);
};
7 changes: 3 additions & 4 deletions test/loader.test.js
Expand Up @@ -903,8 +903,8 @@ describe("loader", () => {
const testId = getTestId("import-absolute-path", syntax);
const options = {
implementation: getImplementationByName(implementationName),
additionalData: (content) => {
return content
additionalData: (content) =>
content
.replace(
/\/scss\/language.scss/g,
`file:///${path
Expand All @@ -916,8 +916,7 @@ describe("loader", () => {
`file:///${path
.resolve(__dirname, "sass/language.sass")
.replace(/\\/g, "/")}`
);
},
),
};
const compiler = getCompiler(testId, { loader: { options } });
const stats = await compile(compiler);
Expand Down
8 changes: 7 additions & 1 deletion test/validate-options.test.js
Expand Up @@ -20,7 +20,13 @@ describe("validate options", () => {
failure: [true, "string"],
},
sassOptions: {
success: [{}, { indentWidth: 6 }, () => ({ indentWidth: 6 })],
success: [
{},
{ indentWidth: 6 },
() => {
return { indentWidth: 6 };
},
],
failure: [true, "string"],
},
additionalData: {
Expand Down