Fixed yarnpkg#1242 - missing transitive dependencies

__tests__/commands/install/integration.js

@@ -136,7 +136,7 @@ test.concurrent('hoisting should factor ignored dependencies', async () => {
   });
 });
 
-test.concurrent('--production flag ignores dev dependencies', () => {
+test('--production flag ignores dev dependencies', () => {
   return runInstall({production: true}, 'install-production', async (config) => {
     assert.ok(
       !await fs.exists(path.join(config.cwd, 'node_modules', 'lodash')),
@@ -467,7 +467,7 @@ test.concurrent(
 );
 
 // disabled to resolve https://github.com/yarnpkg/yarn/pull/1210
-test.skip('install should hoist nested bin scripts', (): Promise<void> => {
+test('install should hoist nested bin scripts', (): Promise<void> => {
   return runInstall({binLinks: true}, 'install-nested-bin', async (config) => {
     const binScripts = await fs.walk(path.join(config.cwd, 'node_modules', '.bin'));
     // need to double the amount as windows makes 2 entries for each dependency
@@ -727,13 +727,15 @@ test.concurrent('install will not overwrite files in symlinked scoped directorie
   });
 });
 
-test.concurrent.skip('install incompatible optional dependency should still install shared child dependencies',
-  (): Promise<void> => {
-    // 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.
-    return runInstall({}, 'install-should-not-skip-required-shared-deps', async (config) => {
-      assert.ok(await fs.exists(path.join(config.cwd, 'node_modules', 'deep-extend')));
-      assert.ok(await fs.exists(path.join(config.cwd, 'node_modules', 'ini')));
-      assert.ok(await fs.exists(path.join(config.cwd, 'node_modules', 'strip-json-comments')));
+if (process.platform !== 'darwin') {
+  test('install incompatible optional dependency should still install shared child dependencies',
+    (): Promise<void> => {
+      // 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.
+      return runInstall({}, 'install-should-not-skip-required-shared-deps', async (config) => {
+        assert.ok(await fs.exists(path.join(config.cwd, 'node_modules', 'deep-extend')));
+        assert.ok(await fs.exists(path.join(config.cwd, 'node_modules', 'ini')));
+        assert.ok(await fs.exists(path.join(config.cwd, 'node_modules', 'strip-json-comments')));
+      });
     });
-  });
+}

src/package-hoister.js

@@ -13,8 +13,8 @@ type Parts = Array<string>;
 let historyCounter = 0;
 
 export class HoistManifest {
-  constructor(key: string, parts: Parts, pkg: Manifest, loc: string, isIgnored: boolean) {
-    this.ignore = isIgnored;
+  constructor(key: string, parts: Parts, pkg: Manifest, loc: string, isIgnored: () => boolean) {
+    this.isIgnored = isIgnored;
     this.loc = loc;
     this.pkg = pkg;
 
@@ -27,7 +27,7 @@ export class HoistManifest {
     this.addHistory(`Start position = ${key}`);
   }
 
-  ignore: boolean;
+  isIgnored: () => boolean;
   pkg: Manifest;
   loc: string;
   parts: Parts;
@@ -130,13 +130,17 @@ export default class PackageHoister {
 
     //
     let parentParts: Parts = [];
-    let isIgnored = ref.ignore;
+    let isIgnored = () => { return ref.ignore; };
 
     if (parent) {
       if (!this.tree.get(parent.key)) {
         return null;
       }
-      isIgnored = isIgnored || parent.ignore;
+      // non ignored dependencies inherit parent's ignored status
+      // but parent may transition from ignored to non ignored when hoisting
+      if (!isIgnored() && parent.isIgnored()) {
+        isIgnored = () => { return parent.isIgnored(); };
+      }
       parentParts = parent.parts;
     }
 
@@ -182,8 +186,8 @@ export default class PackageHoister {
       if (existing) {
         if (existing.loc === info.loc) {
           // deduping an unignored reference to an ignored one
-          if (existing.ignore && !info.ignore) {
-            existing.ignore = false;
+          if (existing.isIgnored() && !info.isIgnored()) {
+            existing.isIgnored = info.isIgnored;
           }
 
           existing.addHistory(`Deduped ${fullKey} to this item`);
@@ -277,7 +281,6 @@ export default class PackageHoister {
     // remove this item from the `tree` map so we can ignore it
     this.tree.delete(key);
 
-    //
     const {parts, duplicate} = this.getNewParts(key, info, rawParts.slice());
     const newKey = this.implodeKey(parts);
     const oldKey = key;
@@ -389,7 +392,7 @@ export default class PackageHoister {
       const ref = info.pkg._reference;
       invariant(ref, 'expected reference');
 
-      if (info.ignore) {
+      if (info.isIgnored()) {
         info.addHistory('Deleted as this module was ignored');
       } else {
         visibleFlatTree.push([loc, info]);