Skip to content

Commit

Permalink
fix: better support for pnpm aliases (#4555)
Browse files Browse the repository at this point in the history
### 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
  • Loading branch information
chris-olszewski committed Apr 14, 2023
1 parent c604be7 commit b92ce94
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 4 deletions.
25 changes: 25 additions & 0 deletions 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)
}
4 changes: 3 additions & 1 deletion cli/internal/lockfile/pnpm_lockfile.go
Expand Up @@ -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
}
Expand Down
5 changes: 2 additions & 3 deletions cli/internal/lockfile/pnpm_lockfile_test.go
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)

Expand All @@ -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},
Expand Down

0 comments on commit b92ce94

Please sign in to comment.