Skip to content

fix(aborist): handle missing link targets in load-virtual.js #8200

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

Open
wants to merge 1 commit into
base: latest
Choose a base branch
from

Conversation

domdomegg
Copy link

@domdomegg domdomegg commented Apr 1, 2025

This PR fixes an issue where npm operations would fail with Cannot read properties of undefined (reading 'extraneous') after a workspace package was renamed or removed without updating its dependents.

Changes

  • Skip loading a target in VirtualLoader if it doesn't exist
  • Add tests verifying the loader handles missing link targets correctly

References

Fixes #8199

@wraithgar
Copy link
Member

I don't think ignoring this error is the right solution. npm will think install succeeded but the tree will not be valid.

@domdomegg
Copy link
Author

@wraithgar currently npm install will succeed with an invalid tree: this just reduces weird behaviour.

Unfortunately a nicer solution of blocking non-existent links being created is blocked on #8201 (see related PR #8202).

An alternative we could have is to throw an error here, with details of the problematic package. This would mean npm install would succeed the first time, but successive npm installs would result in failure messages that would direct the user to fix the offending package.

Given the above, would you be willing to:

  • Advise on how we might block non-existent links being created - i.e. similar to PR [draft] fix(aborist): reify should not create links to non-existent paths #8202. I'm not super familiar with the npm codebase but from my understanding this would require quite a bit more complexity; or
  • Let me know the alternative of throwing an error here would be accepted; or
  • Approve this PR given we're constrained by the above, and this is an improvement on the status quo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants