[resolvers/node] [fix] when "module" does not exist, fall back to "main"

Fixes #2186. This is actually exposing a bug with packages that are broken - that ship an invalid "module" field - but it‘s a more friendly and node-accurate behavior to still work when "main" works.

resolvers/node/index.js

@@ -17,7 +17,8 @@ exports.resolve = function (source, file, config) {
   }
 
   try {
-    resolvedPath = resolve.sync(source, opts(file, config));
+    const cachedFilter = function (pkg, dir) { return packageFilter(pkg, dir, config); };
+    resolvedPath = resolve.sync(source, opts(file, config, cachedFilter));
     log('Resolved to:', resolvedPath);
     return { found: true, path: resolvedPath };
   } catch (err) {
@@ -26,7 +27,7 @@ exports.resolve = function (source, file, config) {
   }
 };
 
-function opts(file, config) {
+function opts(file, config, packageFilter) {
   return Object.assign({
     // more closely matches Node (#333)
     // plus 'mjs' for native modules! (#939)
@@ -36,16 +37,32 @@ function opts(file, config) {
   {
     // path.resolve will handle paths relative to CWD
     basedir: path.dirname(path.resolve(file)),
-    packageFilter: packageFilter,
-
+    packageFilter,
   });
 }
 
-function packageFilter(pkg) {
+function identity(x) { return x; }
+
+function packageFilter(pkg, dir, config) {
+  let found = false;
+  const file = path.join(dir, 'dummy.js');
   if (pkg.module) {
-    pkg.main = pkg.module;
-  } else if (pkg['jsnext:main']) {
-    pkg.main = pkg['jsnext:main'];
+    try {
+      resolve.sync(String(pkg.module).replace(/^(?:\.\/)?/, './'), opts(file, config, identity));
+      pkg.main = pkg.module;
+      found = true;
+    } catch (err) {
+      log('resolve threw error trying to find pkg.module:', err);
+    }
+  }
+  if (!found && pkg['jsnext:main']) {
+    try {
+      resolve.sync(String(pkg['jsnext:main']).replace(/^(?:\.\/)?/, './'), opts(file, config, identity));
+      pkg.main = pkg['jsnext:main'];
+      found = true;
+    } catch (err) {
+      log('resolve threw error trying to find pkg[\'jsnext:main\']:', err);
+    }
   }
   return pkg;
 }

resolvers/node/test/package-mains/module-broken/main.js

@@ -0,0 +1 @@
+module.exports = {};

resolvers/node/test/package-mains/module-broken/package.json

@@ -0,0 +1,4 @@
+{
+  "main": "./main.js",
+  "module": "./doesNotExist.js"
+}

resolvers/node/test/packageMains.js

@@ -4,24 +4,29 @@ const chai = require('chai');
 const expect = chai.expect;
 const path = require('path');
 
-const webpack = require('../');
+const resolver = require('../');
 
 const file = path.join(__dirname, 'package-mains', 'dummy.js');
 
 
 describe('packageMains', function () {
   it('captures module', function () {
-    expect(webpack.resolve('./module', file)).property('path')
+    expect(resolver.resolve('./module', file)).property('path')
       .to.equal(path.join(__dirname, 'package-mains', 'module', 'src', 'index.js'));
   });
 
   it('captures jsnext', function () {
-    expect(webpack.resolve('./jsnext', file)).property('path')
+    expect(resolver.resolve('./jsnext', file)).property('path')
       .to.equal(path.join(__dirname, 'package-mains', 'jsnext', 'src', 'index.js'));
   });
 
   it('captures module instead of jsnext', function () {
-    expect(webpack.resolve('./module-and-jsnext', file)).property('path')
+    expect(resolver.resolve('./module-and-jsnext', file)).property('path')
       .to.equal(path.join(__dirname, 'package-mains', 'module-and-jsnext', 'src', 'index.js'));
   });
+
+  it('falls back from a missing "module" to "main"', function () {
+    expect(resolver.resolve('./module-broken', file)).property('path')
+      .to.equal(path.join(__dirname, 'package-mains', 'module-broken', 'main.js'));
+  });
 });

resolvers/webpack/test/package-mains/module-broken/main.js

@@ -0,0 +1 @@
+module.exports = {};

resolvers/webpack/test/package-mains/module-broken/package.json

@@ -0,0 +1,4 @@
+{
+  "main": "./main.js",
+  "module": "./doesNotExist.js"
+}

resolvers/webpack/test/packageMains.js

@@ -4,51 +4,55 @@ const chai = require('chai');
 const expect = chai.expect;
 const path = require('path');
 
-const webpack = require('../');
+const resolver = require('../');
 
 const file = path.join(__dirname, 'package-mains', 'dummy.js');
 
 
 describe('packageMains', function () {
 
   it('captures module', function () {
-    expect(webpack.resolve('./module', file)).property('path')
+    expect(resolver.resolve('./module', file)).property('path')
       .to.equal(path.join(__dirname, 'package-mains', 'module', 'src', 'index.js'));
   });
 
   it('captures jsnext', function () {
-    expect(webpack.resolve('./jsnext', file)).property('path')
+    expect(resolver.resolve('./jsnext', file)).property('path')
       .to.equal(path.join(__dirname, 'package-mains', 'jsnext', 'src', 'index.js'));
   });
 
   it('captures module instead of jsnext', function () {
-    expect(webpack.resolve('./module-and-jsnext', file)).property('path')
+    expect(resolver.resolve('./module-and-jsnext', file)).property('path')
       .to.equal(path.join(__dirname, 'package-mains', 'module-and-jsnext', 'src', 'index.js'));
   });
 
+  it('falls back from a missing "module" to "main"', function () {
+    expect(resolver.resolve('./module-broken', file)).property('path')
+      .to.equal(path.join(__dirname, 'package-mains', 'module-broken', 'main.js'));
+  });
+
   it('captures webpack', function () {
-    expect(webpack.resolve('./webpack', file)).property('path')
+    expect(resolver.resolve('./webpack', file)).property('path')
       .to.equal(path.join(__dirname, 'package-mains', 'webpack', 'webpack.js'));
   });
 
   it('captures jam (array path)', function () {
-    expect(webpack.resolve('./jam', file)).property('path')
+    expect(resolver.resolve('./jam', file)).property('path')
       .to.equal(path.join(__dirname, 'package-mains', 'jam', 'jam.js'));
   });
 
   it('uses configured packageMains, if provided', function () {
-    expect(webpack.resolve('./webpack', file, { config: 'webpack.alt.config.js' })).property('path')
+    expect(resolver.resolve('./webpack', file, { config: 'webpack.alt.config.js' })).property('path')
       .to.equal(path.join(__dirname, 'package-mains', 'webpack', 'index.js'));
   });
 
   it('always defers to module, regardless of config', function () {
-    expect(webpack.resolve('./module', file, { config: 'webpack.alt.config.js' })).property('path')
+    expect(resolver.resolve('./module', file, { config: 'webpack.alt.config.js' })).property('path')
       .to.equal(path.join(__dirname, 'package-mains', 'module', 'src', 'index.js'));
   });
 
   it('always defers to jsnext:main, regardless of config', function () {
-    expect(webpack.resolve('./jsnext', file, { config: 'webpack.alt.config.js' })).property('path')
+    expect(resolver.resolve('./jsnext', file, { config: 'webpack.alt.config.js' })).property('path')
       .to.equal(path.join(__dirname, 'package-mains', 'jsnext', 'src', 'index.js'));
   });
-
 });