Skip to content

Commit

Permalink
fix: error that sometimes happen on projects with linked deps (#5628)
Browse files Browse the repository at this point in the history
close #5327
close #5614
  • Loading branch information
zkochan committed Nov 14, 2022
1 parent 9ad96e2 commit 4a4b2ac
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 2 deletions.
5 changes: 5 additions & 0 deletions .changeset/mighty-waves-tan.md
@@ -0,0 +1,5 @@
---
"pnpm": patch
---

Fix an error that sometimes happen on projects with linked local dependencies [#5327](https://github.com/pnpm/pnpm/issues/5327).
5 changes: 5 additions & 0 deletions .changeset/poor-berries-yell.md
@@ -0,0 +1,5 @@
---
"@pnpm/resolve-dependencies": patch
---

Fix the nodeId in dependenciesTree for linked local packages.
77 changes: 77 additions & 0 deletions packages/core/test/install/local.ts
Expand Up @@ -8,6 +8,8 @@ import { fixtures } from '@pnpm/test-fixtures'
import {
addDependenciesToPackage,
install,
mutateModules,
MutatedProject,
mutateModulesInSingleProject,
} from '@pnpm/core'
import rimraf from '@zkochan/rimraf'
Expand Down Expand Up @@ -276,3 +278,78 @@ test('deep local', async () => {
const lockfile = await readYamlFile<Lockfile>('pnpm-lock.yaml')
expect(Object.keys(lockfile.packages ?? {})).toStrictEqual(['file:../project-2', 'file:../project-2/project-3'])
})

// Covers https://github.com/pnpm/pnpm/issues/5327
test('resolution should not fail when a peer is resolved from a local package and there are many circular dependencies', async () => {
const manifest1 = {
name: 'chained-iterator',
version: '0.0.4',
dependencies: {
'@bryntum/siesta': '6.0.0-beta-1',
},
}
const manifest2 = {
name: '@bryntum/chronograph',
version: '2.0.3',
dependencies: {
'@bryntum/siesta': '6.0.0-beta-1',
'typescript-serializable-mixin': '0.0.3',
'typescript-mixin-class': 'link:../typescript-mixin-class',
},
}
const manifest3 = {
name: 'typescript-mixin-class',
version: '0.0.3',
dependencies: {
'@bryntum/siesta': '6.0.0-beta-1',
},
}

preparePackages([
{
location: manifest1.name,
package: manifest1,
},
{
location: manifest2.name,
package: manifest2,
},
{
location: manifest3.name,
package: manifest3,
},
])
const importers: MutatedProject[] = [
{
mutation: 'install',
rootDir: path.resolve(manifest1.name),
},
{
mutation: 'install',
rootDir: path.resolve(manifest2.name),
},
{
mutation: 'install',
rootDir: path.resolve(manifest3.name),
},
]
const allProjects = [
{
buildIndex: 0,
manifest: manifest1,
rootDir: path.resolve(manifest1.name),
},
{
buildIndex: 0,
manifest: manifest2,
rootDir: path.resolve(manifest2.name),
},
{
buildIndex: 0,
manifest: manifest3,
rootDir: path.resolve(manifest3.name),
},
]
await mutateModules(importers, await testDefaults({ allProjects, lockfileOnly: true, strictPeerDependencies: false }))
// All we need to know in this test is that installation doesn't fail
})
2 changes: 2 additions & 0 deletions packages/resolve-dependencies/package.json
Expand Up @@ -50,6 +50,7 @@
"get-npm-tarball-url": "^2.0.3",
"is-inner-link": "^4.0.0",
"is-subdir": "^1.2.0",
"normalize-path": "^3.0.0",
"p-defer": "^3.0.0",
"path-exists": "^4.0.0",
"promise-share": "^1.0.0",
Expand All @@ -63,6 +64,7 @@
},
"devDependencies": {
"@pnpm/resolve-dependencies": "workspace:*",
"@types/normalize-path": "^3.0.0",
"@types/ramda": "0.28.15",
"@types/semver": "7.3.13"
},
Expand Down
3 changes: 2 additions & 1 deletion packages/resolve-dependencies/src/index.ts
Expand Up @@ -25,6 +25,7 @@ import promiseShare from 'promise-share'
import difference from 'ramda/src/difference'
import { getWantedDependencies, WantedDependency } from './getWantedDependencies'
import { depPathToRef } from './depPathToRef'
import { createNodeIdForLinkedLocalPkg } from './resolveDependencies'
import {
Importer,
LinkedDependency,
Expand Down Expand Up @@ -179,7 +180,7 @@ export async function resolveDependencies (
topParents.push({
name: linkedDependency.alias,
version: linkedDependency.version,
linkedDir: `link:${path.relative(opts.lockfileDir, linkedDependency.resolution.directory)}`,
linkedDir: createNodeIdForLinkedLocalPkg(opts.lockfileDir, linkedDependency.resolution.directory),
})
})

Expand Down
7 changes: 6 additions & 1 deletion packages/resolve-dependencies/src/resolveDependencies.ts
Expand Up @@ -39,6 +39,7 @@ import {
Registries,
} from '@pnpm/types'
import * as dp from 'dependency-path'
import normalizePath from 'normalize-path'
import exists from 'path-exists'
import pDefer from 'p-defer'
import pShare from 'promise-share'
Expand Down Expand Up @@ -665,7 +666,7 @@ async function resolveDependenciesOfDependency (

if (resolveDependencyResult == null) return { resolveDependencyResult: null }
if (resolveDependencyResult.isLinkedDependency) {
ctx.dependenciesTree[resolveDependencyResult.pkgId] = {
ctx.dependenciesTree[createNodeIdForLinkedLocalPkg(ctx.lockfileDir, resolveDependencyResult.resolution.directory)] = {
children: {},
depth: -1,
installable: true,
Expand Down Expand Up @@ -708,6 +709,10 @@ async function resolveDependenciesOfDependency (
}
}

export function createNodeIdForLinkedLocalPkg (lockfileDir: string, pkgDir: string) {
return `link:${normalizePath(path.relative(lockfileDir, pkgDir))}`
}

function filterMissingPeers (
{ missingPeers, resolvedPeers }: PeersResolutionResult,
parentPkgAliases: ParentPkgAliases
Expand Down
6 changes: 6 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 4a4b2ac

Please sign in to comment.