Fix autoinstall infinite loop (#1241)

src/Bundler.js

@@ -99,7 +99,10 @@ class Bundler extends EventEmitter {
         options.hmrHostname ||
         (options.target === 'electron' ? 'localhost' : ''),
       detailedReport: options.detailedReport || false,
-      autoinstall: (options.autoinstall || false) && !isProduction,
+      autoinstall:
+        typeof options.autoinstall === 'boolean'
+          ? options.autoinstall
+          : !isProduction,
       contentHash:
         typeof options.contentHash === 'boolean'
           ? options.contentHash
@@ -366,16 +369,17 @@ class Bundler extends EventEmitter {
     try {
       return await this.resolveAsset(dep.name, asset.name);
     } catch (err) {
-      let thrown = err;
+      // If the dep is optional, return before we throw
+      if (dep.optional) {
+        return;
+      }
 
-      if (thrown.message.indexOf(`Cannot find module '${dep.name}'`) === 0) {
-        // Check if dependency is a local file
+      if (err.code === 'MODULE_NOT_FOUND') {
         let isLocalFile = /^[/~.]/.test(dep.name);
         let fromNodeModules = asset.name.includes(
           `${Path.sep}node_modules${Path.sep}`
         );
 
-        // If it's not a local file, attempt to install the dep
         if (
           !isLocalFile &&
           !fromNodeModules &&
@@ -385,30 +389,32 @@ class Bundler extends EventEmitter {
           return await this.installDep(asset, dep);
         }
 
-        // If the dep is optional, return before we throw
-        if (dep.optional) {
-          return;
-        }
-
-        thrown.message = `Cannot resolve dependency '${dep.name}'`;
+        err.message = `Cannot resolve dependency '${dep.name}'`;
         if (isLocalFile) {
           const absPath = Path.resolve(Path.dirname(asset.name), dep.name);
-          thrown.message += ` at '${absPath}'`;
+          err.message += ` at '${absPath}'`;
         }
 
-        await this.throwDepError(asset, dep, thrown);
+        await this.throwDepError(asset, dep, err);
       }
 
-      throw thrown;
+      throw err;
     }
   }
 
   async installDep(asset, dep) {
-    let [moduleName] = this.resolver.getModuleParts(dep.name);
-    try {
-      await installPackage([moduleName], asset.name, {saveDev: false});
-    } catch (err) {
-      await this.throwDepError(asset, dep, err);
+    // Check if module exists, prevents useless installs
+    let resolved = await this.resolver.resolveModule(dep.name, asset.name);
+
+    // If the module resolved (i.e. wasn't a local file), but the module directory wasn't found, install it.
+    if (resolved.moduleName && !resolved.moduleDir) {
+      try {
+        await installPackage([resolved.moduleName], asset.name, {
+          saveDev: false
+        });
+      } catch (err) {
+        await this.throwDepError(asset, dep, err);
+      }
     }
 
     return await this.resolveDep(asset, dep, false);

src/Resolver.js

@@ -53,6 +53,28 @@ class Resolver {
 
     extensions.unshift('');
 
+    // Resolve the module directory or local file path
+    let module = await this.resolveModule(filename, parent);
+    let resolved;
+
+    if (module.moduleDir) {
+      resolved = await this.loadNodeModules(module, extensions);
+    } else if (module.filePath) {
+      resolved = await this.loadRelative(module.filePath, extensions);
+    }
+
+    if (!resolved) {
+      let dir = parent ? path.dirname(parent) : process.cwd();
+      let err = new Error(`Cannot find module '${input}' from '${dir}'`);
+      err.code = 'MODULE_NOT_FOUND';
+      throw err;
+    }
+
+    this.cache.set(key, resolved);
+    return resolved;
+  }
+
+  async resolveModule(filename, parent) {
     let dir = parent ? path.dirname(parent) : process.cwd();
 
     // If this isn't the entrypoint, resolve the input file to an absolute path
@@ -63,24 +85,30 @@ class Resolver {
     // Resolve aliases in the parent module for this file.
     filename = await this.loadAlias(filename, dir);
 
-    let resolved;
+    // Return just the file path if this is a file, not in node_modules
     if (path.isAbsolute(filename)) {
-      // load as file
-      resolved = await this.loadRelative(filename, extensions);
-    } else {
-      // load node_modules
-      resolved = await this.loadNodeModules(filename, dir, extensions);
+      return {
+        filePath: filename
+      };
     }
 
+    // Resolve the module in node_modules
+    let resolved;
+    try {
+      resolved = await this.findNodeModulePath(filename, dir);
+    } catch (err) {
+      // ignore
+    }
+
+    // If we couldn't resolve the node_modules path, just return the module name info
     if (!resolved) {
-      let err = new Error(
-        "Cannot find module '" + input + "' from '" + dir + "'"
-      );
-      err.code = 'MODULE_NOT_FOUND';
-      throw err;
+      let parts = this.getModuleParts(filename);
+      resolved = {
+        moduleName: parts[0],
+        subPath: parts[1]
+      };
     }
 
-    this.cache.set(key, resolved);
     return resolved;
   }
 
@@ -127,10 +155,9 @@ class Resolver {
     );
   }
 
-  async loadNodeModules(filename, dir, extensions) {
-    // Check if this is a builtin module
+  async findNodeModulePath(filename, dir) {
     if (builtins[filename]) {
-      return {path: builtins[filename]};
+      return {filePath: builtins[filename]};
     }
 
     let parts = this.getModuleParts(filename);
@@ -147,20 +174,12 @@ class Resolver {
         let moduleDir = path.join(dir, 'node_modules', parts[0]);
         let stats = await fs.stat(moduleDir);
         if (stats.isDirectory()) {
-          let f = path.join(dir, 'node_modules', filename);
-
-          // If a module was specified as a module sub-path (e.g. some-module/some/path),
-          // it is likely a file. Try loading it as a file first.
-          if (parts.length > 1) {
-            let pkg = await this.readPackage(moduleDir);
-            let res = await this.loadAsFile(f, extensions, pkg);
-            if (res) {
-              return res;
-            }
-          }
-
-          // Otherwise, load as a directory.
-          return await this.loadDirectory(f, extensions);
+          return {
+            moduleName: parts[0],
+            subPath: parts[1],
+            moduleDir: moduleDir,
+            filePath: path.join(dir, 'node_modules', filename)
+          };
         }
       } catch (err) {
         // ignore
@@ -171,6 +190,25 @@ class Resolver {
     }
   }
 
+  async loadNodeModules(module, extensions) {
+    try {
+      // If a module was specified as a module sub-path (e.g. some-module/some/path),
+      // it is likely a file. Try loading it as a file first.
+      if (module.subPath) {
+        let pkg = await this.readPackage(module.moduleDir);
+        let res = await this.loadAsFile(module.filePath, extensions, pkg);
+        if (res) {
+          return res;
+        }
+      }
+
+      // Otherwise, load as a directory.
+      return await this.loadDirectory(module.filePath, extensions);
+    } catch (e) {
+      // ignore
+    }
+  }
+
   async isFile(file) {
     try {
       let stat = await fs.stat(file);

test/integration/dont-autoinstall-resolve-alias-fails/index.js

@@ -0,0 +1 @@
+const vue = require('aliasVue/thisDoesNotExist');

test/integration/dont-autoinstall-resolve-alias-fails/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "alias-install",
+  "private": true,
+  "alias": {
+    "aliasVue": "vue"
+  }
+}

test/integration/dont-autoinstall-resolve-fails/index.js

@@ -0,0 +1 @@
+const thisWontwork = require('vue/thisDoesNotExist');

test/javascript.js

@@ -779,6 +779,38 @@ describe('javascript', function() {
     assert.deepEqual(output, err);
   });
 
+  it('should not autoinstall if resolve failed on installed module', async function() {
+    let error;
+    try {
+      await bundle(
+        __dirname + '/integration/dont-autoinstall-resolve-fails/index.js'
+      );
+    } catch (err) {
+      error = err;
+    }
+    assert.equal(
+      error.message,
+      `Cannot resolve dependency 'vue/thisDoesNotExist'`
+    );
+    assert.equal(error.code, 'MODULE_NOT_FOUND');
+  });
+
+  it('should not autoinstall if resolve failed on aliased module', async function() {
+    let error;
+    try {
+      await bundle(
+        __dirname + '/integration/dont-autoinstall-resolve-alias-fails/index.js'
+      );
+    } catch (err) {
+      error = err;
+    }
+    assert.equal(
+      error.message,
+      `Cannot resolve dependency 'aliasVue/thisDoesNotExist'`
+    );
+    assert.equal(error.code, 'MODULE_NOT_FOUND');
+  });
+
   it('should ignore require if it is defined in the scope', async function() {
     let b = await bundle(__dirname + '/integration/require-scope/index.js');
 

test/resolver.js

@@ -424,5 +424,21 @@ describe('resolver', function() {
 
       assert(threw, 'Did not throw');
     });
+
+    it('should throw when a subfile of a node_module cannot be resolved', async function() {
+      let threw = false;
+      try {
+        await resolver.resolve('xyz/test/file', path.join(rootDir, 'foo.js'));
+      } catch (err) {
+        threw = true;
+        assert.equal(
+          err.message,
+          "Cannot find module 'xyz/test/file' from '" + rootDir + "'"
+        );
+        assert.equal(err.code, 'MODULE_NOT_FOUND');
+      }
+
+      assert(threw, 'Did not throw');
+    });
   });
 });