don't allow optional dependency resolution to fail - fixes #103

Author: Sebastian McKenzie

src/cli/commands/install.js

@@ -9,8 +9,8 @@
  * @flow
  */
 
-import type { RegistryNames } from "../../registries/index.js";
 import type { Reporter } from "kreporters";
+import type { DependencyRequestPatterns } from "../../types.js";
 import type Config from "../../config.js";
 import Lockfile from "../../lockfile/index.js";
 import lockStringify from "../../lockfile/stringify.js";
@@ -76,11 +76,7 @@ export class Install {
    */
 
   async fetchRequestFromCwd(excludePatterns?: Array<string> = []): Promise<[
-    Array<{
-      pattern: string,
-      registry: RegistryNames,
-      optional?: boolean
-    }>,
+    DependencyRequestPatterns,
     Array<string>
   ]> {
     let patterns = [];
@@ -122,7 +118,7 @@ export class Install {
           }
 
           patterns.push(pattern);
-          deps.push({ pattern, registry });
+          deps.push({ pattern, registry, ignore: !!this.flags.production });
         }
 
         // optional deps
@@ -394,7 +390,7 @@ export function setFlags(commander: Object) {
   commander.option("-O, --save-optional", "save package to your `optionalDependencies`");
   commander.option("-E, --save-exact", "");
   commander.option("-T, --save-tilde", "");
-  commander.option("--no-optional"); // TODO
+  commander.option("--production, --prod", "");
   commander.option("--no-lockfile");
   commander.option("--init-mirror", "initialise local package mirror and copy module tarballs");
 }

src/package-linker.js

@@ -217,19 +217,26 @@ export default class PackageLinker {
     let flatTree = [];
     for (let key in tree) {
       let info = tree[key];
-      let loc  = path.join(this.config.modulesFolder, key.replace(/#/g, "/node_modules/"));
+
+      // should we ignore this module from linking?
+      let ref = info.pkg.reference;
+      invariant(ref, "expected reference");
+      if (ref.ignore) continue;
+
+      // decompress the location and push it to the flat tree
+      let loc = path.join(this.config.modulesFolder, key.replace(/#/g, "/node_modules/"));
       flatTree.push([loc, info]);
     }
     return flatTree;
   }
 
   async copyModules(patterns: Array<string>): Promise<void> {
     let flatTree = await this.initCopyModules(patterns);
+
     // sorted tree makes file creation and copying not to interfere with each other
     flatTree = flatTree.sort((dep1, dep2) => {
       return dep1[0].localeCompare(dep2[0]);
     });
-    let self = this;
 
     //
     let tickCopyModule = this.reporter.progress(flatTree.length);
@@ -246,9 +253,9 @@ export default class PackageLinker {
 
     //
     let tickBin = this.reporter.progress(flatTree.length);
-    await promise.queue(flatTree, async function ([dest, { pkg }]) {
+    await promise.queue(flatTree, async ([dest, { pkg }]) => {
       let binLoc = path.join(dest, "node_modules");
-      await self.linkBinDependencies([], pkg, binLoc);
+      await this.linkBinDependencies([], pkg, binLoc);
       tickBin(dest);
     }, 4);
   }

src/package-reference.js

@@ -40,6 +40,7 @@ export default class PackageReference {
     this.permissions = {};
     this.patterns    = [];
     this.optional    = null;
+    this.ignore      = null;
     this.location    = null;
     this.saveForOffline = !!saveForOffline;
   }
@@ -52,6 +53,7 @@ export default class PackageReference {
   version: string;
   uid: string;
   optional: ?boolean;
+  ignore: ?boolean;
   saveForOffline: boolean;
   dependencies: Array<string>;
   patterns: Array<string>;
@@ -113,4 +115,13 @@ export default class PackageReference {
       this.optional = false;
     }
   }
+
+  addIgnore(ignore: boolean) {
+    // see comments in addOptional
+    if (this.ignore == null) {
+      this.ignore = ignore;
+    } else if (!ignore) {
+      this.ignore = false;
+    }
+  }
 }

src/package-request.js

@@ -33,12 +33,16 @@ export default class PackageRequest {
     registry,
     resolver,
     lockfile,
-    parentRequest
+    parentRequest,
+    optional,
+    ignore
   }: {
     pattern: string,
     lockfile: ?Lockfile,
     registry: RegistryNames,
     resolver: PackageResolver,
+    optional: boolean,
+    ignore: boolean,
     parentRequest: ?PackageRequest // eslint-disable-line no-unused-vars
   }) {
     this.parentRequest = parentRequest;
@@ -47,8 +51,10 @@ export default class PackageRequest {
     this.registry      = registry;
     this.reporter      = resolver.reporter;
     this.resolver      = resolver;
+    this.optional      = optional;
     this.pattern       = pattern;
     this.config        = resolver.config;
+    this.ignore        = ignore;
   }
 
   static getExoticResolver(pattern: string): ?Function { // TODO make this type more refined
@@ -65,6 +71,8 @@ export default class PackageRequest {
   pattern: string;
   config: Config;
   registry: RegistryNames;
+  ignore: boolean;
+  optional: boolean;
 
   getHuman(): string {
     let chain = [];
@@ -214,22 +222,10 @@ export default class PackageRequest {
    * TODO description
    */
 
-  async find(optional: boolean): Promise<void> {
-    let info: ?Manifest;
-
+  async find(): Promise<void> {
     // find verison info for this package pattern
-    try {
-      info = await this.findVersionInfo();
-      if (!info) throw new MessageError(`Couldn't find package ${this.pattern}`);
-    } catch (err) {
-      if (optional) {
-        // TODO add verbose flag
-        this.reporter.error(err.message);
-        return;
-      } else {
-        throw err;
-      }
-    }
+    let info: ?Manifest = await this.findVersionInfo();
+    if (!info) throw new MessageError(`Couldn't find package ${this.pattern}`);
 
     // check if while we were resolving this dep we've already resolved one that satisfies
     // the same range
@@ -239,7 +235,8 @@ export default class PackageRequest {
       invariant(ref, "Resolved package info has no package reference");
       ref.addRequest(this);
       ref.addPattern(this.pattern);
-      ref.addOptional(optional);
+      ref.addOptional(this.optional);
+      ref.addIgnore(this.ignore);
       this.resolver.addPattern(this.pattern, resolved);
       return;
     }
@@ -304,22 +301,35 @@ export default class PackageRequest {
     for (let depName in info.dependencies) {
       let depPattern = depName + "@" + info.dependencies[depName];
       deps.push(depPattern);
-      promises.push(this.resolver.find(depPattern, remote.registry, false, this, subLockfile));
+      promises.push(this.resolver.find({
+        pattern: depPattern,
+        registry: remote.registry,
+        optional: false,
+        parentRequest: this,
+        subLockfile
+      }));
     }
 
     // optional deps
     for (let depName in info.optionalDependencies) {
       let depPattern = depName + "@" + info.optionalDependencies[depName];
       deps.push(depPattern);
-      promises.push(this.resolver.find(depPattern, remote.registry, true, this, subLockfile));
+      promises.push(this.resolver.find({
+        pattern: depPattern,
+        registry: remote.registry,
+        optional: true,
+        parentRequest: this,
+        subLockfile
+      }));
     }
 
     await Promise.all(promises);
 
     this.resolver.addPattern(this.pattern, info);
     ref.setDependencies(deps);
     ref.addPattern(this.pattern);
-    ref.addOptional(optional);
+    ref.addOptional(this.optional);
+    ref.addIgnore(this.ignore);
     this.resolver.registerPackageReference(ref);
   }
 

src/package-resolver.js

@@ -289,13 +289,21 @@ export default class PackageResolver {
    * TODO description
    */
 
-  async find(
+  async find({
+    pattern,
+    registry,
+    ignore = false,
+    optional = false,
+    parentRequest,
+    subLockfile
+  }: {
     pattern: string,
     registry: RegistryNames,
-    optional?: boolean = false,
+    optional?: boolean,
+    ignore?: boolean,
     parentRequest?: ?PackageRequest,
     subLockfile?: ?Lockfile
-  ): Promise<void> {
+  }): Promise<void> {
     let fetchKey = `${registry}:${pattern}`;
     if (this.fetchingPatterns[fetchKey]) {
       return;
@@ -311,13 +319,20 @@ export default class PackageResolver {
       this.newPatterns.push(pattern);
     }
 
+    // propagate `ignore` option
+    if (parentRequest && parentRequest.ignore) {
+      ignore = true;
+    }
+
     return new PackageRequest({
       lockfile: subLockfile,
       pattern,
       registry,
       parentRequest,
+      optional,
+      ignore,
       resolver: this
-    }).find(optional);
+    }).find();
   }
 
   /**
@@ -334,7 +349,7 @@ export default class PackageResolver {
     // build up promises
     let promises = [];
     for (let { pattern, registry, optional } of deps) {
-      promises.push(this.find(pattern, registry, optional));
+      promises.push(this.find({ pattern, registry, optional }));
     }
     await Promise.all(promises);
 

src/types.js

@@ -17,7 +17,8 @@ import type PackageReference from "./package-reference.js";
 export type DependencyRequestPatterns = Array<{
   pattern: string,
   registry: RegistryNames,
-  optional?: boolean
+  optional?: boolean,
+  ignore?: boolean
 }>;
 
 // person object, the exploded version of a `maintainers`/`authors` field