Use default webpack module resolution (#926)

Since the custom module resolution is not required, and has been the
cause of at least 3 bugs that I have spent hours debugging.

The vast majority of modules referenced by Neutrino already use
`require.resolve()` and so are unaffected by this change. Those that
do not (for example eslint presets, since eslint doesn't support
specifying them as full paths), were not helped by this custom
module resolution anyway, since the resolution happens outside of
webpack, and so relied on hoisting even before this change.

The `node_modules` option has been removed in favour of using the
Neutrino API (or else `NODE_PATH`). Customising module resolution
is a massive footgun and should not be used in most cases.

Fixes #822.

docs/api.md

@@ -163,24 +163,6 @@ Neutrino({
 })
 ```
 
-### `options.node_modules`
-
-Set the directory which contains the Node.js modules of the project. If the option is not set, Neutrino defaults it to
-`node_modules`. If a relative path is specified, it will be resolved relative to `options.root`; absolute paths will be
-used as-is.
-
-```js
-Neutrino({
-  // if not specified, defaults to options.root + node_modules
-
-  // relative, resolves to options.root + modules
-  node_modules: 'modules',
-
-  // absolute
-  node_modules: '/code/website/modules'
-})
-```
-
 ## Other options
 
 ### `options.debug`

docs/creating-presets.md

@@ -306,28 +306,6 @@ module.exports = {
 };
 ```
 
-### `options.node_modules`
-
-Set the directory which contains the Node.js modules of the project. If the option is not set, Neutrino defaults it to
-`node_modules`. If a relative path is specified, it will be resolved relative to `options.root`; absolute paths will be
-used as-is.
-
-```js
-module.exports = neutrino => {
-  // if not specified, defaults to options.root + node_modules
-  neutrino.options.node_modules;
-};
-
-module.exports = {
-  options: {
-    // relative, resolves to options.root + modules
-    node_modules: 'modules',
-    // absolute
-    node_modules: '/code/website/modules'
-  }
-};
-```
-
 ### `options.extensions`
 
 Set the preferred list of module extensions to inform interested middleware. If the option is not set,

docs/customization.md

@@ -142,23 +142,6 @@ module.exports = {
 };
 ```
 
-### `options.node_modules`
-
-Set the directory which contains the Node.js modules of the project. If the option is not set, Neutrino defaults it to
-`node_modules`. If a relative path is specified, it will be resolved relative to `options.root`; absolute paths will be
-used as-is.
-
-```js
-module.exports = {
-  options: {
-    // Override to relative directory, resolves to options.root + modules
-    node_modules: 'modules',
-    // Override to absolute directory
-    node_modules: '/code/website/modules'
-  }
-};
-```
-
 ## Mutating `neutrino.options`
 
 While it is possible to mutate `neutrino.options` directly, this should be avoided.

packages/eslint/index.js

@@ -1,9 +1,6 @@
 const deepmerge = require('deepmerge');
 const clone = require('lodash.clonedeep');
 const omit = require('lodash.omit');
-const { join } = require('path');
-
-const MODULES = join(__dirname, 'node_modules');
 
 const merge = (source, destination) => {
   const sourceRules = (source && source.eslint && source.eslint.rules) || {};
@@ -120,8 +117,6 @@ module.exports = (neutrino, opts = {}) => {
     })
     : options.eslint;
 
-  neutrino.config.resolve.modules.add(MODULES);
-  neutrino.config.resolveLoader.modules.add(MODULES);
   neutrino.config
     .module
       .rule('lint')

packages/jest/src/index.js

@@ -71,14 +71,14 @@ module.exports = neutrino => {
       source,
       tests,
       root,
-      node_modules, // eslint-disable-line camelcase
       debug
     } = neutrino.options;
+    const modulesConfig = neutrino.config.resolve.modules.values();
     const aliases = neutrino.config.resolve.alias.entries() || {};
 
     return override({
       rootDir: root,
-      moduleDirectories: neutrino.config.resolve.modules.values(),
+      moduleDirectories: modulesConfig.length ? modulesConfig : ['node_modules'],
       moduleFileExtensions: neutrino.config.resolve.extensions
         .values()
         .map(extension => extension.replace('.', '')),
@@ -93,8 +93,6 @@ module.exports = neutrino => {
             [extensionsToNames(style)]: require.resolve('./style-mock')
           }),
       bail: true,
-      // eslint-disable-next-line camelcase
-      coveragePathIgnorePatterns: [node_modules],
       collectCoverageFrom: [join(
         relative(root, source),
         `**/*.{${extensions.join(',')}}`

packages/library/index.js

@@ -4,9 +4,6 @@ const clean = require('@neutrinojs/clean');
 const loaderMerge = require('@neutrinojs/loader-merge');
 const merge = require('deepmerge');
 const nodeExternals = require('webpack-node-externals');
-const { join } = require('path');
-
-const MODULES = join(__dirname, 'node_modules');
 
 module.exports = (neutrino, opts = {}) => {
   if (!opts.name) {
@@ -85,29 +82,10 @@ module.exports = (neutrino, opts = {}) => {
       .when(options.libraryTarget === 'umd', (output) => output.umdNamedDefine(true))
       .end()
     .resolve
-      .modules
-        .add('node_modules')
-        .add(neutrino.options.node_modules)
-        .add(MODULES)
-        .when(__dirname.includes('neutrino-dev'), modules => {
-          // Add monorepo node_modules to webpack module resolution
-          modules.add(join(__dirname, '../../node_modules'));
-        })
-        .end()
       .extensions
         .merge(neutrino.options.extensions.concat('json').map(ext => `.${ext}`))
         .end()
       .end()
-    .resolveLoader
-      .modules
-        .add(neutrino.options.node_modules)
-        .add(MODULES)
-        .when(__dirname.includes('neutrino-dev'), modules => {
-          // Add monorepo node_modules to webpack module resolution
-          modules.add(join(__dirname, '../../node_modules'));
-        })
-        .end()
-      .end()
     .node
       .when(options.target === 'node', (node) => {
         node

packages/neutrino/Neutrino.js

@@ -12,8 +12,7 @@ const pathOptions = [
   ['root', '', () => process.cwd()],
   ['source', 'src', getRoot],
   ['output', 'build', getRoot],
-  ['tests', 'test', getRoot],
-  ['node_modules', 'node_modules', getRoot]
+  ['tests', 'test', getRoot]
 ];
 const requireFromRoot = (moduleId, root) => {
   const paths = [
@@ -129,6 +128,10 @@ module.exports = class Neutrino {
         }
       });
 
+    if ('node_modules' in options) {
+      throw new Error('options.node_modules has been removed. Use `neutrino.config.resolve.modules` instead.');
+    }
+
     return options;
   }
 

packages/neutrino/test/api_test.js

@@ -77,16 +77,11 @@ test('options.tests', t => {
   t.is(api.options.tests, '/alpha');
 });
 
-test('options.node_modules', t => {
+test('throws when legacy options.node_modules is set', t => {
   const api = new Neutrino();
+  const options = { node_modules: 'abc' };
 
-  t.is(api.options.node_modules, join(process.cwd(), 'node_modules'));
-  api.options.node_modules = './alpha';
-  t.is(api.options.node_modules, join(process.cwd(), 'alpha'));
-  api.options.root = '/beta';
-  t.is(api.options.node_modules, join('/beta', 'alpha'));
-  api.options.node_modules = '/alpha';
-  t.is(api.options.node_modules, '/alpha');
+  t.throws(() => api.use({ options }), /options\.node_modules has been removed/);
 });
 
 test('options.mains', t => {

packages/node/index.js

@@ -5,12 +5,11 @@ const startServer = require('@neutrinojs/start-server');
 const hot = require('@neutrinojs/hot');
 const nodeExternals = require('webpack-node-externals');
 const {
-  basename, join, parse, format
+  basename, parse, format
 } = require('path');
 const merge = require('deepmerge');
 const omit = require('lodash.omit');
 
-const MODULES = join(__dirname, 'node_modules');
 const getOutputForEntry = entry => basename(
   format(
     merge(
@@ -78,29 +77,10 @@ module.exports = (neutrino, opts = {}) => {
       .chunkFilename('[id].[hash:5]-[chunkhash:7].js')
       .end()
     .resolve
-      .modules
-        .add('node_modules')
-        .add(neutrino.options.node_modules)
-        .add(MODULES)
-        .when(__dirname.includes('neutrino-dev'), modules => {
-          // Add monorepo node_modules to webpack module resolution
-          modules.add(join(__dirname, '../../node_modules'));
-        })
-        .end()
       .extensions
         .merge(neutrino.options.extensions.concat('json').map(ext => `.${ext}`))
         .end()
       .end()
-    .resolveLoader
-      .modules
-        .add(neutrino.options.node_modules)
-        .add(MODULES)
-        .when(__dirname.includes('neutrino-dev'), modules => {
-          // Add monorepo node_modules to webpack module resolution
-          modules.add(join(__dirname, '../../node_modules'));
-        })
-        .end()
-      .end()
     .when(neutrino.options.debug, (config) => {
       config.merge({
         stats: {

packages/preact/index.js

@@ -1,9 +1,6 @@
 const compileLoader = require('@neutrinojs/compile-loader');
 const loaderMerge = require('@neutrinojs/loader-merge');
 const web = require('@neutrinojs/web');
-const { join } = require('path');
-
-const MODULES = join(__dirname, 'node_modules');
 
 module.exports = (neutrino, opts = {}) => {
   const options = {
@@ -22,13 +19,7 @@ module.exports = (neutrino, opts = {}) => {
   neutrino.use(web, options);
 
   neutrino.config
-    .resolveLoader
-      .modules
-        .add(MODULES)
-        .end()
-      .end()
     .resolve
-      .modules.add(MODULES).end()
       .alias
         .set('react', 'preact-compat')
         .set('react-dom', 'preact-compat')

packages/react-components/index.js

@@ -5,8 +5,6 @@ const nodeExternals = require('webpack-node-externals');
 const { extname, join, basename } = require('path');
 const { readdirSync } = require('fs');
 
-const MODULES = join(__dirname, 'node_modules');
-
 module.exports = (neutrino, opts = {}) => {
   const mode = neutrino.config.get('mode');
   const options = merge({
@@ -18,19 +16,6 @@ module.exports = (neutrino, opts = {}) => {
     style: { extract: { plugin: { filename: '[name].css' } } }
   }, opts);
 
-  neutrino.config.resolve.modules
-    .add(MODULES)
-    .when(__dirname.includes('neutrino-dev'), modules => {
-      // Add monorepo node_modules to webpack module resolution
-      modules.add(join(__dirname, '../../node_modules'));
-    });
-  neutrino.config.resolveLoader.modules
-    .add(MODULES)
-    .when(__dirname.includes('neutrino-dev'), modules => {
-      // Add monorepo node_modules to webpack module resolution
-      modules.add(join(__dirname, '../../node_modules'));
-    });
-
   neutrino.config.when(
     mode === 'development',
     () => {

packages/react/index.js

@@ -1,11 +1,8 @@
 const web = require('@neutrinojs/web');
 const compileLoader = require('@neutrinojs/compile-loader');
 const loaderMerge = require('@neutrinojs/loader-merge');
-const { join } = require('path');
 const merge = require('deepmerge');
 
-const MODULES = join(__dirname, 'node_modules');
-
 module.exports = (neutrino, opts = {}) => {
   const mode = neutrino.config.get('mode');
   const options = merge({
@@ -53,14 +50,5 @@ module.exports = (neutrino, opts = {}) => {
     });
   });
 
-  neutrino.config
-    .resolve
-      .batch((resolve) => {
-        resolve.modules.add(MODULES);
-        resolve.alias.set('react-native', 'react-native-web');
-      })
-      .end()
-    .resolveLoader
-      .modules
-        .add(MODULES);
+  neutrino.config.resolve.alias.set('react-native', 'react-native-web');
 };

packages/vue/index.js

@@ -1,9 +1,6 @@
 const loaderMerge = require('@neutrinojs/loader-merge');
 const web = require('@neutrinojs/web');
 const merge = require('deepmerge');
-const path = require('path');
-
-const MODULES = path.join(__dirname, 'node_modules');
 
 module.exports = (neutrino, options = {}) => {
   neutrino.use(web, options);
@@ -56,7 +53,4 @@ module.exports = (neutrino, options = {}) => {
           ...args
         ]);
   }
-
-  neutrino.config.resolve.modules.add(MODULES);
-  neutrino.config.resolveLoader.modules.add(MODULES);
 };

packages/web/index.js

@@ -10,14 +10,11 @@ const clean = require('@neutrinojs/clean');
 const styleMinify = require('@neutrinojs/style-minify');
 const loaderMerge = require('@neutrinojs/loader-merge');
 const devServer = require('@neutrinojs/dev-server');
-const { join } = require('path');
 const { resolve } = require('url');
 const merge = require('deepmerge');
 const HtmlWebpackIncludeSiblingChunksPlugin = require('html-webpack-include-sibling-chunks-plugin');
 const ManifestPlugin = require('webpack-manifest-plugin');
 
-const MODULES = join(__dirname, 'node_modules');
-
 module.exports = (neutrino, opts = {}) => {
   const mode = neutrino.config.get('mode');
   const publicPath = opts.publicPath || './';
@@ -169,32 +166,10 @@ module.exports = (neutrino, opts = {}) => {
       .chunkFilename('[name].[chunkhash].js')
       .end()
     .resolve
-      .modules
-        .add('node_modules')
-        .add(neutrino.options.node_modules)
-        .add(MODULES)
-        .when(__dirname.includes('neutrino-dev'), modules => {
-          // Add monorepo node_modules to webpack module resolution
-          modules.add(join(__dirname, '../../node_modules'));
-          // Work around test failures when using Jest with Preact
-          // https://github.com/mozilla-neutrino/neutrino-dev/issues/822
-          modules.delete(neutrino.options.node_modules);
-        })
-        .end()
       .extensions
         .merge(neutrino.options.extensions.concat('json').map(ext => `.${ext}`))
         .end()
       .end()
-    .resolveLoader
-      .modules
-        .add(neutrino.options.node_modules)
-        .add(MODULES)
-        .when(__dirname.includes('neutrino-dev'), modules => {
-          // Add monorepo node_modules to webpack module resolution
-          modules.add(join(__dirname, '../../node_modules'));
-        })
-        .end()
-      .end()
     .node
       .set('Buffer', false)
       .set('fs', 'empty')