Merge pull request #169 from webpack-contrib/feat/new-resolver

Feat/new resolver

src/createWebpackLessPlugin.js

@@ -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.
@@ -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
@@ -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;
@@ -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,
           };
         });

src/getOptions.js

@@ -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;
 }

src/stringifyLoader.js

@@ -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;

test/fixtures/less/error-mixed-resolvers.less

@@ -0,0 +1,3 @@
+// You can't use include paths and webpack's resolver simulatenously.
+@import "some/module.less";
+@import "~some/module.less";

test/fixtures/less/import-non-less.less

@@ -0,0 +1 @@
+@import "folder/some.file";

test/fixtures/less/import-paths.less

@@ -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";

test/helpers/compile.js

@@ -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,
@@ -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];
@@ -52,10 +32,7 @@ function compile(fixture, loaderOptions, loaderContext) {
         return;
       }
 
-      resolve({
-        inspect,
-        stats,
-      });
+      resolve(stats);
     });
   });
 }

test/helpers/createSpec.js

@@ -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(' '),
@@ -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');

test/helpers/moduleRules.js

@@ -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;

test/helpers/someFileLoader.js

@@ -0,0 +1,5 @@
+function someFileLoader() {
+  return '.some-file { background: hotpink; }';
+}
+
+module.exports = someFileLoader;