From b92ce94b96c12913309c854c87c203fb2cc5369d Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Fri, 14 Apr 2023 09:49:32 -0700 Subject: [PATCH] fix: better support for pnpm aliases (#4555) ### 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](https://pnpm.io/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 --- cli/internal/lockfile/lockfile_test.go | 25 +++++++++++++++++++++ cli/internal/lockfile/pnpm_lockfile.go | 4 +++- cli/internal/lockfile/pnpm_lockfile_test.go | 5 ++--- 3 files changed, 30 insertions(+), 4 deletions(-) create mode 100644 cli/internal/lockfile/lockfile_test.go diff --git a/cli/internal/lockfile/lockfile_test.go b/cli/internal/lockfile/lockfile_test.go new file mode 100644 index 0000000000000..7c666cc4f3000 --- /dev/null +++ b/cli/internal/lockfile/lockfile_test.go @@ -0,0 +1,25 @@ +package lockfile + +import ( + "sort" + "testing" + + "gotest.tools/v3/assert" +) + +func Test_ByKeySortIsStable(t *testing.T) { + packagesA := []Package{ + {"/foo/1.2.3", "1.2.3", true}, + {"/baz/1.0.9", "/baz/1.0.9", true}, + {"/bar/1.2.3", "1.2.3", true}, + {"/foo/1.2.3", "/foo/1.2.3", true}, + {"/baz/1.0.9", "1.0.9", true}, + } + packagesB := make([]Package, len(packagesA)) + copy(packagesB, packagesA) + + sort.Sort(ByKey(packagesA)) + sort.Sort(ByKey(packagesB)) + + assert.DeepEqual(t, packagesA, packagesB) +} diff --git a/cli/internal/lockfile/pnpm_lockfile.go b/cli/internal/lockfile/pnpm_lockfile.go index be259201168be..a51b36e768a9a 100644 --- a/cli/internal/lockfile/pnpm_lockfile.go +++ b/cli/internal/lockfile/pnpm_lockfile.go @@ -230,7 +230,9 @@ func (p *PnpmLockfile) ResolvePackage(workspacePath turbopath.AnchoredUnixPath, if entry.Version != "" { version = entry.Version } else { - version = resolvedVersion + // If there isn't a version field in the entry then the version is + // encoded in the key and we can omit the name from the version. + version = p.extractVersion(resolvedVersion) } return Package{Key: resolvedVersion, Version: version, Found: true}, nil } diff --git a/cli/internal/lockfile/pnpm_lockfile_test.go b/cli/internal/lockfile/pnpm_lockfile_test.go index b93541d153de8..b4c8475711b3a 100644 --- a/cli/internal/lockfile/pnpm_lockfile_test.go +++ b/cli/internal/lockfile/pnpm_lockfile_test.go @@ -307,7 +307,7 @@ func Test_PnpmOverride(t *testing.T) { assert.NilError(t, err, "failure to find package") assert.Assert(t, pkg.Found) assert.DeepEqual(t, pkg.Key, "/hardhat-deploy-ethers/0.3.0-beta.13_yab2ug5tvye2kp6e24l5x3z7uy") - assert.DeepEqual(t, pkg.Version, "/hardhat-deploy-ethers/0.3.0-beta.13_yab2ug5tvye2kp6e24l5x3z7uy") + assert.DeepEqual(t, pkg.Version, "0.3.0-beta.13_yab2ug5tvye2kp6e24l5x3z7uy") } func Test_DepPathParsing(t *testing.T) { @@ -377,7 +377,7 @@ func Test_DepPathParsing(t *testing.T) { } } -func Test_MixedVersioning(t *testing.T) { +func Test_PnpmAliasesOverlap(t *testing.T) { contents, err := getFixture(t, "pnpm-absolute.yaml") assert.NilError(t, err) @@ -398,7 +398,6 @@ func Test_MixedVersioning(t *testing.T) { assert.DeepEqual(t, deps, []Package{ {"/@scope/child/1.0.0", "1.0.0", true}, {"/@scope/parent/1.0.0", "1.0.0", true}, - {"/Special/1.2.3", "/Special/1.2.3", true}, {"/Special/1.2.3", "1.2.3", true}, {"/another/1.0.0", "1.0.0", true}, {"/foo/1.0.0", "1.0.0", true},