Skip to content

Commit

Permalink
fix: resolving _index.import.scss/index.import.scss in packages (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
alexander-akait committed May 31, 2021
1 parent 67aa139 commit 6641a16
Show file tree
Hide file tree
Showing 13 changed files with 165 additions and 10 deletions.
48 changes: 38 additions & 10 deletions src/utils.js
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
@@ -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
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
@@ -0,0 +1 @@
@import '~index-import-package'
1 change: 1 addition & 0 deletions test/sass/import-index-import.sass
@@ -0,0 +1 @@
@import 'index-import'
2 changes: 2 additions & 0 deletions test/sass/index-import/_index.import.sass
@@ -0,0 +1,2 @@
a
display: block
1 change: 1 addition & 0 deletions test/sass/use-index-import.sass
@@ -0,0 +1 @@
@use 'index-import'
1 change: 1 addition & 0 deletions test/scss/import-index-import-from-package.scss
@@ -0,0 +1 @@
@import '~index-import-package';
1 change: 1 addition & 0 deletions test/scss/import-index-import.scss
@@ -0,0 +1 @@
@import 'index-import';
3 changes: 3 additions & 0 deletions test/scss/index-import/_index.import.scss
@@ -0,0 +1,3 @@
a {
display: block;
}
1 change: 1 addition & 0 deletions test/scss/use-index-import.scss
@@ -0,0 +1 @@
@use 'index-import';

0 comments on commit 6641a16

Please sign in to comment.