Skip to content
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

fix: pnpm alias workspace deps #5569

Merged
merged 2 commits into from
Jul 20, 2023
Merged

fix: pnpm alias workspace deps #5569

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4070228
prefactor: add dependency splitter helper
Jul 19, 2023
21fec88
add support for workspace import aliases
Jul 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 31 additions & 3 deletions cli/internal/context/context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,28 @@ func (c *Context) resolveWorkspaceRootDeps(rootPackageJSON *fs.PackageJSON, warn
return nil
}

type dependencySplitter struct {
workspaces map[string]*fs.PackageJSON
pkgDir string
rootPath string
}

func (d *dependencySplitter) isInternal(name, version string) (string, bool) {
resolvedName := name
if withoutProtocol := strings.TrimPrefix(version, "workspace:"); withoutProtocol != version {
parts := strings.Split(withoutProtocol, "@")
lastIndex := len(parts) - 1
if len(parts) > 1 && (parts[lastIndex] == "*" || parts[lastIndex] == "^" || parts[lastIndex] == "~") {
resolvedName = strings.Join(parts[:lastIndex], "@")
}
}
item, ok := d.workspaces[resolvedName]
if ok && isWorkspaceReference(item.Version, version, d.pkgDir, d.rootPath) {
return resolvedName, true
}
return "", false
}

// populateWorkspaceGraphForPackageJSON fills in the edges for the dependencies of the given package
// that are within the monorepo, as well as collecting and hashing the dependencies of the package
// that are not within the monorepo. The vertexName is used to override the package name in the graph.
Expand All @@ -271,11 +293,17 @@ func (c *Context) populateWorkspaceGraphForPackageJSON(pkg *fs.PackageJSON, root
depMap[dep] = version
}

splitter := dependencySplitter{
workspaces: c.WorkspaceInfos.PackageJSONs,
pkgDir: pkg.Dir.ToStringDuringMigration(),
rootPath: rootpath,
}

// split out internal vs. external deps
for depName, depVersion := range depMap {
if item, ok := c.WorkspaceInfos.PackageJSONs[depName]; ok && isWorkspaceReference(item.Version, depVersion, pkg.Dir.ToStringDuringMigration(), rootpath) {
internalDepsSet.Add(depName)
c.WorkspaceGraph.Connect(dag.BasicEdge(vertexName, depName))
if name, internal := splitter.isInternal(depName, depVersion); internal {
internalDepsSet.Add(name)
c.WorkspaceGraph.Connect(dag.BasicEdge(vertexName, name))
} else {
externalUnresolvedDepsSet.Add(depName)
}
Expand Down
59 changes: 55 additions & 4 deletions cli/internal/context/context_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
"gotest.tools/v3/assert"
)

func Test_isWorkspaceReference(t *testing.T) {
func Test_isInternal(t *testing.T) {
rootpath, err := filepath.Abs(filepath.FromSlash("/some/repo"))
if err != nil {
t.Fatalf("failed to create absolute root path %v", err)
Expand All @@ -30,19 +30,23 @@ func Test_isWorkspaceReference(t *testing.T) {
name string
packageVersion string
dependencyVersion string
depName string
want bool
wantDepName string
}{
{
name: "handles exact match",
packageVersion: "1.2.3",
dependencyVersion: "1.2.3",
want: true,
wantDepName: "@scope/foo",
},
{
name: "handles semver range satisfied",
packageVersion: "1.2.3",
dependencyVersion: "^1.0.0",
want: true,
wantDepName: "@scope/foo",
},
{
name: "handles semver range not-satisfied",
Expand All @@ -55,18 +59,28 @@ func Test_isWorkspaceReference(t *testing.T) {
packageVersion: "1.2.3",
dependencyVersion: "workspace:1.2.3",
want: true,
wantDepName: "@scope/foo",
},
{
name: "handles workspace protocol with relative path",
packageVersion: "1.2.3",
dependencyVersion: "workspace:../other-package/",
want: true,
wantDepName: "@scope/foo",
},
{
name: "handles workspace protocol with relative path",
packageVersion: "1.2.3",
dependencyVersion: "workspace:../@scope/foo",
want: true,
wantDepName: "@scope/foo",
},
{
name: "handles npm protocol with satisfied semver range",
packageVersion: "1.2.3",
dependencyVersion: "npm:^1.2.3",
want: true, // default in yarn is to use the workspace version unless `enableTransparentWorkspaces: true`. This isn't currently being checked.
wantDepName: "@scope/foo",
},
{
name: "handles npm protocol with non-satisfied semver range",
Expand All @@ -85,18 +99,21 @@ func Test_isWorkspaceReference(t *testing.T) {
packageVersion: "sometag",
dependencyVersion: "1.2.3",
want: true, // for backwards compatability with the code before versions were verified
wantDepName: "@scope/foo",
},
{
name: "handles non-semver package version",
packageVersion: "1.2.3",
dependencyVersion: "sometag",
want: true, // for backwards compatability with the code before versions were verified
wantDepName: "@scope/foo",
},
{
name: "handles file:... inside repo",
packageVersion: "1.2.3",
dependencyVersion: "file:../libB",
want: true, // this is a sibling package
wantDepName: "@scope/foo",
},
{
name: "handles file:... outside repo",
Expand All @@ -109,6 +126,7 @@ func Test_isWorkspaceReference(t *testing.T) {
packageVersion: "1.2.3",
dependencyVersion: "link:../libB",
want: true, // this is a sibling package
wantDepName: "@scope/foo",
},
{
name: "handles link:... outside repo",
Expand All @@ -121,15 +139,48 @@ func Test_isWorkspaceReference(t *testing.T) {
packageVersion: "0.0.0-development",
dependencyVersion: "*",
want: true, // "*" should always match
wantDepName: "@scope/foo",
},
{
name: "handles pnpm alias star",
packageVersion: "1.2.3",
depName: "foo",
dependencyVersion: "workspace:@scope/foo@*",
want: true,
wantDepName: "@scope/foo",
},
{
name: "handles pnpm alias tilda",
packageVersion: "1.2.3",
depName: "foo",
dependencyVersion: "workspace:@scope/foo@~",
want: true,
wantDepName: "@scope/foo",
},
{
name: "handles pnpm alias caret",
packageVersion: "1.2.3",
depName: "foo",
dependencyVersion: "workspace:@scope/foo@^",
want: true,
wantDepName: "@scope/foo",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := isWorkspaceReference(tt.packageVersion, tt.dependencyVersion, pkgDir, rootpath)
if got != tt.want {
t.Errorf("isWorkspaceReference(%v, %v, %v, %v) got = %v, want %v", tt.packageVersion, tt.dependencyVersion, pkgDir, rootpath, got, tt.want)
splitter := dependencySplitter{
workspaces: map[string]*fs.PackageJSON{"@scope/foo": {Version: tt.packageVersion}},
pkgDir: pkgDir,
rootPath: rootpath,
}
depName := tt.depName
if depName == "" {
depName = "@scope/foo"
}
name, got := splitter.isInternal(depName, tt.dependencyVersion)
assert.Equal(t, got, tt.want, tt.name)
assert.Equal(t, name, tt.wantDepName, tt.name)
})
}
}
Expand Down
133 changes: 94 additions & 39 deletions crates/turborepo-lib/src/package_graph/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,23 +455,14 @@ impl Dependencies {
.expect("package.json path should have parent");
let mut internal = HashSet::new();
let mut external = HashSet::new();
let splitter = DependencySplitter {
repo_root,
workspace_dir,
workspaces,
};
for (name, version) in dependencies.into_iter() {
// TODO implement borrowing for workspaces to allow for zero copy queries
let workspace_name = WorkspaceName::Other(name.clone());
let is_internal = workspaces
.get(&workspace_name)
// This is the current Go behavior, in the future we might not want to paper over a
// missing version
.map(|e| e.package_json.version.as_deref().unwrap_or_default())
.map_or(false, |workspace_version| {
DependencyVersion::new(version).matches_workspace_package(
workspace_version,
workspace_dir,
repo_root,
)
});
if is_internal {
internal.insert(workspace_name);
if let Some(workspace) = splitter.is_internal(name, version) {
internal.insert(workspace);
} else {
external.insert(Package {
name: name.clone(),
Expand All @@ -483,6 +474,43 @@ impl Dependencies {
}
}

struct DependencySplitter<'a, 'b, 'c> {
repo_root: &'a AbsoluteSystemPath,
workspace_dir: &'b AbsoluteSystemPath,
workspaces: &'c HashMap<WorkspaceName, WorkspaceInfo>,
}

impl<'a, 'b, 'c> DependencySplitter<'a, 'b, 'c> {
fn is_internal(&self, name: &str, version: &str) -> Option<WorkspaceName> {
// TODO implement borrowing for workspaces to allow for zero copy queries
let workspace_name = WorkspaceName::Other(
version
.strip_prefix("workspace:")
.and_then(|version| version.rsplit_once('@'))
.filter(|(_, version)| *version == "*" || *version == "^" || *version == "~")
.map_or(name, |(actual_name, _)| actual_name)
.to_string(),
);
let is_internal = self
.workspaces
.get(&workspace_name)
// This is the current Go behavior, in the future we might not want to paper over a
// missing version
.map(|e| e.package_json.version.as_deref().unwrap_or_default())
.map_or(false, |workspace_version| {
DependencyVersion::new(version).matches_workspace_package(
workspace_version,
self.workspace_dir,
self.repo_root,
)
});
match is_internal {
true => Some(workspace_name),
false => None,
}
}
}

struct DependencyVersion<'a> {
protocol: Option<&'a str>,
version: &'a str,
Expand Down Expand Up @@ -586,40 +614,67 @@ mod test {

use super::*;

#[test_case("1.2.3", "1.2.3", true ; "handles exact match")]
#[test_case("1.2.3", "^1.0.0", true ; "handles semver range satisfied")]
#[test_case("2.3.4", "^1.0.0", false ; "handles semver range not satisfied")]
#[test_case("1.2.3", "workspace:1.2.3", true ; "handles workspace protocol with version")]
#[test_case("1.2.3", "workspace:*", true ; "handles workspace protocol with no version")]
#[test_case("1.2.3", "workspace:../other-packages/", true ; "handles workspace protocol with relative path")]
#[test_case("1.2.3", "npm:^1.2.3", true ; "handles npm protocol with satisfied semver range")]
#[test_case("2.3.4", "npm:^1.2.3", false ; "handles npm protocol with not satisfied semver range")]
#[test_case("1.2.3", "1.2.2-alpha-123abcd.0", false ; "handles pre-release versions")]
#[test_case("1.2.3", None, "1.2.3", Some("@scope/foo") ; "handles exact match")]
#[test_case("1.2.3", None, "^1.0.0", Some("@scope/foo") ; "handles semver range satisfied")]
#[test_case("2.3.4", None, "^1.0.0", None ; "handles semver range not satisfied")]
#[test_case("1.2.3", None, "workspace:1.2.3", Some("@scope/foo") ; "handles workspace protocol with version")]
#[test_case("1.2.3", None, "workspace:*", Some("@scope/foo") ; "handles workspace protocol with no version")]
#[test_case("1.2.3", None, "workspace:../other-packages/", Some("@scope/foo") ; "handles workspace protocol with relative path")]
#[test_case("1.2.3", None, "workspace:../@scope/foo", Some("@scope/foo") ; "handles workspace protocol with scoped relative path")]
#[test_case("1.2.3", None, "npm:^1.2.3", Some("@scope/foo") ; "handles npm protocol with satisfied semver range")]
#[test_case("2.3.4", None, "npm:^1.2.3", None ; "handles npm protocol with not satisfied semver range")]
#[test_case("1.2.3", None, "1.2.2-alpha-123abcd.0", None ; "handles pre-release versions")]
// for backwards compatability with the code before versions were verified
#[test_case("sometag", "1.2.3", true ; "handles non-semver package version")]
#[test_case("sometag", None, "1.2.3", Some("@scope/foo") ; "handles non-semver package version")]
// for backwards compatability with the code before versions were verified
#[test_case("1.2.3", "sometag", true ; "handles non-semver dependency version")]
#[test_case("1.2.3", "file:../libB", true ; "handles file:.. inside repo")]
#[test_case("1.2.3", "file:../../../otherproject", false ; "handles file:.. outside repo")]
#[test_case("1.2.3", "link:../libB", true ; "handles link:.. inside repo")]
#[test_case("1.2.3", "link:../../../otherproject", false ; "handles link:.. outside repo")]
#[test_case("0.0.0-development", "*", true ; "handles development versions")]
fn test_matches_workspace_package(package_version: &str, range: &str, expected: bool) {
#[test_case("1.2.3", None, "sometag", Some("@scope/foo") ; "handles non-semver dependency version")]
#[test_case("1.2.3", None, "file:../libB", Some("@scope/foo") ; "handles file:.. inside repo")]
#[test_case("1.2.3", None, "file:../../../otherproject", None ; "handles file:.. outside repo")]
#[test_case("1.2.3", None, "link:../libB", Some("@scope/foo") ; "handles link:.. inside repo")]
#[test_case("1.2.3", None, "link:../../../otherproject", None ; "handles link:.. outside repo")]
#[test_case("0.0.0-development", None, "*", Some("@scope/foo") ; "handles development versions")]
#[test_case("1.2.3", Some("foo"), "workspace:@scope/foo@*", Some("@scope/foo") ; "handles pnpm alias star")]
#[test_case("1.2.3", Some("foo"), "workspace:@scope/foo@~", Some("@scope/foo") ; "handles pnpm alias tilda")]
#[test_case("1.2.3", Some("foo"), "workspace:@scope/foo@^", Some("@scope/foo") ; "handles pnpm alias caret")]
fn test_matches_workspace_package(
package_version: &str,
dependency_name: Option<&str>,
range: &str,
expected: Option<&str>,
) {
let root = AbsoluteSystemPathBuf::new(if cfg!(windows) {
"C:\\some\\repo"
} else {
"/some/repo"
})
.unwrap();
let pkg_dir = root.join_components(&["packages", "libA"]);
let workspaces = {
let mut map = HashMap::new();
map.insert(
WorkspaceName::Other("@scope/foo".to_string()),
WorkspaceInfo {
package_json: PackageJson {
version: Some(package_version.to_string()),
..Default::default()
},
package_json_path: AnchoredSystemPathBuf::from_raw("unused").unwrap(),
unresolved_external_dependencies: None,
transitive_dependencies: None,
},
);
map
};

let splitter = DependencySplitter {
repo_root: &root,
workspace_dir: &pkg_dir,
workspaces: &workspaces,
};

assert_eq!(
DependencyVersion::new(range).matches_workspace_package(
package_version,
&pkg_dir,
&root
),
expected
splitter.is_internal(dependency_name.unwrap_or("@scope/foo"), range),
expected.map(WorkspaceName::from)
);
}

Expand Down