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.

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

Allow prerelease versions when checking peer deps #3361

Merged
merged 2 commits into from
May 12, 2017

Conversation

ide
Copy link
Contributor

@ide ide commented May 10, 2017

Summary
Uses lower-level APIs in node-semver to allow prerelease versions when checking peer deps. This honors the intent of library authors e.g. when they write a React component that asks for "react": ">=15.0.0" they're saying OK to react@16.0.0 (partly because the React team specifically has said that if you don't have any warnings in version N, then version N+1 should work for you). By extension and modulo bugs, react@16.0.0-alpha works too if the user explicitly installs it.

Fixes #2760

Test plan
Added unit tests for the new semver utility.

Tested in a real project that uses React 16.0.0-alpha.11 and verified I didn't get any peer dep warnings that usually appear from libraries that ask for "react": ">=15.0.0". Downgraded React to 0.14.0 (too low) and saw peer dep warnings come back as expected.

Uses lower-level APIs in node-semver to allow prerelease versions when checking peer deps. This honors the intent of library authors e.g. when they write a React component that asks for `"react": ">=15.0.0"` they're saying OK with `react@16.0.0` (partly because the React team specifically has said that if you don't have any warnings in version N, then version N+1 should work for you). By extension and modulo bugs, `react@16.0.0-alpha` works too (if the user explicitly installs it).

Test Plan: Added unit tests for the new semver utility.

Tested in a real project that uses React 16.0.0-alpha.11 and verified I didn't get any peer dep warnings that usually appear from libraries that ask for `"react": ">=15.0.0"`. Downgraded React to 0.14.0 (too low) and saw peer dep warnings come back as expected.
@bestander
Copy link
Member

Thanks, @ide.
Would you fix the lint error please?

node-semver converts ~ and ^ ranges into pairs of >= and < ranges but the upper bounds don't properly exclude prerelease versions. For example, "^1.0.0" is converted to ">=1.0.0 <2.0.0", which includes "2.0.0-pre" since prerelease versions are lower than their non-prerelease counterparts. As a practical workaround we make upper-bound ranges exclude prereleases and convert "<2.0.0" to "<2.0.0-0", for example.

Added unit tests as well.
@bestander bestander merged commit 6e54578 into yarnpkg:master May 12, 2017
@ide ide deleted the peer-dep-semver branch May 29, 2017 17:48
@SimenB
Copy link
Contributor

SimenB commented Jun 26, 2017

Can this be made to work with engine as well? Haven't actually tested with a pre-release of yarn, but from what I can tell it's separate logic.

export function testEngine(name: string, range: string, versions: Versions, looseSemver: boolean): boolean {
const actual = versions[name];
if (!actual) {
return false;
}
if (!semver.valid(actual, looseSemver)) {
return false;
}
if (semver.satisfies(actual, range, looseSemver)) {
return true;
}
if (name === 'node' && semver.gt(actual, '1.0.0', looseSemver)) {
// WARNING: this is a massive hack and is super gross but necessary for compatibility
// some modules have the `engines.node` field set to a caret version below semver major v1
// eg. ^0.12.0. this is problematic as we enforce engines checks and node is now on version >=1
// to allow this pattern we transform the node version to fake ones in the minor range 10-13
const major = semver.major(actual, looseSemver);
const fakes = [`0.10.${major}`, `0.11.${major}`, `0.12.${major}`, `0.13.${major}`];
for (const actualFake of fakes) {
if (semver.satisfies(actualFake, range, looseSemver)) {
return true;
}
}
}
// incompatible version
return false;
}

$ node --version
v8.2.0-rc.1
$ yarn
yarn install v0.24.6
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
error ansi-styles@3.0.0: The engine "node" is incompatible with this module. Expected version ">=4".
error Found incompatible module
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

EDIT: Workaround: YARN_IGNORE_ENGINES=true yarn (it was a lerna repo, so just --ignore-engines didn't work)

@bestander
Copy link
Member

@SimenB I think it is a good idea, would you send a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants