Skip to content

Commit

Permalink
fix: resolution for file scheme
Browse files Browse the repository at this point in the history
  • Loading branch information
evilebottnawi committed Apr 23, 2020
1 parent 744112d commit 17832fd
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 21 deletions.
60 changes: 39 additions & 21 deletions src/webpackImporter.js
Expand Up @@ -70,41 +70,59 @@ function webpackImporter(loaderContext, includePaths) {
mainFiles: [],
modules: [],
});
// TODO implement the `restrictions` option for `enhanced-resolve` and avoid resolution `js` files from the `mainFields`
// TODO avoid resolsing `_index`, `index` and files without extensions
// TODO avoid resolving with multiple extensions - `file.sass.sass`/`file.sass.scss`/`file.sass.css`
const webpackResolve = loaderContext.getResolve({
mainFields: ['sass', 'style', 'main', '...'],
mainFiles: ['_index', 'index', '...'],
extensions: ['.sass', '.scss', '.css'],
});

return (url, prev, done) => {
// 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(
return (originalUrl, prev, done) => {
let url = originalUrl;

const isFileScheme = originalUrl.startsWith('file:');

if (isFileScheme) {
// eslint-disable-next-line no-param-reassign
url = originalUrl.slice(5);
}

let resolutionMap = [];

if (includePaths.length > 0 && !isFileScheme) {
// 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 before `3`, `4` and `5` points, we need to emulate this behavior to avoid wrong resolution.
const sassPossibleRequests = getPossibleRequests(url);

resolutionMap = resolutionMap.concat(
includePaths.map((context) => ({
resolve: sassResolve,
context,
possibleRequests: sassPossibleRequests,
}))
)
.concat({
resolve: webpackResolve,
context: dirContextFrom(prev),
possibleRequests: webpackPossibleRequests,
});
);
}

const webpackPossibleRequests = getPossibleRequests(url, true);

resolutionMap = resolutionMap.concat({
resolve: webpackResolve,
context: isFileScheme ? '/' : dirContextFrom(prev),
possibleRequests: webpackPossibleRequests,
});

startResolving(resolutionMap)
// Catch all resolving errors, return the original file and pass responsibility back to other custom importers
.catch(() => ({ file: url }))
.catch(() => ({ file: originalUrl }))
.then(done);
};
}
Expand Down
4 changes: 4 additions & 0 deletions test/helpers/getCodeFromSass.js
Expand Up @@ -252,6 +252,9 @@ function getCodeFromSass(testId, options) {
testFolder,
'node_modules/package-with-js-and-css-main-files/index'
);
const pathToLanguage = isSass
? path.resolve(testFolder, 'sass/language.sass')
: path.resolve(testFolder, 'scss/language.scss');

// Pseudo importer for tests
function testImporter(url) {
Expand Down Expand Up @@ -710,6 +713,7 @@ function getCodeFromSass(testId, options) {
/^~package-with-js-and-css-main-files/,
pathToPackageWithJsAndCssMainFiles
)
.replace(/^file:language/, pathToLanguage)
.replace(/^~/, testNodeModules);
}

Expand Down
24 changes: 24 additions & 0 deletions test/loader.test.js
Expand Up @@ -721,6 +721,30 @@ describe('loader', () => {
expect(getErrors(stats)).toMatchSnapshot('errors');
});

// TODO need fix on webpack@5 side
it.skip(`should support resolving using the "file" schema (${implementationName}) (${syntax})`, async () => {
const testId = getTestId('import-file-scheme', syntax);
const options = {
implementation: getImplementationByName(implementationName),
};
const compiler = getCompiler(testId, {
loader: { options },
resolve: {
alias: {
'/language': path.resolve('./test', syntax, `language.${syntax}`),
},
},
});
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
1 change: 1 addition & 0 deletions test/sass/import-file-scheme.sass
@@ -0,0 +1 @@
@import 'file:language'
1 change: 1 addition & 0 deletions test/scss/import-file-scheme.scss
@@ -0,0 +1 @@
@import 'file:language';

0 comments on commit 17832fd

Please sign in to comment.