diff --git a/server/utils/dependency-resolver.ts b/server/utils/dependency-resolver.ts index 15c8331a36..2744642520 100644 --- a/server/utils/dependency-resolver.ts +++ b/server/utils/dependency-resolver.ts @@ -117,6 +117,25 @@ export interface ResolvedPackage { deprecated?: string } +type DependencyRequest = { + name: string + range: string + optional: boolean + path: string[] +} + +function requestKey(name: string, range: string): string { + // Match the alias-range extraction used by resolveVersion. + if (range.startsWith('npm:')) { + const atIndex = range.lastIndexOf('@') + if (atIndex > 4) { + range = range.slice(atIndex + 1) + } + } + + return JSON.stringify([name, range]) +} + /** * Resolve the entire dependency tree for a package. * Uses level-by-level BFS to ensure correct depth assignment when trackDepth is enabled. @@ -131,24 +150,27 @@ export async function resolveDependencyTree( // Process level by level for correct depth tracking // Each entry includes the path of package names leading to this dependency - let currentLevel = new Map([ - [rootName, { range: rootVersion, optional: false, path: [] }], + let currentLevel = new Map([ + [ + requestKey(rootName, rootVersion), + { name: rootName, range: rootVersion, optional: false, path: [] }, + ], ]) let level = 0 while (currentLevel.size > 0) { - const nextLevel = new Map() + const nextLevel = new Map() // Mark all packages in current level as seen before processing - for (const name of currentLevel.keys()) { - seen.add(name) + for (const key of currentLevel.keys()) { + seen.add(key) } // Process current level with concurrency limit - const entries = [...currentLevel.entries()] + const entries = [...currentLevel.values()] await mapWithConcurrency( entries, - async ([name, { range, optional, path }]) => { + async ({ name, range, optional, path }) => { const packument = await fetchPackument(name) if (!packument) return @@ -183,9 +205,12 @@ export async function resolveDependencyTree( const key = `${name}@${version}` // Build path for this package (path to parent + this package with version) - const currentPath = [...path, `${name}@${version}`] + const currentPath = [...path, key] - if (!resolved.has(key)) { + const existing = resolved.get(key) + if (existing) { + if (!optional) existing.optional = false + } else { const pkg: ResolvedPackage = { name, version, size, tarballUrl, optional } if (options.trackDepth) { pkg.depth = level === 0 ? 'root' : level === 1 ? 'direct' : 'transitive' @@ -202,8 +227,19 @@ export async function resolveDependencyTree( if (versionData.dependencies) { for (const [depKey, depRange] of Object.entries(versionData.dependencies)) { const depName = parseDependencyVersion(depRange).name ?? depKey - if (!seen.has(depName) && !nextLevel.has(depName)) { - nextLevel.set(depName, { range: depRange, optional: false, path: currentPath }) + const reqKey = requestKey(depName, depRange) + const queued = nextLevel.get(reqKey) + + if (queued) { + // A required request upgrades a queued optional request. + queued.optional = false + } else if (!seen.has(reqKey)) { + nextLevel.set(reqKey, { + name: depName, + range: depRange, + optional: false, + path: currentPath, + }) } } } @@ -212,8 +248,15 @@ export async function resolveDependencyTree( if (versionData.optionalDependencies) { for (const [depKey, depRange] of Object.entries(versionData.optionalDependencies)) { const depName = parseDependencyVersion(depRange).name ?? depKey - if (!seen.has(depName) && !nextLevel.has(depName)) { - nextLevel.set(depName, { range: depRange, optional: true, path: currentPath }) + const reqKey = requestKey(depName, depRange) + + if (!seen.has(reqKey) && !nextLevel.has(reqKey)) { + nextLevel.set(reqKey, { + name: depName, + range: depRange, + optional: true, + path: currentPath, + }) } } } diff --git a/test/unit/server/utils/dependency-resolver.spec.ts b/test/unit/server/utils/dependency-resolver.spec.ts index e59b5c17a4..ac9b4e3fd1 100644 --- a/test/unit/server/utils/dependency-resolver.spec.ts +++ b/test/unit/server/utils/dependency-resolver.spec.ts @@ -538,5 +538,114 @@ describe('dependency-resolver', () => { expect(result.size).toBe(4) expect(result.has('shared@1.0.0')).toBe(true) }) + + it('keeps distinct versions requested through aliases of the same package', async () => { + mockAbbreviatedPackument.mockImplementation(async (name: string) => { + if (name === 'root') + return makePackument('root', [ + { + version: '1.0.0', + deps: { 'shared-v1': 'npm:shared@^1.0.0', 'shared-v2': 'npm:shared@^2.0.0' }, + }, + ]) + if (name === 'shared') + return makePackument('shared', [ + { version: '1.0.0', deps: { 'only-v1': '^1.0.0' } }, + { version: '2.0.0', deps: { 'only-v2': '^1.0.0' } }, + ]) + if (name === 'only-v1') return makePackument('only-v1', [{ version: '1.0.0' }]) + if (name === 'only-v2') return makePackument('only-v2', [{ version: '1.0.0' }]) + return null + }) + + const result = await resolveDependencyTree('root', '1.0.0') + + expect([...result.keys()].sort()).toEqual([ + 'only-v1@1.0.0', + 'only-v2@1.0.0', + 'root@1.0.0', + 'shared@1.0.0', + 'shared@2.0.0', + ]) + }) + + it('resolves a different version of a package that was already visited', async () => { + // root → shared@^1 and root → a → shared@^2 + mockAbbreviatedPackument.mockImplementation(async (name: string) => { + if (name === 'root') + return makePackument('root', [ + { version: '1.0.0', deps: { shared: '^1.0.0', a: '^1.0.0' } }, + ]) + if (name === 'a') + return makePackument('a', [{ version: '1.0.0', deps: { shared: '^2.0.0' } }]) + if (name === 'shared') + return makePackument('shared', [{ version: '1.0.0' }, { version: '2.0.0' }]) + return null + }) + + const result = await resolveDependencyTree('root', '1.0.0', { trackDepth: true }) + + expect(result.get('shared@1.0.0')).toMatchObject({ depth: 'direct' }) + expect(result.get('shared@2.0.0')).toMatchObject({ + depth: 'transitive', + path: ['root@1.0.0', 'a@1.0.0', 'shared@2.0.0'], + }) + }) + + it('keeps distinct versions requested as optional dependencies', async () => { + mockAbbreviatedPackument.mockImplementation(async (name: string) => { + if (name === 'root') + return makePackument('root', [ + { version: '1.0.0', deps: { a: '^1.0.0' }, optionalDeps: { opt: '^1.0.0' } }, + ]) + if (name === 'a') + return makePackument('a', [{ version: '1.0.0', optionalDeps: { opt: '^2.0.0' } }]) + if (name === 'opt') + return makePackument('opt', [{ version: '1.0.0' }, { version: '2.0.0' }]) + return null + }) + + const result = await resolveDependencyTree('root', '1.0.0') + + expect(result.get('opt@1.0.0')).toMatchObject({ optional: true }) + expect(result.get('opt@2.0.0')).toMatchObject({ optional: true }) + }) + + it('resolves different ranges that match the same version once', async () => { + mockAbbreviatedPackument.mockImplementation(async (name: string) => { + if (name === 'root') + return makePackument('root', [{ version: '1.0.0', deps: { a: '^1.0.0', b: '^1.0.0' } }]) + if (name === 'a') + return makePackument('a', [{ version: '1.0.0', deps: { shared: '^1.0.0' } }]) + if (name === 'b') + return makePackument('b', [{ version: '1.0.0', deps: { shared: '~1.2.0' } }]) + if (name === 'shared') return makePackument('shared', [{ version: '1.2.3' }]) + return null + }) + + const result = await resolveDependencyTree('root', '1.0.0') + + expect([...result.keys()].sort()).toEqual([ + 'a@1.0.0', + 'b@1.0.0', + 'root@1.0.0', + 'shared@1.2.3', + ]) + }) + + it('terminates cycles between different versions of the same package', async () => { + mockAbbreviatedPackument.mockImplementation(async (name: string) => { + if (name === 'a') + return makePackument('a', [ + { version: '1.0.0', deps: { a: '^2.0.0' } }, + { version: '2.0.0', deps: { a: '^1.0.0' } }, + ]) + return null + }) + + const result = await resolveDependencyTree('a', '1.0.0') + + expect([...result.keys()].sort()).toEqual(['a@1.0.0', 'a@2.0.0']) + }) }) })