Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove synchronous compilation support #334

Merged
merged 1 commit into from
Dec 26, 2016
Merged

Remove synchronous compilation support #334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 8 additions & 64 deletions index.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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