Skip to content

fix(deploy): preserve relative paths to linked internal dependencies #9677

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

fix(deploy): preserve relative paths to linked internal dependencies #9677

wants to merge 5 commits into from

Conversation

KSXGitHub
Copy link
Contributor

@KSXGitHub KSXGitHub commented Jun 20, 2025 •
edited
Loading

Fixes #9575

@KSXGitHub
Copy link
Contributor Author

Notes

Results of the console.traces: console.trace.log

The bug was suspected to be caused by the following lines:

      projectDir: (
        options.currentDepth > 0 &&
        !wantedDependency.bareSpecifier.startsWith('file:')
      )
        ? ctx.lockfileDir
        : options.parentPkg.rootDir,

But adding && wantedDependency !== 'link:./c' to the condition above would cause another error.

I added another modification to resolvePeers to workaround this error:

diff --git a/pkg-manager/resolve-dependencies/src/resolvePeers.ts b/pkg-manager/resolve-dependencies/src/resolvePeers.ts
index 07830ddd1..6bb42549a 100644
--- a/pkg-manager/resolve-dependencies/src/resolvePeers.ts
+++ b/pkg-manager/resolve-dependencies/src/resolvePeers.ts
@@ -387,7 +387,7 @@ async function resolvePeersOfNode<T extends PartialResolvedPackage> (
   }
 ): Promise<PeersResolution & { finishing?: FinishingResolutionPromise, calculateDepPath?: CalculateDepPath }> {
   const node = ctx.dependenciesTree.get(nodeId)!
-  if (node.depth === -1) return { resolvedPeers: new Map<string, NodeId>(), missingPeers: new Map<string, MissingPeerInfo>() }
+  if (!node || node.depth === -1) return { resolvedPeers: new Map<string, NodeId>(), missingPeers: new Map<string, MissingPeerInfo>() }
   const resolvedPackage = node.resolvedPackage as T
   if (
     ctx.purePkgs.has(resolvedPackage.pkgIdWithPatchHash) &&

pd no longer throws any error now. But c still doesn't point to the correct location. What's more, it still has to same target (not exist) as without any modification above.

I injected console.trace into path.resolve at the near top of main.ts with the following code:

const _resolve = path.resolve
path.resolve = (...paths: string[]) => {
  const last = paths.at(-1)!
  if (
    paths.length > 1 &&
    (last === 'c' || last.endsWith('/c')) &&
    !paths.at(-2)?.endsWith('node_modules') &&
    paths.at(-2) !== 'a' &&
    !paths.at(-2)?.endsWith('/a')
  ) {
    console.trace('PATH RESOLVE', paths)
  }
  return _resolve(...paths)
}

This trace appears (also in the file console.trace.log above):

console.trace('PATH RESOLVE', paths) // injection of `path.resolve` after a temporary edit
Trace: PATH RESOLVE [
  '${REPRODUCTION_REPO}',
  '.',
  'c'
]
    at Object.import_path171.default.resolve (${PNPM_REPO}/pnpm/src/main.ts:55:13)
    at getChildrenPaths (${PNPM_REPO}/deps/graph-builder/src/lockfileToDepGraph.ts:274:30)
    at lockfileToDepGraph2 (${PNPM_REPO}/deps/graph-builder/src/lockfileToDepGraph.ts:125:21)
    at async headlessInstall (${PNPM_REPO}/pkg-manager/headless/src/index.ts:343:7)
    at async installInContext (${PNPM_REPO}/pkg-manager/core/src/install/index.ts:1574:42)
    at async _install (${PNPM_REPO}/pkg-manager/core/src/install/index.ts:633:20)
    at async mutateModules (${PNPM_REPO}/pkg-manager/core/src/install/index.ts:335:18)
    at async recursive (${PNPM_REPO}/pkg-manager/plugin-commands-installation/src/recursive.ts:286:9)
    at async recursiveInstallThenUpdateWorkspaceState (${PNPM_REPO}/pkg-manager/plugin-commands-installation/src/installDeps.ts:413:27)
    at async installDeps (${PNPM_REPO}/pkg-manager/plugin-commands-installation/src/installDeps.ts:215:7)

I investigated lockfileToDepGraph.ts:

  for (const node of Object.values(graph)) {
    const pkgSnapshot = lockfile.packages![node.depPath]
    const allDeps = {
      ...pkgSnapshot.dependencies,
      ...(opts.include.optionalDependencies ? pkgSnapshot.optionalDependencies : {}),
    }
    const peerDeps = pkgSnapshot.peerDependencies ? new Set(Object.keys(pkgSnapshot.peerDependencies)) : null
    node.children = _getChildrenPaths(allDeps, peerDeps, '.')
  }

It turns out that pkgSnapshot that contains link:c was transformed and passed into _getChildrenPaths with . as the project id.

Seeing that it would take far more effort than initially estimated to fix this bug, I decided to abandon this. The above notes may be helpful for anyone who wants to continue.

@KSXGitHub KSXGitHub mentioned this pull request Jun 23, 2025
Open
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BoomchainLabs BoomchainLabs BoomchainLabs approved these changes

@zkochan zkochan Awaiting requested review from zkochan zkochan will be requested when the pull request is marked ready for review zkochan is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

pnpm deploy does not preserve relative paths to linked local dependencies
2 participants
@KSXGitHub @BoomchainLabs