Fix #1435 (failing dependencies of optional dependencies) (#2811)

* Fix #1435 (failing dependencies of optional dependencies)

Mark all dependencies of optional dependencies as themselves optional, so that if they fail, the whole install process doesn't fail.

* Update tests

Add a test to make sure code marking sub-dependencies as optional doesn't do so when another dependency marks it as non-optional, and convert a test (plus a pre-existing one) to use file: URIs instead of offline mirroring, for readability.

Author: Ben Demboski

__tests__/commands/install/integration.js

@@ -894,6 +894,14 @@ test.concurrent('install a module with incompatible optional dependency should s
     });
   });
 
+test.concurrent('install a module with optional dependency should skip incompatible transient dependency',
+  (): Promise<void> => {
+    return runInstall({}, 'install-should-skip-incompatible-optional-sub-dep', async (config) => {
+      assert.ok(await fs.exists(path.join(config.cwd, 'node_modules', 'dep-optional')));
+      assert.ok(!(await fs.exists(path.join(config.cwd, 'node_modules', 'dep-incompatible'))));
+    });
+  });
+
 // this tests for a problem occuring due to optional dependency incompatible with os, in this case fsevents
 // this would fail on os's incompatible with fsevents, which is everything except osx.
 if (process.platform !== 'darwin') {
@@ -914,6 +922,14 @@ test.concurrent('optional dependency that fails to build should not be installed
     });
   });
 
+test.concurrent('failing dependency of optional dependency should not be installed',
+  (): Promise<void> => {
+    return runInstall({}, 'should-not-install-failing-deps-of-optional-deps', async (config) => {
+      assert.equal(await fs.exists(path.join(config.cwd, 'node_modules', 'optional-dep')), true);
+      assert.equal(await fs.exists(path.join(config.cwd, 'node_modules', 'sub-failing')), false);
+    });
+  });
+
 // Covers current behavior, issue opened whether this should be changed https://github.com/yarnpkg/yarn/issues/2274
 test.concurrent('a subdependency of an optional dependency that fails should be installed',
   (): Promise<void> => {
@@ -923,6 +939,19 @@ test.concurrent('a subdependency of an optional dependency that fails should be
     });
   });
 
+test.concurrent('a sub-dependency should be non-optional if any parents mark it non-optional',
+  async (): Promise<void> => {
+    let thrown = false;
+    try {
+      await runInstall({}, 'failing-sub-dep-optional-and-normal', () => {});
+    } catch (err) {
+      thrown = true;
+      expect(err.message).toContain('sub-failing: Command failed');
+    }
+    assert(thrown);
+  });
+
+
 test.concurrent('should not loose dependencies when installing with --production',
   (): Promise<void> => {
     // revealed https://github.com/yarnpkg/yarn/issues/2263

__tests__/fixtures/install/failing-sub-dep-optional-and-normal/normal-dep/install.js

@@ -0,0 +1 @@
+process.exit(0);

__tests__/fixtures/install/failing-sub-dep-optional-and-normal/normal-dep/package.json

@@ -0,0 +1,10 @@
+{
+  "name": "normal-dep",
+  "version": "0.0.0",
+  "scripts": {
+    "install": "node install.js"
+  },
+  "dependencies": {
+    "sub-failing": "file:../sub-failing"
+  }
+}

__tests__/fixtures/install/failing-sub-dep-optional-and-normal/optional-dep/install.js

@@ -0,0 +1 @@
+process.exit(0);

__tests__/fixtures/install/failing-sub-dep-optional-and-normal/optional-dep/package.json

@@ -0,0 +1,10 @@
+{
+  "name": "optional-dep",
+  "version": "0.0.0",
+  "scripts": {
+    "install": "node install.js"
+  },
+  "dependencies": {
+    "sub-failing": "file:../sub-failing"
+  }
+}

__tests__/fixtures/install/failing-sub-dep-optional-and-normal/package.json

@@ -0,0 +1,8 @@
+{
+  "dependencies": {
+    "normal-dep": "file:normal-dep"
+  },
+  "optionalDependencies": {
+    "optional-dep": "file:optional-dep"
+  }
+}

__tests__/fixtures/install/failing-sub-dep-optional-and-normal/sub-failing/install.js

@@ -0,0 +1 @@
+process.exit(1);

__tests__/fixtures/install/failing-sub-dep-optional-and-normal/sub-failing/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "sub-failing",
+  "version": "0.0.0",
+  "scripts": {
+    "install": "node install.js"
+  }
+}

__tests__/fixtures/install/install-should-skip-incompatible-optional-dep/dep-a/package.json

@@ -0,0 +1,12 @@
+{
+  "name": "dep-a",
+  "version": "1.0.0",
+  "description": "",
+  "main": "index.js",
+  "scripts": {
+    "postinstall": "mkdir lib"
+  },
+  "keywords": [],
+  "author": "",
+  "license": "ISC"
+}

__tests__/fixtures/install/install-should-skip-incompatible-optional-dep/dep-incompatible/package.json

@@ -0,0 +1,18 @@
+{
+  "name": "dep-incompatible",
+  "version": "1.0.0",
+  "description": "",
+  "main": "index.js",
+  "scripts": {
+    "postinstall": "mkdir lib"
+  },
+  "dependencies": {
+    "dep-a": "file:../dep-a"
+  },
+  "os": [
+    "invalid"
+  ],
+  "keywords": [],
+  "author": "",
+  "license": "ISC"
+}