diff --git a/__tests__/commands/install/integration.js b/__tests__/commands/install/integration.js index b4d07b5f0c..d782a5c5fb 100644 --- a/__tests__/commands/install/integration.js +++ b/__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 => { +test('install should hoist nested bin scripts', (): Promise => { 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 => { - // 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 => { + // 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'))); + }); }); - }); +} diff --git a/src/package-hoister.js b/src/package-hoister.js index f205ce845f..c6a1b1d4f1 100644 --- a/src/package-hoister.js +++ b/src/package-hoister.js @@ -13,8 +13,8 @@ type Parts = Array; 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]);