From c1cdc0184fec3317da89e2326e80f56697a59453 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Mon, 21 Jun 2021 00:48:56 +0300 Subject: [PATCH] fix: resolve peer dependencies from the root of the workspace (#3549) --- .changeset/lucky-walls-reflect.md | 5 + packages/resolve-dependencies/package.json | 1 - .../resolve-dependencies/src/resolvePeers.ts | 95 ++++++++++--------- .../supi/test/install/peerDependencies.ts | 54 ++++++++--- pnpm-lock.yaml | 9 -- 5 files changed, 97 insertions(+), 67 deletions(-) create mode 100644 .changeset/lucky-walls-reflect.md diff --git a/.changeset/lucky-walls-reflect.md b/.changeset/lucky-walls-reflect.md new file mode 100644 index 0000000000..a484a19c1a --- /dev/null +++ b/.changeset/lucky-walls-reflect.md @@ -0,0 +1,5 @@ +--- +"@pnpm/resolve-dependencies": patch +--- + +Peer dependencies should get resolved from the workspace root. diff --git a/packages/resolve-dependencies/package.json b/packages/resolve-dependencies/package.json index b7396cf1b7..91d99c99e3 100644 --- a/packages/resolve-dependencies/package.json +++ b/packages/resolve-dependencies/package.json @@ -44,7 +44,6 @@ "dependency-path": "workspace:8.0.1", "encode-registry": "^3.0.0", "get-npm-tarball-url": "^2.0.2", - "import-from": "^3.0.0", "path-exists": "^4.0.0", "ramda": "^0.27.1", "replace-string": "^3.1.0", diff --git a/packages/resolve-dependencies/src/resolvePeers.ts b/packages/resolve-dependencies/src/resolvePeers.ts index dd02eedb74..60fcca633f 100644 --- a/packages/resolve-dependencies/src/resolvePeers.ts +++ b/packages/resolve-dependencies/src/resolvePeers.ts @@ -4,7 +4,6 @@ import PnpmError from '@pnpm/error' import logger from '@pnpm/logger' import { Dependencies } from '@pnpm/types' import { depPathToFilename } from 'dependency-path' -import importFrom from 'import-from' import { KeyValuePair } from 'ramda' import fromPairs from 'ramda/src/fromPairs' import isEmpty from 'ramda/src/isEmpty' @@ -66,28 +65,15 @@ export default function ( } { const depGraph: GenericDependenciesGraph = {} const pathsByNodeId = {} + const _createPkgsByName = createPkgsByName.bind(null, opts.dependenciesTree) + const rootProject = opts.projects.length > 1 ? opts.projects.find(({ id }) => id === '.') : null + const rootPkgsByName = rootProject == null ? {} : _createPkgsByName(rootProject) for (const { directNodeIdsByAlias, topParents, rootDir } of opts.projects) { - const pkgsByName = Object.assign( - fromPairs( - topParents.map(({ name, version }: {name: string, version: string}): KeyValuePair => [ - name, - { - depth: 0, - version, - }, - ]) - ), - toPkgByName( - Object - .keys(directNodeIdsByAlias) - .map((alias) => ({ - alias, - node: opts.dependenciesTree[directNodeIdsByAlias[alias]], - nodeId: directNodeIdsByAlias[alias], - })) - ) - ) + const pkgsByName = { + ...rootPkgsByName, + ..._createPkgsByName({ directNodeIdsByAlias, topParents }), + } resolvePeersOfChildren(directNodeIdsByAlias, pkgsByName, { dependenciesTree: opts.dependenciesTree, @@ -122,6 +108,35 @@ export default function ( } } +function createPkgsByName ( + dependenciesTree: DependenciesTree, + { directNodeIdsByAlias, topParents }: { + directNodeIdsByAlias: {[alias: string]: string} + topParents: Array<{name: string, version: string}> + } +) { + return Object.assign( + fromPairs( + topParents.map(({ name, version }): KeyValuePair => [ + name, + { + depth: 0, + version, + }, + ]) + ), + toPkgByName( + Object + .keys(directNodeIdsByAlias) + .map((alias) => ({ + alias, + node: dependenciesTree[directNodeIdsByAlias[alias]], + nodeId: directNodeIdsByAlias[alias], + })) + ) + ) +} + interface PeersCacheItem { depPath: string resolvedPeers: Array<[string, string]> @@ -367,34 +382,26 @@ function resolvePeers ( for (const peerName in ctx.resolvedPackage.peerDependencies) { // eslint-disable-line:forin const peerVersionRange = ctx.resolvedPackage.peerDependencies[peerName] - let resolved = ctx.parentPkgs[peerName] + const resolved = ctx.parentPkgs[peerName] if (!resolved) { missingPeers.push(peerName) - try { - const { version } = importFrom(ctx.rootDir, `${peerName}/package.json`) as { version: string } - resolved = { - depth: -1, - version, - } - } catch (err) { - if ( - ctx.resolvedPackage.peerDependenciesMeta?.[peerName]?.optional === true - ) { - continue - } - const friendlyPath = nodeIdToFriendlyPath(ctx.nodeId, ctx.dependenciesTree) - const message = `${friendlyPath ? `${friendlyPath}: ` : ''}${packageFriendlyId(ctx.resolvedPackage)} \ -requires a peer of ${peerName}@${peerVersionRange} but none was installed.` - if (ctx.strictPeerDependencies) { - throw new PnpmError('MISSING_PEER_DEPENDENCY', message) - } - logger.warn({ - message, - prefix: ctx.rootDir, - }) + if ( + ctx.resolvedPackage.peerDependenciesMeta?.[peerName]?.optional === true + ) { continue } + const friendlyPath = nodeIdToFriendlyPath(ctx.nodeId, ctx.dependenciesTree) + const message = `${friendlyPath ? `${friendlyPath}: ` : ''}${packageFriendlyId(ctx.resolvedPackage)} \ +requires a peer of ${peerName}@${peerVersionRange} but none was installed.` + if (ctx.strictPeerDependencies) { + throw new PnpmError('MISSING_PEER_DEPENDENCY', message) + } + logger.warn({ + message, + prefix: ctx.rootDir, + }) + continue } if (!semver.satisfies(resolved.version, peerVersionRange)) { diff --git a/packages/supi/test/install/peerDependencies.ts b/packages/supi/test/install/peerDependencies.ts index e8d7c8c426..21577d9f1c 100644 --- a/packages/supi/test/install/peerDependencies.ts +++ b/packages/supi/test/install/peerDependencies.ts @@ -178,25 +178,53 @@ test('strict-peer-dependencies: error is thrown when cannot resolve peer depende ).rejects.toThrow(/ajv-keywords@1.5.0 requires a peer of ajv@>=4.10.0 but none was installed./) }) -test('warning is not reported if the peer dependency can be required from a node_modules of a parent directory', async () => { - prepareEmpty() +test('peer dependency is resolved from the dependencies of the workspace root project', async () => { + const projects = preparePackages([ + { + location: '.', + package: { name: 'root' }, + }, + { + location: 'pkg', + package: {}, + }, + ]) + const reporter = jest.fn() + await mutateModules([ + { + buildIndex: 0, + manifest: { + name: 'root', + version: '1.0.0', - const manifest = await addDependenciesToPackage({}, ['ajv@4.10.0'], await testDefaults()) + dependencies: { + ajv: '4.10.0', + }, + }, + mutation: 'install', + rootDir: process.cwd(), + }, + { + buildIndex: 0, + manifest: { + name: 'root', + version: '1.0.0', - await fs.mkdir('pkg') + dependencies: { + 'ajv-keywords': '1.5.0', + }, + }, + mutation: 'install', + rootDir: path.resolve('pkg'), + }, + ], await testDefaults({ reporter })) - process.chdir('pkg') - - const reporter = sinon.spy() - - await addDependenciesToPackage(manifest, ['ajv-keywords@1.5.0'], await testDefaults({ reporter })) - - const logMatcher = sinon.match({ + expect(reporter).not.toHaveBeenCalledWith({ message: 'ajv-keywords@1.5.0 requires a peer of ajv@>=4.10.0 but none was installed.', }) - const reportedTimes = reporter.withArgs(logMatcher).callCount - expect(reportedTimes).toBe(0) + const lockfile = await projects.root.readLockfile() + expect(lockfile.importers.pkg?.dependencies?.['ajv-keywords']).toBe('1.5.0_ajv@4.10.0') }) test('warning is reported when cannot resolve peer dependency for non-top-level dependency', async () => { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a8f279e200..d377624745 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2748,7 +2748,6 @@ importers: dependency-path: workspace:8.0.1 encode-registry: ^3.0.0 get-npm-tarball-url: ^2.0.2 - import-from: ^3.0.0 path-exists: ^4.0.0 ramda: ^0.27.1 replace-string: ^3.1.0 @@ -2772,7 +2771,6 @@ importers: dependency-path: link:../dependency-path encode-registry: 3.0.0 get-npm-tarball-url: 2.0.2 - import-from: 3.0.0 path-exists: 4.0.0 ramda: 0.27.1 replace-string: 3.1.0 @@ -8811,13 +8809,6 @@ packages: resolve-from: 4.0.0 dev: true - /import-from/3.0.0: - resolution: {integrity: sha512-CiuXOFFSzkU5x/CR0+z7T91Iht4CXgfCxVOFRhh2Zyhg5wOpWvvDLQUsWl+gcN+QscYBjez8hDCt85O7RLDttQ==} - engines: {node: '>=8'} - dependencies: - resolve-from: 5.0.0 - dev: false - /import-local/3.0.2: resolution: {integrity: sha512-vjL3+w0oulAVZ0hBHnxa/Nm5TAurf9YLQJDhqRZyqb+VKGOB6LU8t9H1Nr5CIo16vh9XfJTOoHwU0B71S557gA==} engines: {node: '>=8'}