Skip to content

Commit

Permalink
Fix bug where multiple compilations interfered with each other
Browse files Browse the repository at this point in the history
  • Loading branch information
jhnns committed Feb 8, 2017
1 parent b387510 commit 8d54166
Show file tree
Hide file tree
Showing 10 changed files with 77 additions and 4 deletions.
10 changes: 9 additions & 1 deletion lib/normalizeOptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,21 @@ const proxyCustomImporters = require("./proxyCustomImporters");
/**
* Derives the sass options from the loader context and normalizes its values with sane defaults.
*
* Please note: If loaderContext.query is an options object, it will be re-used across multiple invocations.
* That's why we must not modify the object directly.
*
* @param {LoaderContext} loaderContext
* @param {string} content
* @param {Function} webpackImporter
* @returns {Object}
*/
function normalizeOptions(loaderContext, content, webpackImporter) {
const options = utils.parseQuery(loaderContext.query);
const options = loaderContext.query && typeof loaderContext.query === "object" ?
// Make a copy of the query object
// @see https://github.com/jtangelder/sass-loader/issues/368#issuecomment-278330164
Object.assign({}, loaderContext.query) :
utils.parseQuery(loaderContext.query);

const resourcePath = loaderContext.resourcePath;

options.data = options.data ? (options.data + os.EOL + content) : content;
Expand Down
63 changes: 60 additions & 3 deletions test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@ syntaxStyles.forEach(ext => {

runWebpack(baseConfig, (err) => err ? reject(err) : resolve());
}).then(() => {
delete require.cache[path.resolve(__dirname, "./output/bundle." + ext + ".js")];

const actualCss = require("./output/bundle." + ext + ".js");
const actualCss = readBundle("bundle." + ext + ".js");
const expectedCss = readCss(ext, testId);

// writing the actual css to output-dir for better debugging
Expand Down Expand Up @@ -86,6 +84,59 @@ syntaxStyles.forEach(ext => {
});

describe("sass-loader", () => {
describe("multiple compilations", () => {
it("should not interfere with each other", () =>
new Promise((resolve, reject) => {
runWebpack({
entry: {
b: path.join(__dirname, "scss", "multipleCompilations", "b.scss"),
c: path.join(__dirname, "scss", "multipleCompilations", "c.scss"),
a: path.join(__dirname, "scss", "multipleCompilations", "a.scss"),
d: path.join(__dirname, "scss", "multipleCompilations", "d.scss"),
e: path.join(__dirname, "scss", "multipleCompilations", "e.scss"),
f: path.join(__dirname, "scss", "multipleCompilations", "f.scss"),
g: path.join(__dirname, "scss", "multipleCompilations", "g.scss"),
h: path.join(__dirname, "scss", "multipleCompilations", "h.scss")
},
output: {
filename: "bundle.multiple-compilations.[name].js"
},
module: {
rules: [{
test: /\.scss$/,
use: [
{ loader: "raw-loader" },
// We're specifying an empty options object because otherwise, webpack creates a new object for every loader invocation
// Since we want to ensure that our loader is not tampering with the option object, we are triggering webpack to re-use the options object
// @see https://github.com/jtangelder/sass-loader/issues/368#issuecomment-278330164
{ loader: pathToSassLoader, options: {} }
]
}]
}
}, (err) => err ? reject(err) : resolve());
})
.then(() => {
const expectedCss = readCss("scss", "imports");
const a = readBundle("bundle.multiple-compilations.a.js");
const b = readBundle("bundle.multiple-compilations.b.js");
const c = readBundle("bundle.multiple-compilations.c.js");
const d = readBundle("bundle.multiple-compilations.d.js");
const e = readBundle("bundle.multiple-compilations.e.js");
const f = readBundle("bundle.multiple-compilations.f.js");
const g = readBundle("bundle.multiple-compilations.g.js");
const h = readBundle("bundle.multiple-compilations.h.js");

a.should.equal(expectedCss);
b.should.equal(expectedCss);
c.should.equal(expectedCss);
d.should.equal(expectedCss);
e.should.equal(expectedCss);
f.should.equal(expectedCss);
g.should.equal(expectedCss);
h.should.equal(expectedCss);
})
);
});
describe("errors", () => {
it("should throw an error in synchronous loader environments", () => {
try {
Expand Down Expand Up @@ -164,3 +215,9 @@ function runWebpack(baseConfig, done) {
done(err || null);
});
}

function readBundle(filename) {
delete require.cache[path.resolve(__dirname, `./output/${ filename }`)];

return require(`./output/${ filename }`);
}
1 change: 1 addition & 0 deletions test/scss/multipleCompilations/a.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import "../imports";
1 change: 1 addition & 0 deletions test/scss/multipleCompilations/b.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import "../imports";
1 change: 1 addition & 0 deletions test/scss/multipleCompilations/c.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import "../imports";
1 change: 1 addition & 0 deletions test/scss/multipleCompilations/d.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import "../imports";
1 change: 1 addition & 0 deletions test/scss/multipleCompilations/e.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import "../imports";
1 change: 1 addition & 0 deletions test/scss/multipleCompilations/f.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import "../imports";
1 change: 1 addition & 0 deletions test/scss/multipleCompilations/g.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import "../imports";
1 change: 1 addition & 0 deletions test/scss/multipleCompilations/h.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import "../imports";

0 comments on commit 8d54166

Please sign in to comment.