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

fix: sync all injected deps when hoisted node linker is used #5630

Merged
merged 4 commits into from Nov 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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