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: dedupe injected dependencies #7416

Merged
merged 15 commits into from Dec 14, 2023
291 changes: 291 additions & 0 deletions pkg-manager/core/test/install/injectLocalPackages.ts
Expand Up @@ -1780,3 +1780,294 @@ test('do not relink injected dependency on install when disableRelinkLocalDirDep

expect(newInode).toEqual(getInode())
})

test('injected local packages are deduped', async () => {
const project1Manifest = {
name: 'project-1',
version: '1.0.0',
dependencies: {
'is-negative': '1.0.0',
},
devDependencies: {
'@pnpm.e2e/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 project4Manifest = {
name: 'project-4',
version: '1.0.0',
dependencies: {
'project-2': 'workspace:1.0.0',
},
devDependencies: {
'is-positive': '1.0.0',
},
dependenciesMeta: {
'project-2': {
injected: true,
},
},
}
const project5Manifest = {
name: 'project-5',
version: '1.0.0',
dependencies: {
'project-4': 'workspace:1.0.0',
},
devDependencies: {
'is-positive': '1.0.0',
},
dependenciesMeta: {
'project-4': {
injected: true,
},
},
}
const projects = preparePackages([
{
location: 'project-1',
package: project1Manifest,
},
{
location: 'project-2',
package: project2Manifest,
},
{
location: 'project-3',
package: project3Manifest,
},
{
location: 'project-4',
package: project4Manifest,
},
{
location: 'project-5',
package: project5Manifest,
},
])

const importers: MutatedProject[] = [
{
mutation: 'install',
rootDir: path.resolve('project-1'),
},
{
mutation: 'install',
rootDir: path.resolve('project-2'),
},
{
mutation: 'install',
rootDir: path.resolve('project-3'),
},
{
mutation: 'install',
rootDir: path.resolve('project-4'),
},
{
mutation: 'install',
rootDir: path.resolve('project-5'),
},
]
const allProjects: ProjectOptions[] = [
{
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'),
},
{
buildIndex: 0,
manifest: project4Manifest,
rootDir: path.resolve('project-4'),
},
{
buildIndex: 0,
manifest: project5Manifest,
rootDir: path.resolve('project-5'),
},
]
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: project3Manifest,
},
},
'project-4': {
'1.0.0': {
dir: path.resolve('project-4'),
manifest: project4Manifest,
},
},
}
await mutateModules(importers, await testDefaults({
autoInstallPeers: true,
allProjects,
workspacePackages,
}))

await projects['project-1'].has('is-negative')
await projects['project-1'].has('@pnpm.e2e/dep-of-pkg-with-1-dep')
await projects['project-1'].has('is-positive')

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

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

expect(fs.readdirSync('node_modules/.pnpm').length).toBe(7)

const rootModules = assertProject(process.cwd())
{
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',
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',
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]).toContain(`node_modules${path.sep}.pnpm`)
expect(modulesState?.injectedDeps?.['project-1'][1]).toContain(`node_modules${path.sep}.pnpm`)
}

await rimraf('node_modules')
await rimraf('project-1/node_modules')
await rimraf('project-2/node_modules')
await rimraf('project-3/node_modules')

await mutateModules(importers, await testDefaults({
autoInstallPeers: true,
allProjects,
frozenLockfile: true,
workspacePackages,
}))

await projects['project-1'].has('is-negative')
await projects['project-1'].has('@pnpm.e2e/dep-of-pkg-with-1-dep')
await projects['project-1'].hasNot('is-positive')

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

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

expect(fs.readdirSync('node_modules/.pnpm').length).toBe(8)

// The injected project is updated when one of its dependencies needs to be updated
allProjects[0].manifest.dependencies!['is-negative'] = '2.0.0'
await mutateModules(importers, await testDefaults({ autoInstallPeers: true, allProjects, workspacePackages }))
{
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',
peerDependencies: {
'is-positive': '>=1.0.0',
},
dependencies: {
'is-negative': '2.0.0',
'is-positive': '1.0.0',
},
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`)
}
})
1 change: 1 addition & 0 deletions pkg-manager/resolve-dependencies/src/index.ts
Expand Up @@ -183,6 +183,7 @@ export async function resolveDependencies (
projects: projectsToLink,
virtualStoreDir: opts.virtualStoreDir,
resolvePeersFromWorkspaceRoot: Boolean(opts.resolvePeersFromWorkspaceRoot),
resolvedImporters,
})

const linkedDependenciesByProjectId: Record<string, LinkedDependency[]> = {}
Expand Down
20 changes: 11 additions & 9 deletions pkg-manager/resolve-dependencies/src/resolveDependencyTree.ts
Expand Up @@ -31,6 +31,16 @@ import {
export * from './nodeIdUtils'
export type { LinkedDependency, ResolvedPackage, DependenciesTree, DependenciesTreeNode } from './resolveDependencies'

export interface ResolvedImporters {
[id: string]: {
directDependencies: ResolvedDirectDependency[]
directNodeIdsByAlias: {
[alias: string]: string
}
linkedDependencies: LinkedDependency[]
}
}

export interface ResolvedDirectDependency {
alias: string
optional: boolean
Expand Down Expand Up @@ -180,15 +190,7 @@ export async function resolveDependencyTree<T> (
})
})

const resolvedImporters = {} as {
[id: string]: {
directDependencies: ResolvedDirectDependency[]
directNodeIdsByAlias: {
[alias: string]: string
}
linkedDependencies: LinkedDependency[]
}
}
const resolvedImporters: ResolvedImporters = {}

for (const { id, wantedDependencies } of importers) {
const directDeps = dedupeSameAliasDirectDeps(directDepsByImporterId[id], wantedDependencies)
Expand Down