From 0ad2bf91639911e49d68024c56df4e03189fd153 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Tue, 28 Feb 2017 15:20:13 +0100 Subject: [PATCH] Correctly resolve peerDependencies for concurrent installs (#2801) * Correctly resolve peerDependencies for concurrent installs * Tests for resolving peerDependencies when using add command --- __tests__/commands/add.js | 53 +++++++++++++++++++ .../.gitkeep | 0 .../add/add-with-peer-dependency-met/.gitkeep | 0 .../add-with-peer-dependency-not-met/.gitkeep | 0 src/package-linker.js | 50 ++++------------- 5 files changed, 62 insertions(+), 41 deletions(-) create mode 100644 __tests__/fixtures/add/add-with-peer-dependency-incorrect/.gitkeep create mode 100644 __tests__/fixtures/add/add-with-peer-dependency-met/.gitkeep create mode 100644 __tests__/fixtures/add/add-with-peer-dependency-not-met/.gitkeep diff --git a/__tests__/commands/add.js b/__tests__/commands/add.js index 70c5de5d70..b05baeb169 100644 --- a/__tests__/commands/add.js +++ b/__tests__/commands/add.js @@ -1,6 +1,7 @@ /* @flow */ import {ConsoleReporter} from '../../src/reporters/index.js'; +import * as reporters from '../../src/reporters/index.js'; import {getPackageVersion, createLockfile, explodeLockfile, run as buildRun, runInstall} from './_helpers.js'; import {Add} from '../../src/cli/commands/add.js'; import * as constants from '../../src/constants.js'; @@ -702,3 +703,55 @@ test.concurrent('install with latest tag and --prefer-offline flag', (): Promise assert.notEqual(version, '1.1.0'); }); }); + +test.concurrent('doesn\'t warn when peer dependency is met during add', (): Promise => { + return buildRun( + reporters.BufferReporter, + fixturesLoc, + async (args, flags, config, reporter, lockfile): Promise => { + const add = new Add(args, flags, config, reporter, lockfile); + await add.init(); + const output = reporter.getBuffer(); + const warnings = output.filter((entry) => entry.type === 'warning'); + assert(!warnings.some((warning) => warning.data.toString().toLowerCase().includes('unmet peer'))); + assert(!warnings.some((warning) => warning.data.toString().toLowerCase().includes('incorrect peer'))); + }, + ['react@15.4.2', 'react-dom@15.4.2'], + {}, + 'add-with-peer-dependency-met', + ); +}); + +test.concurrent('warns when peer dependency is not met during add', (): Promise => { + return buildRun( + reporters.BufferReporter, + fixturesLoc, + async (args, flags, config, reporter, lockfile): Promise => { + const add = new Add(args, flags, config, reporter, lockfile); + await add.init(); + const output = reporter.getBuffer(); + const warnings = output.filter((entry) => entry.type === 'warning'); + assert(warnings.some((warning) => warning.data.toString().toLowerCase().includes('unmet peer'))); + }, + ['react-dom@15.4.2'], + {}, + 'add-with-peer-dependency-not-met', + ); +}); + +test.concurrent('warns when peer dependency is incorrect during add', (): Promise => { + return buildRun( + reporters.BufferReporter, + fixturesLoc, + async (args, flags, config, reporter, lockfile): Promise => { + const add = new Add(args, flags, config, reporter, lockfile); + await add.init(); + const output = reporter.getBuffer(); + const warnings = output.filter((entry) => entry.type === 'warning'); + assert(warnings.some((warning) => warning.data.toString().toLowerCase().includes('incorrect peer'))); + }, + ['react@0.14.8', 'react-dom@15.4.2'], + {}, + 'add-with-peer-dependency-incorrect', + ); +}); diff --git a/__tests__/fixtures/add/add-with-peer-dependency-incorrect/.gitkeep b/__tests__/fixtures/add/add-with-peer-dependency-incorrect/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/__tests__/fixtures/add/add-with-peer-dependency-met/.gitkeep b/__tests__/fixtures/add/add-with-peer-dependency-met/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/__tests__/fixtures/add/add-with-peer-dependency-not-met/.gitkeep b/__tests__/fixtures/add/add-with-peer-dependency-not-met/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/package-linker.js b/src/package-linker.js index 7ac3661135..ecb4f1501c 100644 --- a/src/package-linker.js +++ b/src/package-linker.js @@ -288,48 +288,16 @@ export default class PackageLinker { for (const name in peerDeps) { const range = peerDeps[name]; - - // find a dependency in the tree above us that matches - let searchPatterns: Array = []; - for (let request of ref.requests) { - do { - // get resolved pattern for this request - const dep = this.resolver.getResolvedPattern(request.pattern); - if (!dep) { - continue; - } - - // - const ref = dep._reference; - invariant(ref, 'expected reference'); - searchPatterns = searchPatterns.concat(ref.dependencies); - } while (request = request.parentRequest); - } - - // if the resolver already knows about the peer dependency, add those patterns as well - const packagePatterns = this.resolver.patternsByPackage[name]; - if (packagePatterns) { - searchPatterns = searchPatterns.concat(packagePatterns); - } - - // include root seed patterns last - searchPatterns = searchPatterns.concat(this.resolver.seedPatterns); - - // find matching dep in search patterns - let foundDep: ?{pattern: string, version: string}; - for (const pattern of searchPatterns) { - const dep = this.resolver.getResolvedPattern(pattern); - if (dep && dep.name === name) { - foundDep = {pattern, version: dep.version}; - break; - } - } - - // validate found peer dependency - if (foundDep && this._satisfiesPeerDependency(range, foundDep.version)) { - ref.addDependencies([foundDep.pattern]); + const patterns = this.resolver.patternsByPackage[name] || []; + const foundPattern = patterns.find((pattern) => { + const resolvedPattern = this.resolver.getResolvedPattern(pattern); + return resolvedPattern ? this._satisfiesPeerDependency(range, resolvedPattern.version) : false; + }); + + if (foundPattern) { + ref.addDependencies([foundPattern]); } else { - const depError = foundDep ? 'incorrectPeer' : 'unmetPeer'; + const depError = patterns.length > 0 ? 'incorrectPeer' : 'unmetPeer'; const [pkgHuman, depHuman] = [`${pkg.name}@${pkg.version}`, `${name}@${range}`]; this.reporter.warn(this.reporter.lang(depError, pkgHuman, depHuman)); }