From df40f0573ebe401d945c2aa3d687700b00ee0ef0 Mon Sep 17 00:00:00 2001 From: Johannes Ewald Date: Tue, 7 Mar 2017 18:55:00 +0100 Subject: [PATCH 1/2] Update to webpack 2 This commit only takes the minimal actions that are required to update to webpack 2. The tests (and probably the loader itself) require more refactoring, but there's currently little time for that. Also removed the synchronous compilation since it has been deprecated for a while: https://github.com/webpack-contrib/less-loader/issues/84 --- .editorconfig | 2 +- index.js | 80 +++++++---------------------- package.json | 18 ++++--- test/index.test.js | 86 ++++++++++++++++---------------- test/sourceMap/entry.js | 3 -- test/sourceMap/index.html | 11 ---- test/sourceMap/webpack.config.js | 29 ----------- 7 files changed, 72 insertions(+), 157 deletions(-) delete mode 100644 test/sourceMap/entry.js delete mode 100644 test/sourceMap/index.html delete mode 100644 test/sourceMap/webpack.config.js diff --git a/.editorconfig b/.editorconfig index b14fd268..4d3466a5 100644 --- a/.editorconfig +++ b/.editorconfig @@ -9,7 +9,7 @@ charset = utf-8 end_of_line = lf insert_final_newline = true indent_style = tab -indent_size = 2 +indent_size = 4 # Matches the exact files either package.json or .travis.yml [{package.json,.travis.yml}] diff --git a/index.js b/index.js index d55e5eab..47037b84 100644 --- a/index.js +++ b/index.js @@ -6,60 +6,43 @@ Johannes Ewald @jhnns */ var less = require("less"); -var fs = require("fs"); var loaderUtils = require("loader-utils"); -var path = require("path"); -var util = require("util"); +var cloneDeep = require("clone-deep"); var trailingSlash = /[\\\/]$/; module.exports = function(source) { var loaderContext = this; - var query = loaderUtils.getOptions(this) || {}; + var options = Object.assign( + { + filename: this.resource, + paths: [], + plugins: [], + relativeUrls: true, + compress: Boolean(this.minimize) + }, + cloneDeep(loaderUtils.getOptions(this)) + ); var cb = this.async(); - var isSync = typeof cb !== "function"; var finalCb = cb || this.callback; - var configKey = query.config || "lessLoader"; - var config = { - filename: this.resource, - paths: [], - relativeUrls: true, - compress: !!this.minimize - }; var webpackPlugin = { install: function(less, pluginManager) { - var WebpackFileManager = getWebpackFileManager(less, loaderContext, query, isSync); + var WebpackFileManager = getWebpackFileManager(less, loaderContext, options); pluginManager.addFileManager(new WebpackFileManager()); }, minVersion: [2, 1, 1] }; - this.cacheable && this.cacheable(); - - Object.keys(query).forEach(function(attr) { - config[attr] = query[attr]; - }); - - // Now we're adding the webpack plugin, because there might have - // been added some before via query-options. - config.plugins = config.plugins || []; - config.plugins.push(webpackPlugin); - - // If present, add custom LESS plugins. - if (this.options[configKey]) { - config.plugins = config.plugins.concat(this.options[configKey].lessPlugins || []); - } + options.plugins.push(webpackPlugin); - // not using the `this.sourceMap` flag because css source maps are different - // @see https://github.com/webpack/css-loader/pull/40 - if (query.sourceMap) { - config.sourceMap = { + if (options.sourceMap) { + options.sourceMap = { outputSourceFiles: true }; } - less.render(source, config, function(e, result) { + less.render(source, options, function(e, result) { var cb = finalCb; // Less is giving us double callbacks sometimes :( // Thus we need to mark the callback as "has been called" @@ -85,23 +68,10 @@ function getWebpackFileManager(less, loaderContext, query, isSync) { }; WebpackFileManager.prototype.supportsSync = function(filename, currentDirectory, options, environment) { - return isSync; + return false; }; WebpackFileManager.prototype.loadFile = function(filename, currentDirectory, options, environment, callback) { - // Unfortunately we don't have any influence on less to call `loadFile` or `loadFileSync` - // thus we need to decide for ourselves. - // @see https://github.com/less/less.js/issues/2325 - if (isSync) { - try { - callback(null, this.loadFileSync(filename, currentDirectory, options, environment)); - } catch (err) { - callback(err); - } - - return; - } - var moduleRequest = loaderUtils.urlToRequest(filename, query.root); // Less is giving us trailing slashes, but the context should have no trailing slash var context = currentDirectory.replace(trailingSlash, ""); @@ -128,22 +98,6 @@ function getWebpackFileManager(less, loaderContext, query, isSync) { }); }; - WebpackFileManager.prototype.loadFileSync = util.deprecate(function(filename, currentDirectory, options, environment) { - var moduleRequest = loaderUtils.urlToRequest(filename, query.root); - // Less is giving us trailing slashes, but the context should have no trailing slash - var context = currentDirectory.replace(trailingSlash, ""); - var data; - - filename = loaderContext.resolveSync(context, moduleRequest); - loaderContext.dependency && loaderContext.dependency(filename); - data = fs.readFileSync(filename, "utf8"); - - return { - contents: data, - filename: filename - }; - }, "We are planing to remove enhanced-require support with the next major release of the less-loader: https://github.com/webpack/less-loader/issues/84"); - return WebpackFileManager; } diff --git a/package.json b/package.json index c688230c..cb7e1490 100644 --- a/package.json +++ b/package.json @@ -5,21 +5,22 @@ "description": "less loader module for webpack", "scripts": { "test": "npm run travis:test", - "travis:test": "node --no-deprecation node_modules/.bin/_mocha -R spec", - "test-source-map": "webpack --config test/sourceMap/webpack.config.js && open ./test/sourceMap/index.html" + "travis:test": "mocha -R spec" + }, + "engines": { + "node": ">=4.0.0" }, "peerDependencies": { "less": "^2.3.1" }, "devDependencies": { - "css-loader": "^0.23.1", - "enhanced-require": "^0.5.0-beta6", - "extract-text-webpack-plugin": "^1.0.1", + "css-loader": "^0.26.2", + "extract-text-webpack-plugin": "^2.1.0", "less": "^2.6.1", - "mocha": "^2.4.5", + "mocha": "^3.2.0", "raw-loader": "^0.5.1", - "should": "^8.3.0", - "webpack": "^1.12.14" + "should": "^11.2.0", + "webpack": "^2.2.1" }, "repository": { "type": "git", @@ -32,6 +33,7 @@ } ], "dependencies": { + "clone-deep": "^0.2.4", "loader-utils": "^1.0.2" } } diff --git a/test/index.test.js b/test/index.test.js index 7858cd83..13883ed0 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -3,7 +3,6 @@ var should = require("should"); var path = require("path"); var webpack = require("webpack"); -var enhancedReqFactory = require("enhanced-require"); var fs = require("fs"); var moveModulesDir = require("./helpers/moveModulesDir.js"); @@ -17,7 +16,7 @@ describe("less-loader", function() { test("should resolve all imports", "imports"); test("should resolve all imports from bower_components", "imports-bower", { before: function(config) { - config.resolve.root.push(bowerComponents); + config.resolve.modules.push(bowerComponents); } }); test("should resolve all imports from node_modules", "imports-node", { @@ -34,16 +33,29 @@ describe("less-loader", function() { test("should transform urls to files above the current directory", "folder/url-path"); test("should transform urls to files above the sibling directory", "folder2/url-path"); test("should generate source-map", "source-map", { - query: "?sourceMap", + options: { + sourceMap: true + }, devtool: "source-map" }); test("should install plugins", "url-path", { - query: "?config=lessLoaderTest", - lessPlugins: [ - { wasCalled: false, install: function() {this.wasCalled = true;} } - ], + options: { + lessPlugins: [ + { wasCalled: false, install: function() {this.wasCalled = true;} } + ] + }, after: function(testVariables) { - this.lessPlugins[0].wasCalled.should.be.true; + this.options.lessPlugins[0].wasCalled.should.be.true; + } + }); + // See https://github.com/webpack/loader-utils/issues/56 + test("should not alter the original options object", "basic", { + options: { + lessPlugins: [] + }, + after: function() { + // We know that the loader will add its own plugin, but it should not alter the original array + this.options.lessPlugins.should.have.length(0); } }); it("should report error correctly", function(done) { @@ -51,7 +63,7 @@ describe("less-loader", function() { entry: path.resolve(__dirname, "../index.js") + "!" + path.resolve(__dirname, "./less/error.less"), output: { - path: __dirname + "/output", + path: path.resolve(__dirname, "output"), filename: "bundle.js" } }, function(err, stats) { @@ -70,58 +82,48 @@ function readCss(id) { return fs.readFileSync(path.resolve(__dirname, "./css/" + id + ".css") ,"utf8").replace(CR, ""); } -function tryMkdirSync(dirname) { - try { - fs.mkdirSync(dirname); - } catch(e) { - if (!e || e.code !== "EEXIST") - throw e; - } -} - function test(name, id, testOptions) { testOptions = testOptions || {}; - testOptions.query = testOptions.query || ""; + testOptions.options = testOptions.options || ""; it(name, function (done) { var expectedCss = readCss(id); - var lessFile = "raw!" + - pathToLessLoader + testOptions.query + "!" + - path.resolve(__dirname, "./less/" + id + ".less"); + var lessFile = path.resolve(__dirname, "./less/" + id + ".less"); var actualCss; var config = { resolve: { - root: [ - __dirname + modules: [ + "node_modules" ] } }; - var enhancedReq; testOptions.before && testOptions.before(config); - enhancedReq = enhancedReqFactory(module, config); - - // run synchronously - actualCss = enhancedReq(lessFile); - // writing the actual css to output-dir for better debugging - tryMkdirSync(__dirname + "/output/"); - fs.writeFileSync(__dirname + "/output/" + name + ".sync.css", actualCss, "utf8"); - - actualCss.should.eql(expectedCss); - // run asynchronously webpack({ entry: lessFile, + context: __dirname, devtool: testOptions.devtool, resolve: config.resolve, output: { - path: __dirname + "/output", + path: path.resolve(__dirname, "output"), filename: "bundle.js", libraryTarget: "commonjs2" }, - lessLoaderTest: { - lessPlugins: testOptions.lessPlugins || [] + module: { + rules: [ + { + test: /\.less$/, + use: [ + "raw-loader", + { + loader: pathToLessLoader, + options: testOptions.options + } + ] + } + ] } }, function onCompilationFinished(err, stats) { var actualMap; @@ -139,16 +141,16 @@ function test(name, id, testOptions) { actualCss = require("./output/bundle.js"); // writing the actual css to output-dir for better debugging - fs.writeFileSync(__dirname + "/output/" + name + ".async.css", actualCss, "utf8"); + fs.writeFileSync(path.resolve(__dirname, "output", name + ".async.css"), actualCss, "utf8"); actualCss.should.eql(expectedCss); testOptions.after && testOptions.after(); if (testOptions.devtool === "source-map") { - actualMap = fs.readFileSync(__dirname + "/output/bundle.js.map", "utf8"); - fs.writeFileSync(__dirname + "/output/" + name + ".sync.css.map", actualMap, "utf8"); + actualMap = fs.readFileSync(path.resolve(__dirname, "output", "bundle.js.map"), "utf8"); + fs.writeFileSync(path.resolve(__dirname, "output", name + ".sync.css.map"), actualMap, "utf8"); actualMap = JSON.parse(actualMap); - actualMap.sources.should.containEql("webpack:///./test/less/" + id + ".less"); + actualMap.sources.should.containEql("webpack:///./less/" + id + ".less"); } done(); diff --git a/test/sourceMap/entry.js b/test/sourceMap/entry.js deleted file mode 100644 index eebb8f4e..00000000 --- a/test/sourceMap/entry.js +++ /dev/null @@ -1,3 +0,0 @@ -"use strict"; - -require("../less/source-map.less"); diff --git a/test/sourceMap/index.html b/test/sourceMap/index.html deleted file mode 100644 index 93b860c3..00000000 --- a/test/sourceMap/index.html +++ /dev/null @@ -1,11 +0,0 @@ - - - - - - - - -

Open the developer tools, dude!

- - diff --git a/test/sourceMap/webpack.config.js b/test/sourceMap/webpack.config.js deleted file mode 100644 index 0de0d1dd..00000000 --- a/test/sourceMap/webpack.config.js +++ /dev/null @@ -1,29 +0,0 @@ -"use strict"; - -var path = require("path"); -var ExtractTextPlugin = require("extract-text-webpack-plugin"); - -var pathToLessLoader = path.resolve(__dirname, "../../index.js"); - -module.exports = { - entry: path.resolve(__dirname, "./entry.js"), - output: { - path: path.resolve(__dirname, "../output"), - filename: "bundle.sourcemap.js" - }, - devtool: "inline-source-map", - module: { - loaders: [ - { - test: /\.less/, - loader: ExtractTextPlugin.extract( - "css-loader?sourceMap!" + - pathToLessLoader + "?sourceMap" - ) - } - ] - }, - plugins: [ - new ExtractTextPlugin("styles.css") - ] -}; From 40aa340fdc199c7a7feadbfb52d79db8d8f3086b Mon Sep 17 00:00:00 2001 From: Johannes Ewald Date: Tue, 7 Mar 2017 19:13:29 +0100 Subject: [PATCH 2/2] Add error when a sync compiliation is detected --- index.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/index.js b/index.js index 47037b84..e4d67c03 100644 --- a/index.js +++ b/index.js @@ -24,6 +24,7 @@ module.exports = function(source) { cloneDeep(loaderUtils.getOptions(this)) ); var cb = this.async(); + var isSync = typeof cb !== "function"; var finalCb = cb || this.callback; var webpackPlugin = { install: function(less, pluginManager) { @@ -34,6 +35,10 @@ module.exports = function(source) { minVersion: [2, 1, 1] }; + if (isSync) { + throw new Error("Synchronous compilation is not supported anymore. See https://github.com/webpack-contrib/less-loader/issues/84"); + } + options.plugins.push(webpackPlugin); if (options.sourceMap) {