From 94d0bc857aa0ef14790926183e88602bab7c2506 Mon Sep 17 00:00:00 2001 From: Ray Date: Sun, 28 Jun 2015 09:49:28 +0800 Subject: [PATCH 1/4] bugfix: after try the underscore file name, should pass the filename not prefix with underscore to libsass result --- index.js | 40 ++++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/index.js b/index.js index 0788a663..155b414b 100644 --- a/index.js +++ b/index.js @@ -104,6 +104,19 @@ module.exports = function (content) { return path.dirname(context); } + // add result files to loader + function addNodeSassResult2WebpackDep(loader, result) { + if (!loader || !result || !result.stats + || !result.stats.includedFiles || 1 > result.stats.includedFiles) { + return; + } + + var depFn = loader.addDependency || loader.dependency; + for (var i in result.stats.includedFiles) { + depFn(result.stats.includedFiles[i]); + } + } + this.cacheable(); opt = utils.parseQuery(this.query); @@ -140,7 +153,10 @@ module.exports = function (content) { // start the actual rendering if (isSync) { try { - return sass.renderSync(opt).css.toString(); + + var ret = sass.renderSync(opt); + addNodeSassResult2WebpackDep(self, ret); + return ret.css.toString(); } catch (err) { formatSassError(err); throw err; @@ -163,6 +179,8 @@ module.exports = function (content) { result.map = null; } + addNodeSassResult2WebpackDep(self, result); + callback(null, result.css.toString(), result.map); }); }; @@ -207,17 +225,17 @@ function syncResolve(loaderContext, url, context) { try { filename = loaderContext.resolveSync(context, url); - loaderContext.dependency && loaderContext.dependency(filename); } catch (err) { basename = path.basename(url); if (requiresLookupForUnderscoreModule(err, basename)) { url = addUnderscoreToBasename(url, basename); return syncResolve(loaderContext, url, context); } - // Unfortunately we can't return an error inside a custom importer yet - // @see https://github.com/sass/node-sass/issues/651#issuecomment-73317319 - filename = url; + + // let the libsass do the rest job, e.g. search module in includePaths + filename = path.join(path.dirname(url), removeUnderscoreFromBasename(basename)); } + return { file: filename }; @@ -242,11 +260,9 @@ function asyncResolve(loaderContext, url, context, done) { url = addUnderscoreToBasename(url, basename); return asyncResolve(loaderContext, url, context, done); } - // Unfortunately we can't return an error inside a custom importer yet - // @see https://github.com/sass/node-sass/issues/651#issuecomment-73317319 - filename = url; - } else { - loaderContext.dependency && loaderContext.dependency(filename); + + // let the libsass do the rest job, e.g. search module in includePaths + filename = path.join(path.dirname(url), removeUnderscoreFromBasename(basename)); } // Use self.loadModule() before calling done() to make imported files available to @@ -277,3 +293,7 @@ function requiresLookupForUnderscoreModule(err, basename) { function addUnderscoreToBasename(url, basename) { return url.slice(0, -basename.length) + '_' + basename; } + +function removeUnderscoreFromBasename(basename) { + return !basename ? basename : ("_" === basename[0] ? basename.substring(1) : basename); +} From 6393a400a1d613b251be1c625351869856808f75 Mon Sep 17 00:00:00 2001 From: Ray Date: Sun, 28 Jun 2015 09:57:23 +0800 Subject: [PATCH 2/4] addDepFn empty condition bugfix --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index 155b414b..39814b96 100644 --- a/index.js +++ b/index.js @@ -107,7 +107,7 @@ module.exports = function (content) { // add result files to loader function addNodeSassResult2WebpackDep(loader, result) { if (!loader || !result || !result.stats - || !result.stats.includedFiles || 1 > result.stats.includedFiles) { + || !result.stats.includedFiles || 1 > result.stats.includedFiles.length) { return; } From 603c8cdd7238bf28f84aeef6c7e3b354e0fe9f49 Mon Sep 17 00:00:00 2001 From: Ray Date: Wed, 1 Jul 2015 16:36:02 +0800 Subject: [PATCH 3/4] add tests for includePath feature in node-sass --- test/another/sass/_underscoreIncPathStyle.sass | 1 + test/another/sass/incPathsStyle.sass | 1 + test/another/scss/_underscoreIncPathStyle.scss | 1 + test/another/scss/incPathsStyle.scss | 1 + test/index.test.js | 7 +++++-- test/prepare.js | 3 ++- test/sass/import-include-paths.sass | 2 ++ test/scss/import-include-paths.scss | 2 ++ 8 files changed, 15 insertions(+), 3 deletions(-) create mode 100644 test/another/sass/_underscoreIncPathStyle.sass create mode 100644 test/another/sass/incPathsStyle.sass create mode 100644 test/another/scss/_underscoreIncPathStyle.scss create mode 100644 test/another/scss/incPathsStyle.scss create mode 100644 test/sass/import-include-paths.sass create mode 100644 test/scss/import-include-paths.scss diff --git a/test/another/sass/_underscoreIncPathStyle.sass b/test/another/sass/_underscoreIncPathStyle.sass new file mode 100644 index 00000000..b960b372 --- /dev/null +++ b/test/another/sass/_underscoreIncPathStyle.sass @@ -0,0 +1 @@ +// just empty for includePaths feature diff --git a/test/another/sass/incPathsStyle.sass b/test/another/sass/incPathsStyle.sass new file mode 100644 index 00000000..b960b372 --- /dev/null +++ b/test/another/sass/incPathsStyle.sass @@ -0,0 +1 @@ +// just empty for includePaths feature diff --git a/test/another/scss/_underscoreIncPathStyle.scss b/test/another/scss/_underscoreIncPathStyle.scss new file mode 100644 index 00000000..b960b372 --- /dev/null +++ b/test/another/scss/_underscoreIncPathStyle.scss @@ -0,0 +1 @@ +// just empty for includePaths feature diff --git a/test/another/scss/incPathsStyle.scss b/test/another/scss/incPathsStyle.scss new file mode 100644 index 00000000..b960b372 --- /dev/null +++ b/test/another/scss/incPathsStyle.scss @@ -0,0 +1 @@ +// just empty for includePaths feature diff --git a/test/index.test.js b/test/index.test.js index 9a377740..9c76bf84 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -38,6 +38,9 @@ describe('sass-loader', function () { testSync('should resolve imports from other language style correctly (sync)', 'import-other-style'); testAsync('should resolve imports from other language style correctly (async)', 'import-other-style'); + // test for includepath not under context + testSync('should resolve imports from another directory declared by includePaths correctly(sync)', 'import-include-paths'); + testAsync('should resolve imports from another directory declared by includePaths correctly(async)', 'import-include-paths'); }); describe('errors', function () { @@ -72,7 +75,7 @@ describe('sass-loader', function () { } catch (err) { // check for file excerpt err.message.should.match(/@import "does-not-exist";/); - err.message.should.match(/File to import not found or unreadable: \.\/_does-not-exist\.scss/); + err.message.should.match(/File to import not found or unreadable: does-not-exist\.scss/); err.message.should.match(/\(line 1, column 9\)/); err.message.indexOf(pathToErrorFileNotFound).should.not.equal(-1); } @@ -140,6 +143,6 @@ function testSync(name, id) { function pathToSassFile(ext, id) { return 'raw!' + pathToSassLoader + '?' + - (ext === 'sass'? '&indentedSyntax' : '') + '!' + + (ext === 'sass'? '&indentedSyntax&' : '') + 'includePaths[]=' + path.join(__dirname, 'another', ext) + '!' + path.join(__dirname, ext, id + '.' + ext); } diff --git a/test/prepare.js b/test/prepare.js index 7ed4a144..614d5d0a 100644 --- a/test/prepare.js +++ b/test/prepare.js @@ -36,7 +36,8 @@ var filesWithTildeImports = [ css = sass.renderSync({ file: fileName, includePaths: [ - path.join(__dirname, ext, 'another') + path.join(__dirname, ext, 'another'), + path.join(__dirname, 'another', ext) ] }).css; diff --git a/test/sass/import-include-paths.sass b/test/sass/import-include-paths.sass new file mode 100644 index 00000000..01adb54c --- /dev/null +++ b/test/sass/import-include-paths.sass @@ -0,0 +1,2 @@ +@import incPathsStyle +@import underscoreIncPathStyle diff --git a/test/scss/import-include-paths.scss b/test/scss/import-include-paths.scss new file mode 100644 index 00000000..c9345f38 --- /dev/null +++ b/test/scss/import-include-paths.scss @@ -0,0 +1,2 @@ +@import "incPathsStyle"; +@import "underscoreIncPathStyle"; From c3cf32feaa83c2b370f8c9c9a7b53d193c8d3513 Mon Sep 17 00:00:00 2001 From: Ray Date: Wed, 1 Jul 2015 16:40:37 +0800 Subject: [PATCH 4/4] add contribution instruction in readme --- README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/README.md b/README.md index 3d3182f4..1c9724f4 100644 --- a/README.md +++ b/README.md @@ -133,6 +133,14 @@ module.exports = { If you want to view the original Sass files inside Chrome and even edit it, [there's a good blog post](https://medium.com/@toolmantim/getting-started-with-css-sourcemaps-and-in-browser-sass-editing-b4daab987fb0). Checkout [test/sourceMap](https://github.com/jtangelder/sass-loader/tree/master/test) for a running example. Make sure to serve the content with an HTTP server. +## Contribution + +### How to run test +```bash + mkdir -p test/node_modules/ && touch test/node_modules/some-module.s{a,c}ss && touch test/node_modules/underscore.s{a,c}ss + npm test +``` + ## License MIT (http://www.opensource.org/licenses/mit-license.php)