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

feat: save the locations of injected deps to the modules state file #4259

Merged
merged 6 commits into from
Jan 20, 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
5 changes: 5 additions & 0 deletions .changeset/big-lemons-learn.md
@@ -0,0 +1,5 @@
---
"pnpm": patch
---

[Injected dependencies](https://pnpm.io/package_json#dependenciesmetainjected) should work properly in projects that use the hoisted node linker [#4259](https://github.com/pnpm/pnpm/pull/4259).
6 changes: 6 additions & 0 deletions .changeset/nasty-walls-live.md
@@ -0,0 +1,6 @@
---
"@pnpm/core": minor
"@pnpm/headless": minor
---

All the locations of injected dependencies are saved in the modules state file at `node_modules/.modules.yaml`.
5 changes: 5 additions & 0 deletions .changeset/selfish-llamas-mate.md
@@ -0,0 +1,5 @@
---
"@pnpm/modules-yaml": minor
---

New field added: injectedDeps.
5 changes: 5 additions & 0 deletions .changeset/spotty-bugs-repair.md
@@ -0,0 +1,5 @@
---
"@pnpm/lockfile-utils": minor
---

Injected package location should be properly detected in a hoisted `node_modules`.
34 changes: 21 additions & 13 deletions packages/core/src/install/index.ts
Expand Up @@ -308,7 +308,6 @@ export async function mutateModules (

const projectsToInstall = [] as ImporterToUpdate[]

const projectsToBeInstalled = ctx.projects.filter(({ mutation }) => mutation === 'install') as ProjectToBeInstalled[]
let preferredSpecs: Record<string, string> | null = null

// TODO: make it concurrent
Expand Down Expand Up @@ -460,21 +459,10 @@ export async function mutateModules (
makePartialCurrentLockfile,
needsFullResolution,
pruneVirtualStore,
scriptsOpts,
updateLockfileMinorVersion: true,
})

if (!opts.ignoreScripts) {
if (opts.enablePnp) {
scriptsOpts.extraEnv = makeNodeRequireOption(path.join(opts.lockfileDir, '.pnp.cjs'))
}
const projectsToBeBuilt = extendProjectsWithTargetDirs(projectsToBeInstalled, result.newLockfile, ctx)
await runLifecycleHooksConcurrently(['preinstall', 'install', 'postinstall', 'prepare'],
projectsToBeBuilt,
opts.childConcurrency,
scriptsOpts
)
}

return result.projects
}
}
Expand Down Expand Up @@ -625,6 +613,7 @@ type InstallFunction = (
updateLockfileMinorVersion: boolean
preferredVersions?: PreferredVersions
pruneVirtualStore: boolean
scriptsOpts: RunLifecycleHooksConcurrentlyOptions
currentLockfileIsUpToDate: boolean
}
) => Promise<InstallFunctionResult>
Expand Down Expand Up @@ -907,6 +896,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
}
}))

const projectsWithTargetDirs = extendProjectsWithTargetDirs(projects, newLockfile, ctx)
await Promise.all([
opts.useLockfile
? writeLockfiles({
Expand All @@ -921,11 +911,18 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
if (result.currentLockfile.packages === undefined && result.removedDepPaths.size === 0) {
return Promise.resolve()
}
const injectedDeps = {}
for (const project of projectsWithTargetDirs) {
if (project.targetDirs.length > 0) {
injectedDeps[project.id] = project.targetDirs.map((targetDir) => path.relative(opts.lockfileDir, targetDir))
}
}
return writeModulesYaml(ctx.rootModulesDir, {
...ctx.modulesFile,
hoistedDependencies: result.newHoistedDependencies,
hoistPattern: ctx.hoistPattern,
included: ctx.include,
injectedDeps,
layoutVersion: LAYOUT_VERSION,
nodeLinker: opts.nodeLinker,
packageManager: `${opts.packageManager.name}@${opts.packageManager.version}`,
Expand All @@ -941,6 +938,17 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
})
})(),
])
if (!opts.ignoreScripts) {
if (opts.enablePnp) {
opts.scriptsOpts.extraEnv = makeNodeRequireOption(path.join(opts.lockfileDir, '.pnp.cjs'))
}
const projectsToBeBuilt = projectsWithTargetDirs.filter(({ mutation }) => mutation === 'install') as ProjectToBeInstalled[]
await runLifecycleHooksConcurrently(['preinstall', 'install', 'postinstall', 'prepare'],
projectsToBeBuilt,
opts.childConcurrency,
opts.scriptsOpts
)
}
} else {
await finishLockfileUpdates()
if (opts.useLockfile) {
Expand Down
170 changes: 170 additions & 0 deletions packages/core/test/install/injectLocalPackages.ts
Expand Up @@ -161,6 +161,11 @@ test('inject local packages', async () => {
transitivePeerDependencies: ['is-positive'],
dev: false,
})

const modulesState = await rootModules.readModulesManifest()
expect(modulesState?.injectedDeps?.['project-1'].length).toEqual(2)
expect(modulesState?.injectedDeps?.['project-1'][0]).toContain(`node_modules${path.sep}.pnpm`)
expect(modulesState?.injectedDeps?.['project-1'][1]).toContain(`node_modules${path.sep}.pnpm`)
}

await rimraf('node_modules')
Expand Down Expand Up @@ -212,6 +217,10 @@ test('inject local packages', async () => {
},
dev: false,
})
const modulesState = await rootModules.readModulesManifest()
expect(modulesState?.injectedDeps?.['project-1'].length).toEqual(2)
expect(modulesState?.injectedDeps?.['project-1'][0]).toContain(`node_modules${path.sep}.pnpm`)
expect(modulesState?.injectedDeps?.['project-1'][1]).toContain(`node_modules${path.sep}.pnpm`)
}
})

Expand Down Expand Up @@ -341,3 +350,164 @@ test('inject local packages and relink them after build', async () => {

expect(await pathExists(path.resolve('project-2/node_modules/project-1/main.js'))).toBeTruthy()
})

test('inject local packages when node-linker is hoisted', async () => {
const project1Manifest = {
name: 'project-1',
version: '1.0.0',
dependencies: {
'is-negative': '1.0.0',
},
devDependencies: {
'dep-of-pkg-with-1-dep': '100.0.0',
},
peerDependencies: {
'is-positive': '>=1.0.0',
},
}
const project2Manifest = {
name: 'project-2',
version: '1.0.0',
dependencies: {
'project-1': 'workspace:1.0.0',
},
devDependencies: {
'is-positive': '1.0.0',
},
dependenciesMeta: {
'project-1': {
injected: true,
},
},
}
const project3Manifest = {
name: 'project-3',
version: '1.0.0',
dependencies: {
'project-2': 'workspace:1.0.0',
},
devDependencies: {
'is-positive': '2.0.0',
},
dependenciesMeta: {
'project-2': {
injected: true,
},
},
}
const projects = preparePackages([
{
location: 'project-1',
package: project1Manifest,
},
{
location: 'project-2',
package: project2Manifest,
},
{
location: 'project-3',
package: project3Manifest,
},
])

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

const rootModules = assertProject(process.cwd())
await rootModules.has('is-negative')
await rootModules.has('dep-of-pkg-with-1-dep')
await rootModules.has('is-positive')

await projects['project-2'].has('project-1')

await projects['project-3'].has('project-1')
await projects['project-3'].has('project-2')
await projects['project-3'].has('is-positive')

{
const lockfile = await rootModules.readLockfile()
expect(lockfile.importers['project-2'].dependenciesMeta).toEqual({
'project-1': {
injected: true,
},
})
expect(lockfile.packages['file:project-1_is-positive@1.0.0']).toEqual({
resolution: {
directory: 'project-1',
type: 'directory',
},
id: 'file:project-1',
name: 'project-1',
version: '1.0.0',
peerDependencies: {
'is-positive': '>=1.0.0',
},
dependencies: {
'is-negative': '1.0.0',
'is-positive': '1.0.0',
},
dev: false,
})
expect(lockfile.packages['file:project-2_is-positive@2.0.0']).toEqual({
resolution: {
directory: 'project-2',
type: 'directory',
},
id: 'file:project-2',
name: 'project-2',
version: '1.0.0',
dependencies: {
'project-1': 'file:project-1_is-positive@2.0.0',
},
transitivePeerDependencies: ['is-positive'],
dev: false,
})

const modulesState = await rootModules.readModulesManifest()
expect(modulesState?.injectedDeps?.['project-1'].length).toEqual(2)
expect(modulesState?.injectedDeps?.['project-1'][0]).toEqual(path.join('project-2', 'node_modules', 'project-1'))
expect(modulesState?.injectedDeps?.['project-1'][1]).toEqual(path.join('project-3', 'node_modules', 'project-1'))
}
})
18 changes: 14 additions & 4 deletions packages/headless/src/index.ts
Expand Up @@ -244,6 +244,7 @@ export default async (opts: HeadlessOptions) => {
directDependenciesByImporterId,
graph,
hierarchy,
pkgLocationByDepPath,
prevGraph,
symlinkedDirectDependenciesByImporterId,
} = await (
Expand Down Expand Up @@ -427,6 +428,12 @@ export default async (opts: HeadlessOptions) => {
})
}

const projectsToBeBuilt = extendProjectsWithTargetDirs(opts.projects, wantedLockfile, {
lockfileDir: opts.lockfileDir,
pkgLocationByDepPath,
virtualStoreDir,
})

if (opts.enableModulesDir !== false) {
/** Skip linking and due to no project manifest */
if (!opts.ignorePackageManifest) {
Expand Down Expand Up @@ -454,10 +461,17 @@ export default async (opts: HeadlessOptions) => {
}
}))
}
const injectedDeps = {}
for (const project of projectsToBeBuilt) {
if (project.targetDirs.length > 0) {
injectedDeps[project.id] = project.targetDirs.map((targetDir) => path.relative(opts.lockfileDir, targetDir))
}
}
await writeModulesYaml(rootModulesDir, {
hoistedDependencies: newHoistedDependencies,
hoistPattern: opts.hoistPattern,
included: opts.include,
injectedDeps,
layoutVersion: LAYOUT_VERSION,
nodeLinker: opts.nodeLinker,
packageManager: `${opts.packageManager.name}@${opts.packageManager.version}`,
Expand All @@ -482,10 +496,6 @@ export default async (opts: HeadlessOptions) => {
await opts.storeController.close()

if (!opts.ignoreScripts && !opts.ignorePackageManifest) {
const projectsToBeBuilt = extendProjectsWithTargetDirs(opts.projects, wantedLockfile, {
lockfileDir: opts.lockfileDir,
virtualStoreDir,
})
await runLifecycleHooksConcurrently(
['preinstall', 'install', 'postinstall', 'prepare'],
projectsToBeBuilt,
Expand Down
1 change: 1 addition & 0 deletions packages/headless/src/lockfileToDepGraph.ts
Expand Up @@ -80,6 +80,7 @@ export interface LockfileToDepGraphResult {
hierarchy?: DepHierarchy
symlinkedDirectDependenciesByImporterId?: DirectDependenciesByImporterId
prevGraph?: DependenciesGraph
pkgLocationByDepPath?: Record<string, string>
}

export default async function lockfileToDepGraph (
Expand Down
1 change: 1 addition & 0 deletions packages/headless/src/lockfileToHoistedDepGraph.ts
Expand Up @@ -97,6 +97,7 @@ async function _lockfileToHoistedDepGraph (
directDependenciesByImporterId,
graph,
hierarchy,
pkgLocationByDepPath: fetchDepsOpts.pkgLocationByDepPath,
symlinkedDirectDependenciesByImporterId,
}
}
Expand Down
12 changes: 8 additions & 4 deletions packages/lockfile-utils/src/extendProjectsWithTargetDirs.ts
Expand Up @@ -9,19 +9,23 @@ export default function extendProjectsWithTargetDirs<T> (
ctx: {
lockfileDir: string
virtualStoreDir: string
pkgLocationByDepPath?: Record<string, string>
}
) {
const projectsById: Record<string, T & { targetDirs: string[], stages?: 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, ctx.lockfileDir), '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 ?? {})
.forEach(([depPath, pkg]) => {
if (pkg.resolution?.['type'] !== 'directory') return
const pkgId = pkg.id ?? depPath
const importerId = pkgId.replace(/^file:/, '')
if (projectsById[importerId] == null) return
const localLocation = path.join(ctx.virtualStoreDir, depPathToFilename(depPath, ctx.lockfileDir), 'node_modules', pkg.name!)
const localLocation = getLocalLocation(depPath, pkg.name!)
projectsById[importerId].targetDirs.push(localLocation)
projectsById[importerId].stages = ['preinstall', 'install', 'postinstall', 'prepare', 'prepublishOnly']
})
return Object.values(projectsById)
return Object.values(projectsById) as Array<T & { id: string, stages: string[], targetDirs: string[] }>
}