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

Fixed: workspace entries should not be used when trying to check for peer dependencies #7812

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@
<!-- -->
Please add one entry in this file for each change in Yarn's behavior. Use the same format for all entries, including the third-person verb. Make sure you don't add more than one line of text to keep it clean. Thanks!

## Master

- Ensure peer dependencies of workspace projects are appropriately checked.

[#7812](https://github.com/yarnpkg/yarn/pull/7812) - [**Antoine Malliarakis**](https://github.com/antoine-malliarakis)

## 1.22.2

- Sorts files when running `yarn pack` to produce identical layout on Windows and Unix systems
Expand Down
16 changes: 16 additions & 0 deletions __tests__/commands/install/workspaces-install.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,22 @@ test.concurrent("workspaces warn and get ignored if they don't have a name and a
);
});

test.concurrent('should only warn for unmet peer dependency on workspace package', (): Promise<void> => {
return buildRun(
reporters.BufferReporter,
path.join(__dirname, '..', '..', 'fixtures', 'install'),
async (args, flags, config, reporter, lockfile): Promise<void> => {
const install = new Install(flags, config, reporter, lockfile);
await install.init();
const warnings = reporter.getBuffer().filter(({type}) => type === 'warning').map(({data}) => data);
expect(warnings).toEqual(['" > nomore-test-b@0.0.1" has unmet peer dependency "nomore-test-a@^0.0.1".']);
antoine-malliarakis marked this conversation as resolved.
Show resolved Hide resolved
},
[],
{},
'workspaces-install-transient-peer-dep-satisfied',
);
});

test.concurrent('installs workspaces dependencies into root folder', (): Promise<void> => {
return runInstall({}, 'workspaces-install-basic', async (config): Promise<void> => {
const lockfile = await fs.readFile(path.join(config.cwd, 'yarn.lock'));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"private": true,
"devDependencies": {
"nomore-test-b": "0.0.1"
},
"workspaces": [
"packages/*"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "nomore-test-a",
"version": "0.0.1",
"dependencies": {},
"devDependencies": {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "nomore-test-b",
"version": "0.0.1",
"dependencies": {},
"devDependencies": {},
"peerDependencies": {
"nomore-test-a": "^0.0.1"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "nomore-test-dev-deps",
"version": "0.0.1",
"dependencies": {},
"devDependencies": {
"nomore-test-a": "0.0.1"
},
"peerDependencies": {
"nomore-test-a": "^0.0.1"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"name": "nomore-test-prod-deps",
"version": "0.0.1",
"dependencies": {
"nomore-test-a": "0.0.1"
},
"devDependencies": {
},
"peerDependencies": {
"nomore-test-a": "^0.0.1"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "nomore-test-react",
"version": "0.0.1",
"dependencies": {},
"devDependencies": {},
"peerDependencies": {
"react": "^16.12.0"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "nomore-transitive-dev-deps",
"version": "0.0.1",
"dependencies": {},
"devDependencies": {
"nomore-test-a": "0.0.1"
},
"peerDependencies": {
"nomore-test-b": "0.0.1"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"name": "nomore-transitive-prod-deps",
"version": "0.0.1",
"dependencies": {
"nomore-test-a": "0.0.1"
},
"devDependencies": {
},
"peerDependencies": {
"nomore-test-b": "^0.0.1"
}
}
82 changes: 2 additions & 80 deletions src/package-linker.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import {normalizePattern} from './util/normalize-pattern.js';
import {entries} from './util/misc.js';
import * as fs from './util/fs.js';
import lockMutex from './util/mutex.js';
import {satisfiesWithPrereleases} from './util/semver.js';
import WorkspaceLayout from './workspace-layout.js';
import PackagePeerResolver from './package-peer-resolver';

const invariant = require('invariant');
const cmdShim = require('@zkochan/cmd-shim');
Expand Down Expand Up @@ -593,85 +593,7 @@ export default class PackageLinker {
}

resolvePeerModules() {
for (const pkg of this.resolver.getManifests()) {
const peerDeps = pkg.peerDependencies;
const peerDepsMeta = pkg.peerDependenciesMeta;

if (!peerDeps) {
continue;
}

const ref = pkg._reference;
invariant(ref, 'Package reference is missing');

// TODO: We are taking the "shortest" ref tree but there may be multiple ref trees with the same length
const refTree = ref.requests.map(req => req.parentNames).sort((arr1, arr2) => arr1.length - arr2.length)[0];

const getLevelDistance = pkgRef => {
let minDistance = Infinity;
for (const req of pkgRef.requests) {
const distance = refTree.length - req.parentNames.length;

if (distance >= 0 && distance < minDistance && req.parentNames.every((name, idx) => name === refTree[idx])) {
minDistance = distance;
}
}

return minDistance;
};

for (const peerDepName in peerDeps) {
const range = peerDeps[peerDepName];
const meta = peerDepsMeta && peerDepsMeta[peerDepName];

const isOptional = !!(meta && meta.optional);

const peerPkgs = this.resolver.getAllInfoForPackageName(peerDepName);

let peerError = 'unmetPeer';
let resolvedLevelDistance = Infinity;
let resolvedPeerPkg;
for (const peerPkg of peerPkgs) {
const peerPkgRef = peerPkg._reference;
if (!(peerPkgRef && peerPkgRef.patterns)) {
continue;
}
const levelDistance = getLevelDistance(peerPkgRef);
if (isFinite(levelDistance) && levelDistance < resolvedLevelDistance) {
if (this._satisfiesPeerDependency(range, peerPkgRef.version)) {
resolvedLevelDistance = levelDistance;
resolvedPeerPkg = peerPkgRef;
} else {
peerError = 'incorrectPeer';
}
}
}

if (resolvedPeerPkg) {
ref.addDependencies(resolvedPeerPkg.patterns);
this.reporter.verbose(
this.reporter.lang(
'selectedPeer',
`${pkg.name}@${pkg.version}`,
`${peerDepName}@${resolvedPeerPkg.version}`,
resolvedPeerPkg.level,
),
);
} else if (!isOptional) {
this.reporter.warn(
this.reporter.lang(
peerError,
`${refTree.join(' > ')} > ${pkg.name}@${pkg.version}`,
`${peerDepName}@${range}`,
),
);
}
}
}
}

_satisfiesPeerDependency(range: string, version: string): boolean {
return range === '*' || satisfiesWithPrereleases(version, range, this.config.looseSemver);
new PackagePeerResolver(this.config, this.resolver).resolvePeerModules(this.resolver.getManifests());
}

async _warnForMissingBundledDependencies(pkg: Manifest): Promise<void> {
Expand Down
Loading