fix: better support for pnpm aliases

[chris-olszewski]

Description

This is a follow up to #4541 that fixes the underlying issue. We now will attempt to return the extracted version instead of the full key when we encounter one.

A quick explanation for how this happened is that when pnpm aliases are used the resolved version in the lockfile will be the full lockfile key instead of a version since the name won't match the specifier (e.g. "foo: npm:bar@^1.0.0" will have a version of /bar/1.0.0 instead of just 1.0.0). This is all fine unless an external dependency also depends on bar@1.0.0 then we get a dependency of Package{Key: "/bar/1.0.0", Version: "1.0.0"} when going through the external dependency where we would get Package{Key: "/bar/1.0.0", Version: "/bar/1.0.0"} going through the workspace with the alias.

Testing Instructions

Updated the unit test added in the initial PR to reflect that we're no longer getting a package with a version that's actually a lockfile key. This change also changes the test for pnpm overrides, but this change is more correct as we're now only returning the version. I also tested that the changes don't trigger the repro that was provided in #3638

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
turbo-site	✅ Ready (Inspect)	Visit Preview	Apr 12, 2023 9:27pm

10 Ignored Deployments

Name	Status	Preview	Updated (UTC)
examples-basic-web	⬜️ Ignored (Inspect)		Apr 12, 2023 9:27pm
examples-cra-web	⬜️ Ignored (Inspect)		Apr 12, 2023 9:27pm
examples-designsystem-docs	⬜️ Ignored (Inspect)		Apr 12, 2023 9:27pm
examples-gatsby-web	⬜️ Ignored (Inspect)		Apr 12, 2023 9:27pm
examples-kitchensink-blog	⬜️ Ignored (Inspect)		Apr 12, 2023 9:27pm
examples-native-web	⬜️ Ignored (Inspect)		Apr 12, 2023 9:27pm
examples-nonmonorepo	⬜️ Ignored (Inspect)		Apr 12, 2023 9:27pm
examples-svelte-web	⬜️ Ignored (Inspect)		Apr 12, 2023 9:27pm
examples-tailwind-web	⬜️ Ignored (Inspect)		Apr 12, 2023 9:27pm
examples-vite-web	⬜️ Ignored (Inspect)		Apr 12, 2023 9:27pm

[github-actions]

🟢 CI successful 🟢

Thanks

[mehulkar]

Shuold we also revert the ByKey change if it's not necessary anymore? Or at least test that the tests pass without it?

[chris-olszewski]

Shuold we also revert the ByKey change if it's not necessary anymore?

No, between the pnpm lockfile versions we support there could still be edge cases where we end up with differing versions and without the ByKey changes that would introduce hash instability. Having two differing Package structs refer to the same package isn't the worst thing as the package changing in the lockfile will invalidate both.

Or at least test that the tests pass without it?

Which tests?

[mehulkar]

Or at least test that the tests pass without it?

Which tests?

I meant all of them, but especially the one you added in #4542. but not sure if that unit test would actually exercise this. in either case, ✅