Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix: Peer dependencies should only be looked up from top level dependencies #3803

Merged
merged 7 commits into from
Jul 7, 2017
Merged

Fix: Peer dependencies should only be looked up from top level dependencies #3803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7095d66
Fix peer dependency to only look from top level dependencies
Jun 30, 2017
ad26fbd
lint
Jul 3, 2017
2c88930
Upgrade/add peer dependencies
Jul 3, 2017
5477c00
Fix babel-eslint
Jul 3, 2017
4d1011d
variable name fix
Jul 3, 2017
960fff6
feedback from @byk
Jul 7, 2017
318db97
Merge branch 'master' into peer-dependency
Jul 7, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions __tests__/commands/add.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -803,6 +803,29 @@ test.concurrent('warns when peer dependency is incorrect during add', (): Promis
);
});

test.concurrent('should only refer to root to satisfy peer dependency', (): Promise<void> => {
return buildRun(
reporters.BufferReporter,
fixturesLoc,
async (args, flags, config, reporter, lockfile): Promise<void> => {
const add = new Add(args, flags, config, reporter, lockfile);
await add.init();

const output = reporter.getBuffer();
const warnings = output.filter(entry => entry.type === 'warning');

expect(
warnings.some(warning => {
return warning.data.toString().toLowerCase().indexOf('incorrect peer') > -1;
}),
).toEqual(true);
},
['file:c'],
{},
'add-with-multiple-versions-of-peer-dependency',
);
});

test.concurrent('should retain build artifacts after add', (): Promise<void> => {
return buildRun(
reporters.BufferReporter,
Expand Down
7 changes: 7 additions & 0 deletions __tests__/fixtures/add/add-with-multiple-versions-of-peer-dependency/a/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "a",
"version": "0.0.0",
"dependencies": {
"b": "file:../b-2.0.0"
}
}
4 changes: 4 additions & 0 deletions __tests__/fixtures/add/add-with-multiple-versions-of-peer-dependency/b-2.0.0/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "b",
"version": "2.0.0"
}
4 changes: 4 additions & 0 deletions __tests__/fixtures/add/add-with-multiple-versions-of-peer-dependency/b/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "b",
"version": "1.0.0"
}
7 changes: 7 additions & 0 deletions __tests__/fixtures/add/add-with-multiple-versions-of-peer-dependency/c/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "c",
"version": "1.0.0",
"peerDependencies": {
"b": "file:b-2.0.0"
}
}
8 changes: 8 additions & 0 deletions __tests__/fixtures/add/add-with-multiple-versions-of-peer-dependency/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "d",
"version": "3.0.0",
"dependencies": {
"a": "file:a",
"b": "file:b"
}
}
6 changes: 4 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
},
"devDependencies": {
"babel-core": "^6.24.1",
"babel-eslint": "^6.1.2",
"babel-eslint": "^7.2.3",
"babel-jest": "^19.0.0",
"babel-loader": "^6.2.5",
"babel-plugin-array-includes": "^2.0.3",
Expand All @@ -56,8 +56,10 @@
"babylon": "^6.5.0",
"eslint": "^3.19.0",
"eslint-config-fb-strict": "^20.0.1",
"eslint-plugin-babel": "^3.3.0",
"eslint-plugin-babel": "^4.0.0",
"eslint-plugin-flowtype": "^2.32.1",
"eslint-plugin-jasmine": "^2.6.2",
"eslint-plugin-prefer-object-spread": "^1.2.1",
"eslint-plugin-prettier": "^2.1.2",
"eslint-plugin-react": "^6.7.1",
"eslint-plugin-yarn-internal": "file:scripts/eslint-rules",
Expand Down
13 changes: 7 additions & 6 deletions src/package-linker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,16 +409,17 @@ export default class PackageLinker {

for (const name in peerDeps) {
const range = peerDeps[name];
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;
const pkgs = this.resolver.getAllInfoForPackageName(name);
const found = pkgs.find(pkg => {
const {root, version} = pkg._reference || {};
return root && this._satisfiesPeerDependency(range, version);
});
const foundPattern = found && found._reference && found._reference.patterns;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is you or flow yelling at you but you don't need to check found._reference since it is required for .find() to return something for found. Actually you can even move patterns up there and just check found as I suggested earlier. I'm guessing you did it this way because of flow complaining tho :D

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup exactly, made flow unhappy. thanks!


if (foundPattern) {
ref.addDependencies([foundPattern]);
ref.addDependencies(foundPattern);
} else {
const depError = patterns.length > 0 ? 'incorrectPeer' : 'unmetPeer';
const depError = pkgs.length > 0 ? 'incorrectPeer' : 'unmetPeer';
const [pkgHuman, depHuman] = [`${pkg.name}@${pkg.version}`, `${name}@${range}`];
this.reporter.warn(this.reporter.lang(depError, pkgHuman, depHuman));
}
Expand Down
3 changes: 2 additions & 1 deletion src/package-resolver.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,10 +224,11 @@ export default class PackageResolver {
*/

getAllInfoForPackageName(name: string): Array<Manifest> {
const patterns = this.patternsByPackage[name] || [];
const infos = [];
const seen = new Set();

for (const pattern of this.patternsByPackage[name]) {
for (const pattern of patterns) {
const info = this.patterns[pattern];
if (seen.has(info)) {
continue;
Expand Down
44 changes: 17 additions & 27 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,17 +267,7 @@ babel-core@^6.0.0, babel-core@^6.0.2, babel-core@^6.24.1:
slash "^1.0.0"
source-map "^0.5.0"

babel-eslint@^6.1.2:
version "6.1.2"
resolved "https://registry.yarnpkg.com/babel-eslint/-/babel-eslint-6.1.2.tgz#5293419fe3672d66598d327da9694567ba6a5f2f"
dependencies:
babel-traverse "^6.0.20"
babel-types "^6.0.19"
babylon "^6.0.18"
lodash.assign "^4.0.0"
lodash.pickby "^4.0.0"

babel-eslint@^7.1.1:
babel-eslint@^7.1.1, babel-eslint@^7.2.3:
version "7.2.3"
resolved "https://registry.yarnpkg.com/babel-eslint/-/babel-eslint-7.2.3.tgz#b2fe2d80126470f5c19442dc757253a897710827"
dependencies:
Expand Down Expand Up @@ -783,7 +773,7 @@ babel-template@^6.16.0, babel-template@^6.24.1:
babylon "^6.11.0"
lodash "^4.2.0"

babel-traverse@^6.0.20, babel-traverse@^6.18.0, babel-traverse@^6.23.1, babel-traverse@^6.24.1:
babel-traverse@^6.18.0, babel-traverse@^6.23.1, babel-traverse@^6.24.1:
version "6.24.1"
resolved "https://registry.yarnpkg.com/babel-traverse/-/babel-traverse-6.24.1.tgz#ab36673fd356f9a0948659e7b338d5feadb31695"
dependencies:
Expand All @@ -797,7 +787,7 @@ babel-traverse@^6.0.20, babel-traverse@^6.18.0, babel-traverse@^6.23.1, babel-tr
invariant "^2.2.0"
lodash "^4.2.0"

babel-types@^6.0.19, babel-types@^6.18.0, babel-types@^6.23.0, babel-types@^6.24.1:
babel-types@^6.18.0, babel-types@^6.23.0, babel-types@^6.24.1:
version "6.24.1"
resolved "https://registry.yarnpkg.com/babel-types/-/babel-types-6.24.1.tgz#a136879dc15b3606bda0d90c1fc74304c2ff0975"
dependencies:
Expand All @@ -806,7 +796,11 @@ babel-types@^6.0.19, babel-types@^6.18.0, babel-types@^6.23.0, babel-types@^6.24
lodash "^4.2.0"
to-fast-properties "^1.0.1"

babylon@^6.0.18, babylon@^6.11.0, babylon@^6.13.0, babylon@^6.15.0, babylon@^6.17.0, babylon@^6.5.0:
babylon@7.0.0-beta.8:
version "7.0.0-beta.8"
resolved "https://registry.yarnpkg.com/babylon/-/babylon-7.0.0-beta.8.tgz#2bdc5ae366041442c27e068cce6f0d7c06ea9949"

babylon@^6.11.0, babylon@^6.13.0, babylon@^6.15.0, babylon@^6.17.0, babylon@^6.5.0:
version "6.17.1"
resolved "https://registry.yarnpkg.com/babylon/-/babylon-6.17.1.tgz#17f14fddf361b695981fe679385e4f1c01ebd86f"

Expand Down Expand Up @@ -1589,11 +1583,7 @@ eslint-config-fbjs@^2.0.0-alpha.1:
version "2.0.0-alpha.1"
resolved "https://registry.yarnpkg.com/eslint-config-fbjs/-/eslint-config-fbjs-2.0.0-alpha.1.tgz#addda12e1e68cec9271436df01ddcfe67bd1e88d"

eslint-plugin-babel@^3.3.0:
version "3.3.0"
resolved "https://registry.yarnpkg.com/eslint-plugin-babel/-/eslint-plugin-babel-3.3.0.tgz#2f494aedcf6f4aa4e75b9155980837bc1fbde193"

eslint-plugin-babel@^4.1.1:
eslint-plugin-babel@^4.0.0, eslint-plugin-babel@^4.1.1:
version "4.1.1"
resolved "https://registry.yarnpkg.com/eslint-plugin-babel/-/eslint-plugin-babel-4.1.1.tgz#ef285c87039b67beb3bbd227f5b0eed4fb376b87"

Expand All @@ -1603,10 +1593,18 @@ eslint-plugin-flowtype@^2.32.1:
dependencies:
lodash "^4.15.0"

eslint-plugin-jasmine@^2.6.2:
version "2.6.2"
resolved "https://registry.yarnpkg.com/eslint-plugin-jasmine/-/eslint-plugin-jasmine-2.6.2.tgz#259331192835100f4055ff3f9a6a97a160797935"

eslint-plugin-jest@^20.0.3:
version "20.0.3"
resolved "https://registry.yarnpkg.com/eslint-plugin-jest/-/eslint-plugin-jest-20.0.3.tgz#ec15eba6ac0ab44a67ebf6e02672ca9d7e7cba29"

eslint-plugin-prefer-object-spread@^1.2.1:
version "1.2.1"
resolved "https://registry.yarnpkg.com/eslint-plugin-prefer-object-spread/-/eslint-plugin-prefer-object-spread-1.2.1.tgz#27fb91853690cceb3ae6101d9c8aecc6a67a402c"

eslint-plugin-prettier@^2.1.2:
version "2.1.2"
resolved "https://registry.yarnpkg.com/eslint-plugin-prettier/-/eslint-plugin-prettier-2.1.2.tgz#4b90f4ee7f92bfbe2e926017e1ca40eb628965ea"
Expand Down Expand Up @@ -3199,10 +3197,6 @@ lodash._root@^3.0.0:
version "3.0.1"
resolved "https://registry.yarnpkg.com/lodash._root/-/lodash._root-3.0.1.tgz#fba1c4524c19ee9a5f8136b4609f017cf4ded692"

lodash.assign@^4.0.0:
version "4.2.0"
resolved "https://registry.yarnpkg.com/lodash.assign/-/lodash.assign-4.2.0.tgz#0d99f3ccd7a6d261d19bdaeb9245005d285808e7"

lodash.assignwith@^4.0.7:
version "4.2.0"
resolved "https://registry.yarnpkg.com/lodash.assignwith/-/lodash.assignwith-4.2.0.tgz#127a97f02adc41751a954d24b0de17e100e038eb"
Expand Down Expand Up @@ -3249,10 +3243,6 @@ lodash.pick@^4.2.1:
version "4.4.0"
resolved "https://registry.yarnpkg.com/lodash.pick/-/lodash.pick-4.4.0.tgz#52f05610fff9ded422611441ed1fc123a03001b3"

lodash.pickby@^4.0.0:
version "4.6.0"
resolved "https://registry.yarnpkg.com/lodash.pickby/-/lodash.pickby-4.6.0.tgz#7dea21d8c18d7703a27c704c15d3b84a67e33aff"

lodash.restparam@^3.0.0:
version "3.6.1"
resolved "https://registry.yarnpkg.com/lodash.restparam/-/lodash.restparam-3.6.1.tgz#936a4e309ef330a7645ed4145986c85ae5b20805"
Expand Down