Skip to content

Commit

Permalink
fix: sync all injected deps when hoisted node linker is used (#5630)
Browse files Browse the repository at this point in the history
  • Loading branch information
zkochan committed Nov 15, 2022
1 parent 9c1e144 commit ecc8794
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 17 deletions.
7 changes: 7 additions & 0 deletions .changeset/olive-icons-glow.md
@@ -0,0 +1,7 @@
---
"@pnpm/headless": patch
"@pnpm/lockfile-utils": patch
"pnpm": patch
---

Sync all injected dependencies when hoisted node linker is used.
5 changes: 5 additions & 0 deletions .changeset/rare-seas-grin.md
@@ -0,0 +1,5 @@
---
"@pnpm/lockfile-utils": major
---

Breaking change to the API of the `extendProjectsWithTargetDirs` function.
124 changes: 124 additions & 0 deletions packages/core/test/install/injectLocalPackages.ts
Expand Up @@ -1483,3 +1483,127 @@ test('do not modify the manifest of the injected workpspace project', async () =
},
})
})

test('injected package is kept up-to-date when it is hoisted to multiple places', async () => {
// We create a root project with is-positive in the dependencies, so that the local is-positive
// inside project-1 and project-2 will be nested into their node_modules
const rootProjectManifest = {
name: 'project-1',
version: '1.0.0',
dependencies: {
'is-positive': '2.0.0',
},
}
const project1Manifest = {
name: 'project-1',
version: '1.0.0',
dependencies: {
'is-positive': 'workspace:1.0.0',
},
dependenciesMeta: {
'is-positive': {
injected: true,
},
},
}
const project2Manifest = {
name: 'project-2',
version: '1.0.0',
dependencies: {
'is-positive': 'workspace:1.0.0',
},
dependenciesMeta: {
'is-positive': {
injected: true,
},
},
}
const project3Manifest = {
name: 'is-positive',
version: '1.0.0',
scripts: {
prepare: 'node -e "require(\'fs\').writeFileSync(\'prepare.txt\', \'prepare\', \'utf8\')"',
},
}
const projects = preparePackages([
{
location: '',
package: rootProjectManifest,
},
{
location: 'project-1',
package: project1Manifest,
},
{
location: 'project-2',
package: project2Manifest,
},
{
location: 'project-3',
package: project3Manifest,
},
])

const importers: MutatedProject[] = [
{
mutation: 'install',
rootDir: process.cwd(),
},
{
mutation: 'install',
rootDir: path.resolve('project-1'),
},
{
mutation: 'install',
rootDir: path.resolve('project-2'),
},
{
mutation: 'install',
rootDir: path.resolve('project-3'),
},
]
const allProjects = [
{
buildIndex: 0,
manifest: rootProjectManifest,
rootDir: process.cwd(),
},
{
buildIndex: 0,
manifest: project1Manifest,
rootDir: path.resolve('project-1'),
},
{
buildIndex: 0,
manifest: project2Manifest,
rootDir: path.resolve('project-2'),
},
{
buildIndex: 0,
manifest: project3Manifest,
rootDir: path.resolve('project-3'),
},
]
const workspacePackages = {
'is-positive': {
'1.0.0': {
dir: path.resolve('project-3'),
manifest: project1Manifest,
},
},
}
await mutateModules(importers, await testDefaults({
allProjects,
nodeLinker: 'hoisted',
workspacePackages,
}))

await projects['project-1'].has('is-positive/prepare.txt')
await projects['project-2'].has('is-positive/prepare.txt')

const rootModules = assertProject(process.cwd())
const modulesState = await rootModules.readModulesManifest()
expect(modulesState?.injectedDeps?.['project-3'].length).toEqual(2)
expect(modulesState?.injectedDeps?.['project-3'][0]).toEqual(path.join('project-1', 'node_modules', 'is-positive'))
expect(modulesState?.injectedDeps?.['project-3'][1]).toEqual(path.join('project-2', 'node_modules', 'is-positive'))
})
4 changes: 2 additions & 2 deletions packages/headless/src/index.ts
Expand Up @@ -278,7 +278,7 @@ export async function headlessInstall (opts: HeadlessOptions) {
directDependenciesByImporterId,
graph,
hierarchy,
pkgLocationByDepPath,
pkgLocationsByDepPath,
prevGraph,
symlinkedDirectDependenciesByImporterId,
} = await (
Expand Down Expand Up @@ -475,7 +475,7 @@ export async function headlessInstall (opts: HeadlessOptions) {
}

const projectsToBeBuilt = extendProjectsWithTargetDirs(selectedProjects, wantedLockfile, {
pkgLocationByDepPath,
pkgLocationsByDepPath,
virtualStoreDir,
})

Expand Down
2 changes: 1 addition & 1 deletion packages/headless/src/lockfileToDepGraph.ts
Expand Up @@ -82,7 +82,7 @@ export interface LockfileToDepGraphResult {
hierarchy?: DepHierarchy
symlinkedDirectDependenciesByImporterId?: DirectDependenciesByImporterId
prevGraph?: DependenciesGraph
pkgLocationByDepPath?: Record<string, string>
pkgLocationsByDepPath?: Record<string, string[]>
}

export async function lockfileToDepGraph (
Expand Down
19 changes: 11 additions & 8 deletions packages/headless/src/lockfileToHoistedDepGraph.ts
Expand Up @@ -71,7 +71,7 @@ async function _lockfileToHoistedDepGraph (
...opts,
lockfile,
graph,
pkgLocationByDepPath: {},
pkgLocationsByDepPath: {},
}
const hierarchy = {
[opts.lockfileDir]: await fetchDeps(fetchDepsOpts, modulesDir, tree.dependencies),
Expand Down Expand Up @@ -99,7 +99,7 @@ async function _lockfileToHoistedDepGraph (
directDependenciesByImporterId,
graph,
hierarchy,
pkgLocationByDepPath: fetchDepsOpts.pkgLocationByDepPath,
pkgLocationsByDepPath: fetchDepsOpts.pkgLocationsByDepPath,
symlinkedDirectDependenciesByImporterId,
}
}
Expand Down Expand Up @@ -135,7 +135,7 @@ async function fetchDeps (
opts: {
graph: DependenciesGraph
lockfile: Lockfile
pkgLocationByDepPath: Record<string, string>
pkgLocationsByDepPath: Record<string, string[]>
} & LockfileToHoistedDepGraphOptions,
modules: string,
deps: Set<HoisterResult>
Expand Down Expand Up @@ -211,16 +211,19 @@ async function fetchDeps (
requiresBuild: pkgSnapshot.requiresBuild === true,
patchFile: opts.patchedDependencies?.[`${pkgName}@${pkgVersion}`],
}
opts.pkgLocationByDepPath[depPath] = dir
if (!opts.pkgLocationsByDepPath[depPath]) {
opts.pkgLocationsByDepPath[depPath] = []
}
opts.pkgLocationsByDepPath[depPath].push(dir)
depHierarchy[dir] = await fetchDeps(opts, path.join(dir, 'node_modules'), dep.dependencies)
opts.graph[dir].children = getChildren(pkgSnapshot, opts.pkgLocationByDepPath, opts)
opts.graph[dir].children = getChildren(pkgSnapshot, opts.pkgLocationsByDepPath, opts)
}))
return depHierarchy
}

function getChildren (
pkgSnapshot: PackageSnapshot,
pkgLocationByDepPath: Record<string, string>,
pkgLocationsByDepPath: Record<string, string[]>,
opts: { include: IncludedDependencies }
) {
const allDeps = {
Expand All @@ -230,8 +233,8 @@ function getChildren (
const children = {}
for (const [childName, childRef] of Object.entries(allDeps)) {
const childDepPath = dp.refToRelative(childRef, childName)
if (childDepPath && pkgLocationByDepPath[childDepPath]) {
children[childName] = pkgLocationByDepPath[childDepPath]
if (childDepPath && pkgLocationsByDepPath[childDepPath]) {
children[childName] = pkgLocationsByDepPath[childDepPath][0]
}
}
return children
Expand Down
14 changes: 8 additions & 6 deletions packages/lockfile-utils/src/extendProjectsWithTargetDirs.ts
Expand Up @@ -3,17 +3,19 @@ import { Lockfile } from '@pnpm/lockfile-types'
import { depPathToFilename } from 'dependency-path'
import fromPairs from 'ramda/src/fromPairs'

type GetLocalLocations = (depPath: string, pkgName: string) => string[]

export function extendProjectsWithTargetDirs<T> (
projects: Array<T & { id: string }>,
lockfile: Lockfile,
ctx: {
virtualStoreDir: string
pkgLocationByDepPath?: Record<string, string>
pkgLocationsByDepPath?: Record<string, string[]>
}
): Array<T & { id: string, stages: string[], targetDirs: string[] }> {
const getLocalLocation = ctx.pkgLocationByDepPath != null
? (depPath: string) => ctx.pkgLocationByDepPath![depPath]
: (depPath: string, pkgName: string) => path.join(ctx.virtualStoreDir, depPathToFilename(depPath), 'node_modules', pkgName)
const getLocalLocations: GetLocalLocations = ctx.pkgLocationsByDepPath != null
? (depPath: string) => ctx.pkgLocationsByDepPath![depPath]
: (depPath: string, pkgName: string) => [path.join(ctx.virtualStoreDir, depPathToFilename(depPath), 'node_modules', pkgName)]
const projectsById: Record<string, T & { id: string, targetDirs: string[], stages?: string[] }> =
fromPairs(projects.map((project) => [project.id, { ...project, targetDirs: [] as string[] }]))
Object.entries(lockfile.packages ?? {})
Expand All @@ -22,8 +24,8 @@ export function extendProjectsWithTargetDirs<T> (
const pkgId = pkg.id ?? depPath
const importerId = pkgId.replace(/^file:/, '')
if (projectsById[importerId] == null) return
const localLocation = getLocalLocation(depPath, pkg.name!)
projectsById[importerId].targetDirs.push(localLocation)
const localLocations = getLocalLocations(depPath, pkg.name!)
projectsById[importerId].targetDirs.push(...localLocations)
projectsById[importerId].stages = ['preinstall', 'install', 'postinstall', 'prepare', 'prepublishOnly']
})
return Object.values(projectsById) as Array<T & { id: string, stages: string[], targetDirs: string[] }>
Expand Down

0 comments on commit ecc8794

Please sign in to comment.