Skip to content

Commit

Permalink
Merge pull request #334 from jtangelder/remove/sync-compilation
Browse files Browse the repository at this point in the history 
Remove synchronous compilation support
  • Loading branch information
@jhnns
jhnns committed Dec 26, 2016
2 parents f5a0e77 + a9c9007 commit b68db45
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 123 deletions.
72 changes: 8 additions & 64 deletions index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,13 @@ var asyncSassJobQueue = async.queue(sass.render, threadPoolSize - 1);
* The sass-loader makes node-sass available to webpack modules.
*
* @param {string} content
* @returns {string}
*/
module.exports = function (content) {
var callback = this.async();
var isSync = typeof callback !== 'function';
var self = this;
var resourcePath = this.resourcePath;
var sassOptions = getLoaderConfig(this);
var result;
var sassOptions;

/**
* Enhances the sass error with additional information about what actually went wrong.
Expand Down Expand Up @@ -84,19 +82,6 @@ module.exports = function (content) {
* @returns {function}
*/
function getWebpackImporter() {
if (isSync) {
return function syncWebpackImporter(url, fileContext) {
var dirContext;
var request;

// node-sass returns UNIX-style paths
fileContext = path.normalize(fileContext);
request = utils.urlToRequest(url, sassOptions.root);
dirContext = fileToDirContext(fileContext);

return resolveSync(dirContext, url, getImportsToResolve(request));
};
}
return function asyncWebpackImporter(url, fileContext, done) {
var dirContext;
var request;
Expand All @@ -110,41 +95,6 @@ module.exports = function (content) {
};
}

/**
* Tries to resolve the first url of importsToResolve. If that resolve fails, the next url is tried.
* If all imports fail, the import is passed to libsass which also take includePaths into account.
*
* @param {string} dirContext
* @param {string} originalImport
* @param {Array} importsToResolve
* @returns {object}
*/
function resolveSync(dirContext, originalImport, importsToResolve) {
var importToResolve = importsToResolve.shift();
var resolvedFilename;

if (!importToResolve) {
// No import possibilities left. Let's pass that one back to libsass...
return {
file: originalImport
};
}

try {
resolvedFilename = self.resolveSync(dirContext, importToResolve);
// 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.
addNormalizedDependency(resolvedFilename);
// By removing the CSS file extension, we trigger node-sass to include the CSS file instead of just linking it.
resolvedFilename = resolvedFilename.replace(matchCss, '');
return {
file: resolvedFilename
};
} catch (err) {
return resolveSync(dirContext, originalImport, importsToResolve);
}
}

/**
* Tries to resolve the first url of importsToResolve. If that resolve fails, the next url is tried.
* If all imports fail, the import is passed to libsass which also take includePaths into account.
Expand Down Expand Up @@ -203,14 +153,20 @@ module.exports = function (content) {
// node-sass returns UNIX-style paths
self.dependency(path.normalize(file));
}

if (isSync) {
throw new Error('Synchronous compilation is not supported anymore. See https://github.com/jtangelder/sass-loader/issues/333');
}

this.cacheable();

sassOptions = getLoaderConfig(this);
sassOptions.data = sassOptions.data ? (sassOptions.data + os.EOL + content) : content;

// Skip empty files, otherwise it will stop webpack, see issue #21
if (sassOptions.data.trim() === '') {
return isSync ? content : callback(null, content);
callback(null, content);
return;
}

// opt.outputStyle
Expand Down Expand Up @@ -254,18 +210,6 @@ module.exports = function (content) {
sassOptions.includePaths.push(path.dirname(resourcePath));

// start the actual rendering
if (isSync) {
try {
result = sass.renderSync(sassOptions);
addIncludedFilesToWebpack(result.stats.includedFiles);
return result.css.toString();
} catch (err) {
formatSassError(err);
err.file && this.dependency(err.file);
throw err;
}
}

asyncSassJobQueue.push(sassOptions, function onRender(err, result) {
if (err) {
formatSassError(err);
Expand Down
4 changes: 2 additions & 2 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
"devDependencies": {
"bootstrap-sass": "^3.3.5",
"css-loader": "^0.24.0",
"enhanced-require": "^0.5.0-beta6",
"file-loader": "^0.9.0",
"jshint": "^2.9.2",
"mocha": "^3.0.2",
Expand All @@ -47,6 +46,7 @@
"should": "^11.1.0",
"style-loader": "^0.13.1",
"webpack": "^1.13.1",
"webpack-dev-server": "^1.7.0"
"webpack-dev-server": "^1.7.0",
"webpack-merge": "^2.0.0"
}
}
128 changes: 71 additions & 57 deletions test/index.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ var should = require('should');
var path = require('path');
var webpack = require('webpack');
var fs = require('fs');
var enhancedReqFactory = require('enhanced-require');
var merge = require('webpack-merge');
var customImporter = require('./tools/customImporter.js');
var customFunctions = require('./tools/customFunctions.js');
var pathToSassLoader = require.resolve('../index.js');
var sassLoader = require(pathToSassLoader);

var CR = /\r/g;
var syntaxStyles = ['scss', 'sass'];
var pathToSassLoader = path.resolve(__dirname, '../index.js');
var pathToErrorFileNotFound = path.resolve(__dirname, './scss/error-file-not-found.scss');
var pathToErrorFileNotFound2 = path.resolve(__dirname, './scss/error-file-not-found-2.scss');
var pathToErrorFile = path.resolve(__dirname, './scss/error.scss');
Expand All @@ -29,7 +30,8 @@ describe('sass-loader', function () {

describe('config', function () {

it('should override sassLoader config with loader query', function () {
// Will be removed with webpack 2 support
it.skip('should override sassLoader config with loader query', function () {
var expectedCss = readCss('sass', 'language');
var webpackConfig = Object.assign({}, {
entry: 'raw!' + pathToSassLoader + '?indentedSyntax!' + path.join(__dirname, 'sass', 'language.sass'),
Expand All @@ -41,7 +43,7 @@ describe('sass-loader', function () {
var enhancedReq;
var actualCss;

enhancedReq = enhancedReqFactory(module, webpackConfig);
//enhancedReq = enhancedReqFactory(module, webpackConfig);
actualCss = enhancedReq(webpackConfig.entry);

fs.writeFileSync(__dirname + '/output/should override sassLoader config with loader query.sass.sync.css', actualCss, 'utf8');
Expand Down Expand Up @@ -144,59 +146,70 @@ describe('sass-loader', function () {
});

describe('errors', function () {

it('should output understandable errors in entry files', function () {
try {
enhancedReqFactory(module)(pathToSassLoader + '!' + pathToErrorFile);

it('should throw an error in synchronous loader environments', function () {
try {
sassLoader.call({
async: Function.prototype
}, '');
} catch (err) {
// check for file excerpt
err.message.should.equal('Synchronous compilation is not supported anymore. See https://github.com/jtangelder/sass-loader/issues/333');
}
});

it('should output understandable errors in entry files', function (done) {
runWebpack({
entry: pathToSassLoader + '!' + pathToErrorFile
}, function (err) {
err.message.should.match(/\.syntax-error''/);
err.message.should.match(/Invalid CSS after/);
err.message.should.match(/\(line 1, column 14\)/);
err.message.indexOf(pathToErrorFile).should.not.equal(-1);
}
done();
});
});

it('should output understandable errors of imported files', function () {
try {
enhancedReqFactory(module)(pathToSassLoader + '!' + pathToErrorImport);
} catch (err) {
it('should output understandable errors of imported files', function (done) {
runWebpack({
entry: pathToSassLoader + '!' + pathToErrorImport
}, function (err) {
// check for file excerpt
err.message.should.match(/\.syntax-error''/);
err.message.should.match(/Invalid CSS after "\.syntax-error''": expected "\{", was ""/);
err.message.should.match(/\(line 1, column 14\)/);
err.message.indexOf(pathToErrorFile).should.not.equal(-1);
}
done();
});
});

it('should output understandable errors when a file could not be found', function () {
try {
enhancedReqFactory(module)(pathToSassLoader + '!' + pathToErrorFileNotFound);
} catch (err) {
// check for file excerpt
it('should output understandable errors when a file could not be found', function (done) {
runWebpack({
entry: pathToSassLoader + '!' + pathToErrorFileNotFound
}, function (err) {
err.message.should.match(/@import "does-not-exist";/);
err.message.should.match(/File to import not found or unreadable: does-not-exist/);
err.message.should.match(/\(line 1, column 1\)/);
err.message.indexOf(pathToErrorFileNotFound).should.not.equal(-1);
}
done();
});
});

it('should not auto-resolve imports with explicit file names', function () {
try {
enhancedReqFactory(module)(pathToSassLoader + '!' + pathToErrorFileNotFound2);
} catch (err) {
// check for file excerpt
it('should not auto-resolve imports with explicit file names', function (done) {
runWebpack({
entry: pathToSassLoader + '!' + pathToErrorFileNotFound2
}, function (err) {
err.message.should.match(/@import "\.\/another\/_module\.scss";/);
err.message.should.match(/File to import not found or unreadable: \.\/another\/_module\.scss/);
err.message.should.match(/\(line 1, column 1\)/);
err.message.indexOf(pathToErrorFileNotFound2).should.not.equal(-1);
}
done();
});
});

});
});


function readCss(ext, id) {
return fs.readFileSync(path.join(__dirname, ext, 'spec', id + '.css'), 'utf8').replace(CR, '');
}
Expand All @@ -206,26 +219,20 @@ function testAsync(name, id, config) {
it(name + ' (' + ext + ')', function (done) {
var expectedCss = readCss(ext, id);
var sassFile = pathToSassFile(ext, id);
var webpackConfig = Object.assign(config ? config(ext) : {}, {
var baseConfig = merge({
entry: sassFile,
output: {
path: __dirname + '/output',
filename: 'bundle.' + ext + '.js',
libraryTarget: 'commonjs2'
filename: 'bundle.' + ext + '.js'
}
});
}, config ? config(ext) : {});
var actualCss;

webpack(webpackConfig, function onCompilationFinished(err, stats) {
runWebpack(baseConfig, function (err) {
if (err) {
return done(err);
}
if (stats.hasErrors()) {
return done(stats.compilation.errors[0]);
}
if (stats.hasWarnings()) {
return done(stats.compilation.warnings[0]);
done(err);
return;
}

delete require.cache[path.resolve(__dirname, './output/bundle.' + ext + '.js')];

actualCss = require('./output/bundle.' + ext + '.js');
Expand All @@ -239,23 +246,30 @@ function testAsync(name, id, config) {
});
}

function testSync(name, id, config) {
syntaxStyles.forEach(function forEachSyntaxStyle(ext) {
it(name + ' (' + ext + ')', function () {
var expectedCss = readCss(ext, id);
var sassFile = pathToSassFile(ext, id);
var webpackConfig = Object.assign(config ? config(ext) : {}, {
entry: sassFile
});
var enhancedReq;
var actualCss;

enhancedReq = enhancedReqFactory(module, webpackConfig);
actualCss = enhancedReq(webpackConfig.entry);
function testSync() {

}

fs.writeFileSync(__dirname + '/output/' + name + '.' + ext + '.sync.css', actualCss, 'utf8');
actualCss.should.eql(expectedCss);
});
function runWebpack(baseConfig, done) {
var webpackConfig = merge({
output: {
path: __dirname + '/output',
filename: 'bundle.js',
libraryTarget: 'commonjs2'
}
}, baseConfig);

webpack(webpackConfig, function onCompilationFinished(err, stats) {
if (err) {
return done(err);
}
if (stats.hasErrors()) {
return done(stats.compilation.errors[0]);
}
if (stats.hasWarnings()) {
return done(stats.compilation.warnings[0]);
}
done();
});
}

Expand Down

0 comments on commit b68db45

Please sign in to comment.