Skip to content

Commit

Permalink
fix: resolution logic (#839)
Browse files Browse the repository at this point in the history
  • Loading branch information
evilebottnawi committed Apr 20, 2020
1 parent 7380b7b commit aeb86f0
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 30 deletions.
27 changes: 17 additions & 10 deletions src/getPossibleRequests.js
Expand Up @@ -18,13 +18,14 @@ const matchModuleImport = /^~([^/]+|[^/]+\/|@[^/]+[/][^/]+|@[^/]+\/?|@[^/]+[/][^
* to enable straight-forward webpack.config aliases.
*
* @param {string} url
* @param {boolean} forWebpackResolver
* @returns {Array<string>}
*/
export default function getPossibleRequests(url) {
export default function getPossibleRequests(url, forWebpackResolver = false) {
const request = utils.urlToRequest(url);

// In case there is module request, send this to webpack resolver
if (matchModuleImport.test(url)) {
if (forWebpackResolver && matchModuleImport.test(url)) {
return [request, url];
}

Expand All @@ -51,23 +52,29 @@ export default function getPossibleRequests(url) {
//
// 1. Try to resolve `_` file.
// 2. Try to resolve file without `_`.
// 3. Send a original url to webpack resolver, maybe it is alias.
if (['.scss', '.sass'].includes(ext)) {
return [`${dirname}/_${basename}`, `${dirname}/${basename}`, url];
// 3. Send a original url to webpack resolver, maybe it is alias for webpack resolving.
if (['.scss', '.sass', '.css'].includes(ext)) {
return [`${dirname}/_${basename}`, `${dirname}/${basename}`].concat(
forWebpackResolver ? [url] : []
);
}

// In case there is no file extension
//
// 1. Try to resolve files starts with `_` and normal with order `sass`, `scss` and `css`
// 2. Send a original url to webpack resolver, maybe it is alias.
// 1. Try to resolve files starts with `_` and normal with order `sass`, `scss` and `css`.
// 2. Send a original url to webpack resolver, maybe it is alias for webpack resolving.
return [
`${dirname}/_${basename}.sass`,
`${dirname}/${basename}.sass`,
`${dirname}/_${basename}.scss`,
`${dirname}/${basename}.scss`,
`${dirname}/_${basename}.css`,
`${dirname}/${basename}.css`,
request,
url,
];
`${dirname}/${basename}/_index.sass`,
`${dirname}/${basename}/index.sass`,
`${dirname}/${basename}/_index.scss`,
`${dirname}/${basename}/index.scss`,
`${dirname}/${basename}/_index.css`,
`${dirname}/${basename}/index.css`,
].concat(forWebpackResolver ? [request, url] : []);
}
7 changes: 2 additions & 5 deletions src/index.js
Expand Up @@ -36,13 +36,10 @@ function loader(content) {
const resolve = this.getResolve({
mainFields: ['sass', 'style', 'main', '...'],
mainFiles: ['_index', 'index', '...'],
extensions: ['.scss', '.sass', '.css'],
extensions: ['.sass', '.scss', '.css'],
});

const includePaths =
options.sassOptions && options.sassOptions.includePaths
? options.sassOptions.includePaths
: [];
const { includePaths } = sassOptions;

sassOptions.importer.push(webpackImporter(this, resolve, includePaths));
}
Expand Down
33 changes: 21 additions & 12 deletions src/webpackImporter.js
@@ -1,11 +1,3 @@
/**
* @name PromisedResolve
* @type {Function}
* @param {string} dir
* @param {string} request
* @returns Promise
*/

/**
* @name Importer
* @type {Function}
Expand Down Expand Up @@ -69,11 +61,28 @@ function webpackImporter(loaderContext, resolve, includePaths) {
}

return (url, prev, done) => {
const possibleRequests = getPossibleRequests(url);
// The order of import precedence is as follows:
//
// 1. Filesystem imports relative to the base file.
// 2. Custom importer imports.
// 3. Filesystem imports relative to the working directory.
// 4. Filesystem imports relative to an `includePaths` path.
// 5. Filesystem imports relative to a `SASS_PATH` path.
//
// Because `sass`/`node-sass` run custom importers after `3`, `4` and `5` points, we need to emulate this behavior to avoid wrong resolution.
const sassPossibleRequests = getPossibleRequests(url);
const webpackPossibleRequests = getPossibleRequests(url, true);
const resolutionMap = []
.concat(includePaths)
.concat(dirContextFrom(prev))
.map((context) => ({ context, possibleRequests }));
.concat(
includePaths.map((context) => ({
context,
possibleRequests: sassPossibleRequests,
}))
)
.concat({
context: dirContextFrom(prev),
possibleRequests: webpackPossibleRequests,
});

startResolving(resolutionMap)
// Catch all resolving errors, return the original file and pass responsibility back to other custom importers
Expand Down
40 changes: 40 additions & 0 deletions test/__snapshots__/loader.test.js.snap
Expand Up @@ -224,6 +224,46 @@ SassError: expected \\"{\\".",

exports[`loader should output an understandable error with a problem in "@use" (dart-sass) (scss): warnings 1`] = `Array []`;

exports[`loader should respect resolving directory with the "index" file from "process.cwd()" (dart-sass) (sass): css 1`] = `
".dir-with-underscore-index {
color: red;
}"
`;

exports[`loader should respect resolving directory with the "index" file from "process.cwd()" (dart-sass) (sass): errors 1`] = `Array []`;

exports[`loader should respect resolving directory with the "index" file from "process.cwd()" (dart-sass) (sass): warnings 1`] = `Array []`;

exports[`loader should respect resolving directory with the "index" file from "process.cwd()" (dart-sass) (scss): css 1`] = `
".dir-with-underscore-index {
color: red;
}"
`;

exports[`loader should respect resolving directory with the "index" file from "process.cwd()" (dart-sass) (scss): errors 1`] = `Array []`;

exports[`loader should respect resolving directory with the "index" file from "process.cwd()" (dart-sass) (scss): warnings 1`] = `Array []`;

exports[`loader should respect resolving directory with the "index" file from "process.cwd()" (node-sass) (sass): css 1`] = `
".dir-with-underscore-index {
color: red; }
"
`;

exports[`loader should respect resolving directory with the "index" file from "process.cwd()" (node-sass) (sass): errors 1`] = `Array []`;

exports[`loader should respect resolving directory with the "index" file from "process.cwd()" (node-sass) (sass): warnings 1`] = `Array []`;

exports[`loader should respect resolving directory with the "index" file from "process.cwd()" (node-sass) (scss): css 1`] = `
".dir-with-underscore-index {
color: red; }
"
`;

exports[`loader should respect resolving directory with the "index" file from "process.cwd()" (node-sass) (scss): errors 1`] = `Array []`;

exports[`loader should respect resolving directory with the "index" file from "process.cwd()" (node-sass) (scss): warnings 1`] = `Array []`;

exports[`loader should respect resolving from "process.cwd()" (dart-sass) (sass): css 1`] = `
"body {
font: 100% Helvetica, sans-serif;
Expand Down
12 changes: 12 additions & 0 deletions test/helpers/getCodeFromSass.js
Expand Up @@ -259,6 +259,18 @@ function getCodeFromSass(testId, options) {
if (/\.css$/i.test(url) === false) {
// Polyfill for node-sass implementation
if (isNodeSassImplementation) {
if (url === 'test/scss/dir-with-underscore-index') {
return {
file: pathToSCSSIndexAlias,
};
}

if (url === 'test/sass/dir-with-underscore-index') {
return {
file: pathToSassIndexAlias,
};
}

if (url === '~sass-package-with-index') {
return {
file: pathToSassPackageWithIndexFile,
Expand Down
23 changes: 20 additions & 3 deletions test/loader.test.js
Expand Up @@ -656,8 +656,6 @@ describe('loader', () => {
});

it(`should respect resolving from the "SASS_PATH" environment variable (${implementationName}) (${syntax})`, async () => {
const OLD_SASS_PATH = process.env.SASS_PATH;

process.env.SASS_PATH =
process.platform === 'win32'
? `${path.resolve('test', syntax, 'sass_path')};${path.resolve(
Expand Down Expand Up @@ -685,7 +683,7 @@ describe('loader', () => {
expect(getWarnings(stats)).toMatchSnapshot('warnings');
expect(getErrors(stats)).toMatchSnapshot('errors');

process.env.SASS_PATH = OLD_SASS_PATH;
delete process.env.SASS_PATH;
});

it(`should respect resolving from "process.cwd()" (${implementationName}) (${syntax})`, async () => {
Expand All @@ -704,6 +702,25 @@ describe('loader', () => {
expect(getErrors(stats)).toMatchSnapshot('errors');
});

it(`should respect resolving directory with the "index" file from "process.cwd()" (${implementationName}) (${syntax})`, async () => {
const testId = getTestId(
'process-cwd-with-index-file-inside-directory',
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');
});

if (implementation === dartSass) {
it(`should output an understandable error with a problem in "@use" (${implementationName}) (${syntax})`, async () => {
const testId = getTestId('error-use', syntax);
Expand Down
@@ -0,0 +1 @@
@import 'test/sass/dir-with-underscore-index'
@@ -0,0 +1 @@
@import 'test/scss/dir-with-underscore-index';

0 comments on commit aeb86f0

Please sign in to comment.