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: resolving .import #906

Merged
merged 3 commits into from
May 31, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 38 additions & 10 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ function getWebpackResolver(
}

const isDartSass = implementation.info.includes("dart-sass");
const sassResolve = promiseResolve(
const sassModuleResolve = promiseResolve(
resolverFactory({
alias: [],
aliasFields: [],
Expand All @@ -373,7 +373,22 @@ function getWebpackResolver(
preferRelative: true,
})
);
const webpackResolve = promiseResolve(
const sassImportResolve = promiseResolve(
resolverFactory({
alias: [],
aliasFields: [],
conditionNames: [],
descriptionFiles: [],
extensions: [".sass", ".scss", ".css"],
exportsFields: [],
mainFields: [],
mainFiles: ["_index.import", "_index", "index.import", "index"],
modules: [],
restrictions: [/\.((sa|sc|c)ss)$/i],
preferRelative: true,
})
);
const webpackModuleResolve = promiseResolve(
resolverFactory({
dependencyType: "sass",
conditionNames: ["sass", "style"],
Expand All @@ -384,8 +399,19 @@ function getWebpackResolver(
preferRelative: true,
})
);
const webpackImportResolve = promiseResolve(
resolverFactory({
dependencyType: "sass",
conditionNames: ["sass", "style"],
mainFields: ["sass", "style", "main", "..."],
mainFiles: ["_index.import", "_index", "index.import", "index", "..."],
extensions: [".sass", ".scss", ".css"],
restrictions: [/\.((sa|sc|c)ss)$/i],
preferRelative: true,
})
);

return (context, request) => {
return (context, request, fromImport) => {
// 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
Expand Down Expand Up @@ -434,7 +460,7 @@ function getWebpackResolver(
// `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,
resolve: fromImport ? sassImportResolve : sassModuleResolve,
context: path.dirname(context),
possibleRequests: sassPossibleRequests,
});
Expand All @@ -444,7 +470,7 @@ function getWebpackResolver(
// eslint-disable-next-line no-shadow
includePaths.map((context) => {
return {
resolve: sassResolve,
resolve: fromImport ? sassImportResolve : sassModuleResolve,
context,
possibleRequests: sassPossibleRequests,
};
Expand All @@ -455,7 +481,7 @@ function getWebpackResolver(
const webpackPossibleRequests = getPossibleRequests(request, true);

resolutionMap = resolutionMap.concat({
resolve: webpackResolve,
resolve: fromImport ? webpackImportResolve : webpackModuleResolve,
context: path.dirname(context),
possibleRequests: webpackPossibleRequests,
});
Expand All @@ -464,7 +490,7 @@ function getWebpackResolver(
};
}

const matchCss = /\.css$/i;
const MATCH_CSS = /\.css$/i;

function getWebpackImporter(loaderContext, implementation, includePaths) {
const resolve = getWebpackResolver(
Expand All @@ -473,16 +499,18 @@ function getWebpackImporter(loaderContext, implementation, includePaths) {
includePaths
);

return (originalUrl, prev, done) => {
resolve(prev, originalUrl)
return function importer(originalUrl, prev, done) {
const { fromImport } = this;

resolve(prev, originalUrl, fromImport)
.then((result) => {
// Add the result as dependency.
// 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, "") });
done({ file: result.replace(MATCH_CSS, "") });
})
// Catch all resolving errors, return the original file and pass responsibility back to other custom importers
.catch(() => {
Expand Down
58 changes: 58 additions & 0 deletions test/__snapshots__/loader.test.js.snap
Original file line number Diff line number Diff line change
@@ -1,5 +1,45 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`loader should import .import.sass files (dart-sass) (sass): css 1`] = `
"a {
display: block;
}"
`;

exports[`loader should import .import.sass files (dart-sass) (sass): errors 1`] = `Array []`;

exports[`loader should import .import.sass files (dart-sass) (sass): warnings 1`] = `Array []`;

exports[`loader should import .import.sass files from a package (dart-sass) (sass): css 1`] = `
"a {
display: block;
}"
`;

exports[`loader should import .import.sass files from a package (dart-sass) (sass): errors 1`] = `Array []`;

exports[`loader should import .import.sass files from a package (dart-sass) (sass): warnings 1`] = `Array []`;

exports[`loader should import .import.scss files (dart-sass) (scss): css 1`] = `
"a {
display: block;
}"
`;

exports[`loader should import .import.scss files (dart-sass) (scss): errors 1`] = `Array []`;

exports[`loader should import .import.scss files (dart-sass) (scss): warnings 1`] = `Array []`;

exports[`loader should import .import.scss files from a package (dart-sass) (scss): css 1`] = `
"a {
display: block;
}"
`;

exports[`loader should import .import.scss files from a package (dart-sass) (scss): errors 1`] = `Array []`;

exports[`loader should import .import.scss files from a package (dart-sass) (scss): warnings 1`] = `Array []`;

exports[`loader should load files with underscore in the name (dart-sass) (sass): css 1`] = `
"a {
color: red;
Expand Down Expand Up @@ -80,6 +120,24 @@ exports[`loader should load only sass/scss files for the "mainFiles" (node-sass)

exports[`loader should load only sass/scss files for the "mainFiles" (node-sass) (scss): warnings 1`] = `Array []`;

exports[`loader should not use .import.sass files (dart-sass) (sass): errors 1`] = `
Array [
"ModuleBuildError: Module build failed (from ../src/cjs.js):
SassError: Can't find stylesheet to import.",
]
`;

exports[`loader should not use .import.sass files (dart-sass) (sass): warnings 1`] = `Array []`;

exports[`loader should not use .import.scss files (dart-sass) (scss): errors 1`] = `
Array [
"ModuleBuildError: Module build failed (from ../src/cjs.js):
SassError: Can't find stylesheet to import.",
]
`;

exports[`loader should not use .import.scss files (dart-sass) (scss): warnings 1`] = `Array []`;

exports[`loader should output an understandable error (dart-sass) (sass): errors 1`] = `
Array [
"ModuleBuildError: Module build failed (from ../src/cjs.js):
Expand Down
44 changes: 44 additions & 0 deletions test/loader.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1599,6 +1599,50 @@ describe("loader", () => {
expect(getErrors(stats)).toMatchSnapshot("errors");
});

it(`should import .import.${syntax} files (${implementationName}) (${syntax})`, async () => {
const testId = getTestId("import-index-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");
});

it(`should import .import.${syntax} files from a package (${implementationName}) (${syntax})`, async () => {
const testId = getTestId("import-index-import-from-package", 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 not use .import.${syntax} files (${implementationName}) (${syntax})`, async () => {
const testId = getTestId("use-index-import", syntax);
const options = {
implementation: getImplementationByName(implementationName),
};
const compiler = getCompiler(testId, { loader: { options } });
const stats = await compile(compiler);

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
3 changes: 3 additions & 0 deletions test/node_modules/index-import-package/_index.import.scss

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions test/node_modules/index-import-package/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions test/sass/import-index-import-from-package.sass
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import '~index-import-package'
1 change: 1 addition & 0 deletions test/sass/import-index-import.sass
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import 'index-import'
2 changes: 2 additions & 0 deletions test/sass/index-import/_index.import.sass
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
a
display: block
1 change: 1 addition & 0 deletions test/sass/use-index-import.sass
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@use 'index-import'
1 change: 1 addition & 0 deletions test/scss/import-index-import-from-package.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import '~index-import-package';
1 change: 1 addition & 0 deletions test/scss/import-index-import.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import 'index-import';
3 changes: 3 additions & 0 deletions test/scss/index-import/_index.import.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
a {
display: block;
}
1 change: 1 addition & 0 deletions test/scss/use-index-import.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@use 'index-import';