From aecd4acdd6712a0372ebb57f3da0a62e43befe80 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sat, 9 Apr 2022 15:59:52 +0300 Subject: [PATCH] fix: resolve peers from linked in dependencies (#4541) --- .changeset/odd-crabs-shop.md | 7 +++ .../core/test/install/multipleImporters.ts | 6 +-- .../core/test/install/peerDependencies.ts | 43 +++++++++++++++++++ packages/resolve-dependencies/package.json | 1 + packages/resolve-dependencies/src/index.ts | 9 +++- .../resolve-dependencies/src/resolvePeers.ts | 22 +++++++--- pnpm-lock.yaml | 32 +++++++++++++- 7 files changed, 110 insertions(+), 10 deletions(-) create mode 100644 .changeset/odd-crabs-shop.md diff --git a/.changeset/odd-crabs-shop.md b/.changeset/odd-crabs-shop.md new file mode 100644 index 0000000000..ae4ed7d3b4 --- /dev/null +++ b/.changeset/odd-crabs-shop.md @@ -0,0 +1,7 @@ +--- +"@pnpm/core": patch +"@pnpm/resolve-dependencies": patch +"pnpm": patch +--- + +Linked in dependencies should be considered when resolving peer dependencies [#4541](https://github.com/pnpm/pnpm/pull/4541). diff --git a/packages/core/test/install/multipleImporters.ts b/packages/core/test/install/multipleImporters.ts index b69e69b9c2..faf98f1c88 100644 --- a/packages/core/test/install/multipleImporters.ts +++ b/packages/core/test/install/multipleImporters.ts @@ -1163,8 +1163,8 @@ test('resolve a subdependency from the workspace and use it as a peer', async () '/abc-grand-parent-with-c/1.0.0', '/abc-parent-with-ab/1.0.0', '/abc-parent-with-ab/1.0.0_peer-c@1.0.1', - '/abc/1.0.0_20890f3ae006d9839e924c7177030952', - '/abc/1.0.0_peer-a@1.0.1+peer-b@1.0.0', + '/abc/1.0.0_2ff3f699e79762943311edf90e6e1302', + '/abc/1.0.0_peer-a@peer-a+peer-b@1.0.0', '/dep-of-pkg-with-1-dep/100.0.0', '/is-positive/1.0.0', '/peer-b/1.0.0', @@ -1172,7 +1172,7 @@ test('resolve a subdependency from the workspace and use it as a peer', async () ] ) expect(wantedLockfile.packages['/abc-parent-with-ab/1.0.0'].dependencies?.['peer-a']).toBe('link:peer-a') - expect(wantedLockfile.packages['/abc/1.0.0_peer-a@1.0.1+peer-b@1.0.0'].dependencies?.['peer-a']).toBe('link:peer-a') + expect(wantedLockfile.packages['/abc/1.0.0_peer-a@peer-a+peer-b@1.0.0'].dependencies?.['peer-a']).toBe('link:peer-a') }) test('resolve a subdependency from the workspace, when it uses the workspace protocol', async () => { diff --git a/packages/core/test/install/peerDependencies.ts b/packages/core/test/install/peerDependencies.ts index 92227037aa..8a83296894 100644 --- a/packages/core/test/install/peerDependencies.ts +++ b/packages/core/test/install/peerDependencies.ts @@ -1127,3 +1127,46 @@ test('peer dependency that is resolved by a dev dependency', async () => { await project.has('@typegoose/typegoose') await project.hasNot('@types/mongoose') }) + +test('peer dependency is grouped with dependency when peer is resolved not from a top dependency', async () => { + const project1Manifest = { + name: 'project-1', + version: '1.0.0', + dependencies: { + 'ajv-keywords': '1.5.0', + ajv: 'link:../ajv', + }, + } + const project2Manifest = { + name: 'ajv', + version: '4.10.4', + } + preparePackages([ + { + location: 'project-1', + package: project1Manifest, + }, + { + location: 'ajv', + package: project2Manifest, + }, + ]) + const importers: MutatedProject[] = [ + { + buildIndex: 0, + manifest: project1Manifest, + mutation: 'install', + rootDir: path.resolve('project-1'), + }, + { + buildIndex: 0, + manifest: project2Manifest, + mutation: 'install', + rootDir: path.resolve('ajv'), + }, + ] + await mutateModules(importers, await testDefaults({})) + + const lockfile = await readYamlFile(path.resolve(WANTED_LOCKFILE)) + expect(lockfile.packages?.['/ajv-keywords/1.5.0_ajv@ajv'].dependencies?.['ajv']).toBe('link:ajv') +}) diff --git a/packages/resolve-dependencies/package.json b/packages/resolve-dependencies/package.json index fe00682cb1..35af86c9bf 100644 --- a/packages/resolve-dependencies/package.json +++ b/packages/resolve-dependencies/package.json @@ -46,6 +46,7 @@ "@yarnpkg/core": "3.2.0", "dependency-path": "workspace:9.0.0", "encode-registry": "^3.0.0", + "filenamify": "^4.3.0", "get-npm-tarball-url": "^2.0.3", "is-inner-link": "^4.0.0", "is-subdir": "^1.1.1", diff --git a/packages/resolve-dependencies/src/index.ts b/packages/resolve-dependencies/src/index.ts index c816bc6598..aca49c9396 100644 --- a/packages/resolve-dependencies/src/index.ts +++ b/packages/resolve-dependencies/src/index.ts @@ -131,7 +131,7 @@ export default async function ( ) } - const topParents = project.manifest + const topParents: Array<{ name: string, version: string, linkedDir?: string }> = project.manifest ? await getTopParents( difference( Object.keys(getAllDependenciesFromManifest(project.manifest)), @@ -142,6 +142,13 @@ export default async function ( project.modulesDir ) : [] + resolvedImporter.linkedDependencies.forEach((linkedDependency) => { + topParents.push({ + name: linkedDependency.alias, + version: linkedDependency.version, + linkedDir: `link:${path.relative(opts.lockfileDir, linkedDependency.resolution.directory)}`, + }) + }) const manifest = updatedOriginalManifest ?? project.originalManifest ?? project.manifest importers[index].manifest = manifest diff --git a/packages/resolve-dependencies/src/resolvePeers.ts b/packages/resolve-dependencies/src/resolvePeers.ts index fadfb41cfd..07d7493d82 100644 --- a/packages/resolve-dependencies/src/resolvePeers.ts +++ b/packages/resolve-dependencies/src/resolvePeers.ts @@ -1,4 +1,5 @@ import crypto from 'crypto' +import filenamify from 'filenamify' import path from 'path' import { satisfiesWithPrereleases } from '@yarnpkg/core/lib/semverUtils' import { @@ -117,16 +118,17 @@ function createPkgsByName ( dependenciesTree: DependenciesTree, { directNodeIdsByAlias, topParents }: { directNodeIdsByAlias: {[alias: string]: string} - topParents: Array<{name: string, version: string}> + topParents: Array<{name: string, version: string, linkedDir?: string}> } ) { return Object.assign( fromPairs( - topParents.map(({ name, version }): KeyValuePair => [ + topParents.map(({ name, version, linkedDir }): KeyValuePair => [ name, { depth: 0, version, + nodeId: linkedDir, }, ]) ), @@ -249,9 +251,19 @@ function resolvePeersOfNode ( depPath = resolvedPackage.depPath } else { const peersFolderSuffix = createPeersFolderSuffix( - Object.keys(allResolvedPeers) - .map((alias) => ctx.dependenciesTree[allResolvedPeers[alias]].resolvedPackage) - .map(({ name, version }) => ({ name, version }))) + Object.entries(allResolvedPeers) + .map(([alias, nodeId]) => { + if (nodeId.startsWith('link:')) { + const linkedDir = nodeId.slice(5) + return { + name: alias, + version: filenamify(linkedDir, { replacement: '+' }), + } + } + const { name, version } = ctx.dependenciesTree[nodeId].resolvedPackage + return { name, version } + }) + ) depPath = `${resolvedPackage.depPath}${peersFolderSuffix}` } const localLocation = path.join(ctx.virtualStoreDir, depPathToFilename(depPath)) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 72a2950b7d..af6eaaa1f8 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -3178,6 +3178,7 @@ importers: '@yarnpkg/core': 3.2.0 dependency-path: workspace:9.0.0 encode-registry: ^3.0.0 + filenamify: ^4.3.0 get-npm-tarball-url: ^2.0.3 is-inner-link: ^4.0.0 is-subdir: ^1.1.1 @@ -3205,6 +3206,7 @@ importers: '@yarnpkg/core': 3.2.0 dependency-path: link:../dependency-path encode-registry: 3.0.0 + filenamify: 4.3.0 get-npm-tarball-url: 2.0.3 is-inner-link: 4.0.0 is-subdir: 1.2.0 @@ -8257,7 +8259,7 @@ packages: /eslint-config-standard-with-typescript/21.0.1_716c3012ce9c6cb0db5ff56082353a72: resolution: {integrity: sha512-FeiMHljEJ346Y0I/HpAymNKdrgKEpHpcg/D93FvPHWfCzbT4QyUJba/0FwntZeGLXfUiWDSeKmdJD597d9wwiw==} peerDependencies: - '@typescript-eslint/eslint-plugin': ^4.0.1 || ^5.6.0 || ^5.6.0 + '@typescript-eslint/eslint-plugin': ^4.0.1 || ^5.6.0 eslint: '*' eslint-plugin-import: ^2.22.1 eslint-plugin-node: ^11.1.0 @@ -8769,6 +8771,20 @@ packages: dependencies: flat-cache: 3.0.4 + /filename-reserved-regex/2.0.0: + resolution: {integrity: sha1-q/c9+rc10EVECr/qLZHzieu/oik=} + engines: {node: '>=4'} + dev: false + + /filenamify/4.3.0: + resolution: {integrity: sha512-hcFKyUG57yWGAzu1CMt/dPzYZuv+jAJUT85bL8mrXvNe6hWj6yEHEc4EdcgiA6Z3oi1/9wXJdZPXF2dZNgwgOg==} + engines: {node: '>=8'} + dependencies: + filename-reserved-regex: 2.0.0 + strip-outer: 1.0.1 + trim-repeated: 1.0.0 + dev: false + /fill-keys/1.0.2: resolution: {integrity: sha1-mo+jb06K1jTjv2tPPIiCVRRS6yA=} engines: {node: '>=0.10.0'} @@ -13883,6 +13899,13 @@ packages: resolution: {integrity: sha512-6fPc+R4ihwqP6N/aIv2f1gMH8lOVtWQHoqC4yK6oSDVVocumAsfCqjkXnqiYMhmMwS/mEHLp7Vehlt3ql6lEig==} engines: {node: '>=8'} + /strip-outer/1.0.1: + resolution: {integrity: sha512-k55yxKHwaXnpYGsOzg4Vl8+tDrWylxDEpknGjhTiZB8dFRU5rTo9CAzeycivxV3s+zlTKwrs6WxMxR95n26kwg==} + engines: {node: '>=0.10.0'} + dependencies: + escape-string-regexp: 1.0.5 + dev: false + /supports-color/5.5.0: resolution: {integrity: sha512-QjVjwdXIt408MIiAqCX4oUKsgU2EqAGzs2Ppkm4aQYbjm+ZEWEcW4SfFNTr4uMNZma0ey4f5lgLrkB0aX0QMow==} engines: {node: '>=4'} @@ -14216,6 +14239,13 @@ packages: engines: {node: '>=12'} dev: true + /trim-repeated/1.0.0: + resolution: {integrity: sha1-42RqLqTokTEr9+rObPsFOAvAHCE=} + engines: {node: '>=0.10.0'} + dependencies: + escape-string-regexp: 1.0.5 + dev: false + /trim-trailing-lines/1.1.4: resolution: {integrity: sha512-rjUWSqnfTNrjbB9NQWfPMH/xRK1deHeGsHoVfpxJ++XeYXE0d6B1En37AHfw3jtfTU7dzMzZL2jjpe8Qb5gLIQ==} dev: false