From dc19cd525c8802d35dd9273d1f33df9252d25d21 Mon Sep 17 00:00:00 2001 From: Nowell Strite Date: Mon, 15 Feb 2016 14:56:17 -0500 Subject: [PATCH 1/2] Custom importers cause out of order chained imports. --- test/api.js | 23 +++++++++++++++++++ .../chained-imports-with-custom-importer.scss | 1 + .../file-not-processed-by-loader.scss | 1 + .../file-processed-by-loader.scss | 3 +++ 4 files changed, 28 insertions(+) create mode 100644 test/fixtures/include-files/chained-imports-with-custom-importer.scss create mode 100644 test/fixtures/include-files/file-not-processed-by-loader.scss create mode 100644 test/fixtures/include-files/file-processed-by-loader.scss diff --git a/test/api.js b/test/api.js index bbdd754c1..b5ae5aa23 100644 --- a/test/api.js +++ b/test/api.js @@ -186,6 +186,29 @@ describe('api', function() { describe('.render(importer)', function() { var src = read(fixture('include-files/index.scss'), 'utf8'); + it('should respect the order of chained imports when using custom importers and one file is custom imported and the other is not.', function(done) { + sass.render({ + file: fixture('include-files/chained-imports-with-custom-importer.scss'), + importer: function(url, prev, done) { + if (url !== 'file-processed-by-loader') { + return sass.NULL; + } + done({ + file: fixture('include-files/file-processed-by-loader.scss') + }); + } + }, function(err, data) { + assert.equal(err, null); + + assert.equal( + data.css.toString().trim(), + 'body {\n color: "red"; }' + ); + + done(); + }); + }); + it('should still call the next importer with the resolved prev path when the previous importer returned both a file and contents property - issue #1219', function(done) { sass.render({ data: '@import "a";', diff --git a/test/fixtures/include-files/chained-imports-with-custom-importer.scss b/test/fixtures/include-files/chained-imports-with-custom-importer.scss new file mode 100644 index 000000000..8dbe66528 --- /dev/null +++ b/test/fixtures/include-files/chained-imports-with-custom-importer.scss @@ -0,0 +1 @@ +@import "file-not-processed-by-loader", "file-processed-by-loader"; diff --git a/test/fixtures/include-files/file-not-processed-by-loader.scss b/test/fixtures/include-files/file-not-processed-by-loader.scss new file mode 100644 index 000000000..47f8c1dda --- /dev/null +++ b/test/fixtures/include-files/file-not-processed-by-loader.scss @@ -0,0 +1 @@ +$variable-defined-by-file-not-processed-by-loader: 'red'; diff --git a/test/fixtures/include-files/file-processed-by-loader.scss b/test/fixtures/include-files/file-processed-by-loader.scss new file mode 100644 index 000000000..4c79efe0a --- /dev/null +++ b/test/fixtures/include-files/file-processed-by-loader.scss @@ -0,0 +1,3 @@ +body { + color: $variable-defined-by-file-not-processed-by-loader; +} From 6d664bfb19c3cb7df75a80b531c89a842b6ead63 Mon Sep 17 00:00:00 2001 From: Nowell Strite Date: Mon, 15 Feb 2016 15:12:25 -0500 Subject: [PATCH 2/2] Added a better example failing test case. --- test/api.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/test/api.js b/test/api.js index b5ae5aa23..322fd2cf9 100644 --- a/test/api.js +++ b/test/api.js @@ -190,11 +190,22 @@ describe('api', function() { sass.render({ file: fixture('include-files/chained-imports-with-custom-importer.scss'), importer: function(url, prev, done) { + // NOTE: to see that this test failure is only due to the stated + // issue do each of the following and see that the tests pass. + // + // a) add `return sass.NULL;` as the first line in this function to + // cause non-custom importers to always be used. + // b) comment out the conditional below to force our custom + // importer to always be used. + // + // You will notice that the tests pass when either all native, or + // all custom importers are used, but not when a native + custom + // import chain is used. if (url !== 'file-processed-by-loader') { return sass.NULL; } done({ - file: fixture('include-files/file-processed-by-loader.scss') + file: fixture('include-files/' + url + '.scss') }); } }, function(err, data) {