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.

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

Feat/new resolver #169

Merged
merged 7 commits into from
Mar 18, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 14 additions & 4 deletions src/createWebpackLessPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ const less = require('less');
const loaderUtils = require('loader-utils');
const pify = require('pify');

const stringifyLoader = require.resolve('./stringifyLoader.js');
const trailingSlash = /[\\/]$/;
const isLessCompatible = /\.(le|c)ss$/;

/**
* Creates a Less plugin that uses webpack's resolving engine that is provided by the loaderContext.
Expand All @@ -11,9 +13,11 @@ const trailingSlash = /[\\/]$/;
* @param {string=} root
* @returns {LessPlugin}
*/
function createWebpackLessPlugin(loaderContext, root) {
function createWebpackLessPlugin(loaderContext) {
const { fs } = loaderContext;
const resolve = pify(loaderContext.resolve.bind(loaderContext));
const loadModule = pify(loaderContext.loadModule.bind(loaderContext));
const readFile = pify(fs.readFile.bind(fs));

class WebpackFileManager extends less.FileManager {
supports(/* filename, currentDirectory, options, environment */) { // eslint-disable-line class-methods-use-this
Expand All @@ -22,7 +26,7 @@ function createWebpackLessPlugin(loaderContext, root) {
}

loadFile(filename, currentDirectory /* , options, environment */) { // eslint-disable-line class-methods-use-this
const moduleRequest = loaderUtils.urlToRequest(filename, root);
const moduleRequest = loaderUtils.urlToRequest(filename);
// Less is giving us trailing slashes, but the context should have no trailing slash
const context = currentDirectory.replace(trailingSlash, '');
let resolvedFilename;
Expand All @@ -32,11 +36,17 @@ function createWebpackLessPlugin(loaderContext, root) {
resolvedFilename = f;
loaderContext.addDependency(resolvedFilename);

return loadModule(`-!${__dirname}/stringify.loader.js!${resolvedFilename}`);
if (isLessCompatible.test(resolvedFilename)) {
return readFile(resolvedFilename)
.then(contents => contents.toString('utf8'));
}

return loadModule([stringifyLoader, resolvedFilename].join('!'))
.then(JSON.parse);
})
.then((contents) => {
return {
contents: JSON.parse(contents),
contents,
filename: resolvedFilename,
};
});
Expand Down
7 changes: 5 additions & 2 deletions src/getOptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ function getOptions(loaderContext) {
// We need to set the filename because otherwise our WebpackFileManager will receive an undefined path for the entry
options.filename = loaderContext.resource;

// It's safe to mutate the array now because it has already been cloned
options.plugins.push(createWebpackLessPlugin(loaderContext, options.root));
// When no paths are given, we use the webpack resolver
if ('paths' in options === false) {
// It's safe to mutate the array now because it has already been cloned
options.plugins.push(createWebpackLessPlugin(loaderContext));
}

return options;
}
Expand Down
10 changes: 0 additions & 10 deletions src/stringify.loader.js

This file was deleted.

12 changes: 12 additions & 0 deletions src/stringifyLoader.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* The stringifyLoader turns any content into a valid JavaScript string. This allows us to load any content
* with loaderContext.loadModule() without webpack complaining about non-JS modules.
*
* @param {string} content
* @return {string}
*/
function stringifyLoader(content) {
return JSON.stringify(content);
}

module.exports = stringifyLoader;
3 changes: 3 additions & 0 deletions test/fixtures/less/error-mixed-resolvers.less
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// You can't use include paths and webpack's resolver simulatenously.
@import "some/module.less";
@import "~some/module.less";
Empty file.
1 change: 1 addition & 0 deletions test/fixtures/less/import-non-less.less
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import "folder/some.file";
3 changes: 3 additions & 0 deletions test/fixtures/less/import-paths.less
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// some/module.less is intended to be loaded from node_modules if the folder is configured as include path.
// webpack would expect this import to be prepended with a ~ character.
@import "some/module.less";
File renamed without changes.
File renamed without changes.
29 changes: 3 additions & 26 deletions test/helpers/compile.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
const path = require('path');
const webpack = require('webpack');

const lessLoader = require.resolve('../../src');
const helperLoader = require.resolve('./helperLoader.js');
const fixturePath = path.resolve(__dirname, '..', 'fixtures');
const outputPath = path.resolve(__dirname, '..', 'output');

function compile(fixture, loaderOptions, loaderContext) {
function compile(fixture, moduleRules) {
return new Promise((resolve, reject) => {
const entry = path.resolve(fixturePath, 'less', `${fixture}.less`);
let inspect;

webpack({
entry,
Expand All @@ -18,24 +15,7 @@ function compile(fixture, loaderOptions, loaderContext) {
filename: 'bundle', // omitting the js extension to prevent jest's watcher from triggering
},
module: {
rules: [{
test: /\.less$/,
loaders: [{
loader: helperLoader,
options: loaderContext,
}, {
loader: 'inspect-loader',
options: {
callback(i) {
inspect = i;
},
},
},
{
loader: lessLoader,
options: loaderOptions,
}],
}],
rules: moduleRules,
},
}, (err, stats) => {
const problem = err || stats.compilation.errors[0] || stats.compilation.warnings[0];
Expand All @@ -52,10 +32,7 @@ function compile(fixture, loaderOptions, loaderContext) {
return;
}

resolve({
inspect,
stats,
});
resolve(stats);
});
});
}
Expand Down
55 changes: 28 additions & 27 deletions test/helpers/createSpec.js
Original file line number Diff line number Diff line change
@@ -1,58 +1,61 @@


const { exec } = require('child_process');
const fs = require('fs');
const path = require('path');
const removeSourceMappingUrl = require('../../src/removeSourceMappingUrl');

const projectPath = path.resolve(__dirname, '..', '..');
const lessFixtures = path.resolve(__dirname, '..', 'fixtures', 'less');
const cssFixtures = path.resolve(__dirname, '..', 'fixtures', 'css');
const matchWebpackImports = /(@import\s+(\([^)]+\))?\s*["'])~/g;
const fixturesPath = path.resolve(projectPath, 'test', 'fixtures');
const lessFixturesPath = path.resolve(fixturesPath, 'less');
const cssFixturesPath = path.resolve(fixturesPath, 'css');
const lessBin = require.resolve('.bin/lessc');
const ignore = [
'error',
'import-non-less',
'error-import-not-existing',
'error-mixed-resolvers',
];
const lessReplacements = [
[/~some\//g, '../node_modules/some/'],
];
const cssReplacements = [
[/\.\.\/node_modules\/some\//g, '~some/'],
];
/**
* This object specifies the replacements for the ~-character per test.
*
* Since less doesn't understand the semantics of ~, we need to replace the
* import path with a relative path.
*
* The object keys are the test ids.
*/
const tildeReplacements = {
imports: '../node_modules/',
'imports-node': '../node_modules/',
'source-map': '../node_modules/',
};
// Maps test ids on cli arguments
const lessOptions = {
'source-map': [
'--source-map',
`--source-map-basepath=${projectPath}`,
`--source-map-rootpath=${projectPath}`,
],
'import-paths': [
`--include-path=${path.resolve(fixturesPath, 'node_modules')}`,
],
};
const testIds = fs.readdirSync(lessFixtures)
const testIds = fs.readdirSync(lessFixturesPath)
.filter(name =>
path.extname(name) === '.less' && ignore.indexOf(path.basename(name, '.less')) === -1,
)
.map(name =>
path.basename(name, '.less'),
);

function replace(content, replacements) {
return replacements.reduce(
(intermediate, [pattern, replacement]) => intermediate.replace(pattern, replacement),
content,
);
}

testIds
.forEach((testId) => {
const lessFile = path.resolve(lessFixtures, `${testId}.less`);
const cssFile = path.resolve(cssFixtures, `${testId}.css`);
const tildeReplacement = tildeReplacements[testId];
const lessFile = path.resolve(lessFixturesPath, `${testId}.less`);
const cssFile = path.resolve(cssFixturesPath, `${testId}.css`);
const originalLessContent = fs.readFileSync(lessFile, 'utf8');
const replacedLessContent = replace(originalLessContent, lessReplacements);

// It's safer to change the file and write it back to disk instead of piping it to the Less process
// because Less tends to create broken paths in url() statements and source maps when the content is read from stdin
// See also https://github.com/less/less.js/issues/3038
fs.writeFileSync(lessFile, originalLessContent.replace(matchWebpackImports, `$1${tildeReplacement}`), 'utf8');
fs.writeFileSync(lessFile, replacedLessContent, 'utf8');

exec(
[lessBin, '--relative-urls', ...lessOptions[testId] || '', lessFile, cssFile].join(' '),
Expand All @@ -65,9 +68,7 @@ testIds
// We remove the source mapping url because the less-loader will do it also.
// See removeSourceMappingUrl.js for the reasoning behind this.
const cssContent = removeSourceMappingUrl(
fs.readFileSync(cssFile, 'utf8')
// Change back tilde replacements
.replace(new RegExp(`(@import\\s+["'])${tildeReplacement}`, 'g'), '$1~'),
replace(fs.readFileSync(cssFile, 'utf8'), cssReplacements),
);

fs.writeFileSync(lessFile, originalLessContent, 'utf8');
Expand Down
46 changes: 46 additions & 0 deletions test/helpers/moduleRules.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
const lessLoader = require.resolve('../../src');
const helperLoader = require.resolve('./helperLoader.js');
const someFileLoader = require.resolve('./someFileLoader.js');

function basic(lessLoaderOptions, lessLoaderContext = {}, inspectCallback = () => {}) {
return [{
test: /\.less$/,
loaders: [{
loader: helperLoader,
options: lessLoaderContext,
}, {
loader: 'inspect-loader',
options: {
callback: inspectCallback,
},
},
{
loader: lessLoader,
options: lessLoaderOptions,
}],
}];
}

function nonLessImport(inspectCallback) {
return [{
test: /\.less$/,
loaders: [{
loader: helperLoader,
}, {
loader: 'inspect-loader',
options: {
callback: inspectCallback,
},
}, {
loader: lessLoader,
}],
}, {
test: /some\.file$/,
loaders: [{
loader: someFileLoader,
}],
}];
}

exports.basic = basic;
exports.nonLessImport = nonLessImport;
5 changes: 5 additions & 0 deletions test/helpers/someFileLoader.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function someFileLoader() {
return '.some-file { background: hotpink; }';
}

module.exports = someFileLoader;
Loading