From cbe5ad407582a617be097d3eadd3ad8619e52507 Mon Sep 17 00:00:00 2001 From: Evilebot Tnawi Date: Sun, 12 Apr 2020 18:59:55 +0300 Subject: [PATCH] fix: resolution logic when the `includePaths` option used (#827) --- src/index.js | 14 ++++---- src/webpackImporter.js | 36 +++++++++++++------ test/__snapshots__/loader.test.js.snap | 8 ++--- test/helpers/getCodeFromSass.js | 8 ----- test/loader.test.js | 4 ++- .../package.json | 2 +- 6 files changed, 39 insertions(+), 33 deletions(-) diff --git a/src/index.js b/src/index.js index 09897066..18c0d824 100644 --- a/src/index.js +++ b/src/index.js @@ -33,20 +33,18 @@ function loader(content) { : true; if (shouldUseWebpackImporter) { - let modules = ['...']; - - if (options.sassOptions && options.sassOptions.includePaths) { - modules = options.sassOptions.includePaths.concat(modules); - } - const resolve = this.getResolve({ - modules, mainFields: ['sass', 'style', 'main', '...'], mainFiles: ['_index', 'index', '...'], extensions: ['.scss', '.sass', '.css', '...'], }); - sassOptions.importer.push(webpackImporter(this, resolve)); + const includePaths = + options.sassOptions && options.sassOptions.includePaths + ? options.sassOptions.includePaths + : []; + + sassOptions.importer.push(webpackImporter(this, resolve, includePaths)); } const callback = this.async(); diff --git a/src/webpackImporter.js b/src/webpackImporter.js index b5972938..5d8dfde4 100644 --- a/src/webpackImporter.js +++ b/src/webpackImporter.js @@ -27,7 +27,7 @@ const matchCss = /\.css$/i; * (based on whether the call is sync or async) because otherwise node-sass doesn't exit. * */ -function webpackImporter(loaderContext, resolve) { +function webpackImporter(loaderContext, resolve, includePaths) { function dirContextFrom(fileContext) { return path.dirname( // The first file is 'stdin' when we're using the data option @@ -35,15 +35,18 @@ function webpackImporter(loaderContext, resolve) { ); } - function startResolving(context, possibleRequests) { - if (possibleRequests.length === 0) { - return Promise.resolve(); + function startResolving(resolutionMap) { + if (resolutionMap.length === 0) { + return Promise.reject(); } + const [{ context, possibleRequests }] = resolutionMap; + return resolve(context, possibleRequests[0]) .then((result) => { - // Add the resolvedFilename 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. + // 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. @@ -52,18 +55,29 @@ function webpackImporter(loaderContext, resolve) { .catch(() => { const [, ...tailResult] = possibleRequests; - return startResolving(context, tailResult); + if (tailResult.length === 0) { + const [, ...tailResolutionMap] = resolutionMap; + + return startResolving(tailResolutionMap); + } + + // eslint-disable-next-line no-param-reassign + resolutionMap[0].possibleRequests = tailResult; + + return startResolving(resolutionMap); }); } return (url, prev, done) => { const possibleRequests = getPossibleRequests(url); + const resolutionMap = [] + .concat(includePaths) + .concat(dirContextFrom(prev)) + .map((context) => ({ context, possibleRequests })); - startResolving(dirContextFrom(prev), possibleRequests) + startResolving(resolutionMap) // Catch all resolving errors, return the original file and pass responsibility back to other custom importers - .catch(() => { - return { file: url }; - }) + .catch(() => ({ file: url })) .then(done); }; } diff --git a/test/__snapshots__/loader.test.js.snap b/test/__snapshots__/loader.test.js.snap index 89665d2f..8b5d682a 100644 --- a/test/__snapshots__/loader.test.js.snap +++ b/test/__snapshots__/loader.test.js.snap @@ -462,9 +462,9 @@ exports[`loader should work and ignore all css "@import" at-rules (node-sass) (s "@import url(~css/some-css-module.css); @import url(./does/not/exist.css); @import url(theme.css); -@import \\"http://fonts.googleapis.com/css?family=Droid+Sans\\"; +@import http://fonts.googleapis.com/css?family=Droid+Sans; @import url(theme); -@import \\"landscape\\" screen and (orientation: landscape); +@import landscape screen and (orientation: landscape); .some-css-module { background: hotpink; } " @@ -478,9 +478,9 @@ exports[`loader should work and ignore all css "@import" at-rules (node-sass) (s "@import url(~css/some-css-module.css); @import url(./does/not/exist.css); @import url(theme.css); -@import \\"http://fonts.googleapis.com/css?family=Droid+Sans\\"; +@import http://fonts.googleapis.com/css?family=Droid+Sans; @import url(theme); -@import \\"landscape\\" screen and (orientation: landscape); +@import landscape screen and (orientation: landscape); .some-css-module { background: hotpink; } " diff --git a/test/helpers/getCodeFromSass.js b/test/helpers/getCodeFromSass.js index 6892c3c1..c1a21a4f 100644 --- a/test/helpers/getCodeFromSass.js +++ b/test/helpers/getCodeFromSass.js @@ -693,14 +693,6 @@ function getCodeFromSass(testId, options) { .replace(/^~/, testNodeModules); } - if ( - isNodeSassImplementation && - (url === 'landscape' || url.startsWith('http://')) - ) { - // eslint-disable-next-line consistent-return - return; - } - return { file: url, }; diff --git a/test/loader.test.js b/test/loader.test.js index 237c5f85..5852b889 100644 --- a/test/loader.test.js +++ b/test/loader.test.js @@ -559,7 +559,9 @@ describe('loader', () => { const options = { sassOptions: { includePaths: [ - `node_modules/package-with-style-field-and-css/${syntax}`, + path.resolve( + `./test/node_modules/package-with-style-field-and-css/${syntax}` + ), ], }, implementation: getImplementationByName(implementationName), diff --git a/test/node_modules/package-with-style-field-and-css/package.json b/test/node_modules/package-with-style-field-and-css/package.json index 7cb463e5..3e87e996 100644 --- a/test/node_modules/package-with-style-field-and-css/package.json +++ b/test/node_modules/package-with-style-field-and-css/package.json @@ -7,5 +7,5 @@ }, "author": "", "license": "MIT", - "style": "css/font-awesome.css" + "style": "css/package-with-style-field-and-css.css" }