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.

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

feat(@turbo/repository): return dependencies in graph #7616

Merged
merged 10 commits into from
Mar 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
61 changes: 61 additions & 0 deletions packages/turbo-repository/__tests__/affected-packages.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import * as path from "node:path";
import { Workspace, Package, PackageManager } from "../js/dist/index.js";

type PackageReduced = Pick<Package, "name" | "relativePath">;

interface AffectedPackagesTestParams {
files: string[];
expected: PackageReduced[];
description: string;
}

describe("affectedPackages", () => {
const tests: AffectedPackagesTestParams[] = [
{
description: "app change",
files: ["apps/app/file.txt"],
expected: [{ name: "app-a", relativePath: "apps/app" }],
},
{
description: "lib change",
files: ["packages/ui/a.txt"],
expected: [{ name: "ui", relativePath: "packages/ui" }],
},
{
description: "global change",
files: ["package.json"],
expected: [
{ name: "app-a", relativePath: "apps/app" },
{ name: "ui", relativePath: "packages/ui" },
],
},
{
description:
"global change should be irrelevant but still triggers all packages",
files: ["README.md"],
expected: [
{ name: "app-a", relativePath: "apps/app" },
{ name: "ui", relativePath: "packages/ui" },
],
},
];

test.each(tests)(
"$description",
async (testParams: AffectedPackagesTestParams) => {
const { files, expected } = testParams;
const dir = path.resolve(__dirname, "./fixtures/monorepo");
const workspace = await Workspace.find(dir);
const reduced: PackageReduced[] = (
await workspace.affectedPackages(files)
).map((pkg) => {
return {
name: pkg.name,
relativePath: pkg.relativePath,
};
});

expect(reduced).toEqual(expected);
}
);
});
27 changes: 27 additions & 0 deletions packages/turbo-repository/__tests__/find-packages.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import * as path from "node:path";
import { Workspace, Package } from "../js/dist/index.js";

describe("findPackages", () => {
it("enumerates packages", async () => {
const workspace = await Workspace.find("./fixtures/monorepo");
const packages: Package[] = await workspace.findPackages();
expect(packages.length).not.toBe(0);
});

it("returns a package graph", async () => {
const dir = path.resolve(__dirname, "./fixtures/monorepo");
const workspace = await Workspace.find(dir);
const packages = await workspace.findPackagesWithGraph();

expect(Object.keys(packages).length).toBe(2);

const pkg1 = packages["apps/app"];
const pkg2 = packages["packages/ui"];

expect(pkg1.dependencies).toEqual(["packages/ui"]);
expect(pkg1.dependents).toEqual([]);

expect(pkg2.dependencies).toEqual([]);
expect(pkg2.dependents).toEqual(["apps/app"]);
});
});
91 changes: 0 additions & 91 deletions packages/turbo-repository/__tests__/find.test.ts

This file was deleted.

16 changes: 16 additions & 0 deletions packages/turbo-repository/__tests__/workspace.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import * as path from "node:path";
import { Workspace, PackageManager } from "../js/dist/index.js";

describe("Workspace", () => {
it("finds a workspace", async () => {
const workspace = await Workspace.find();
const expectedRoot = path.resolve(__dirname, "../../..");
expect(workspace.absolutePath).toBe(expectedRoot);
});

it("finds a package manager", async () => {
const workspace = await Workspace.find();
const packageManager: PackageManager = workspace.packageManager;
expect(packageManager.name).toBe("pnpm");
});
});
16 changes: 13 additions & 3 deletions packages/turbo-repository/js/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ export class Package {
/** The relative path from the workspace root to the package root. */
readonly relativePath: string;
}
export class PackageDetails {
readonly dependencies: Array<RelativePath>;
readonly dependents: Array<RelativePath>;
}
export class PackageManager {
/** The package manager name in lower case. */
readonly name: string;
Expand All @@ -29,10 +33,16 @@ export class Workspace {
/** Finds and returns packages within the workspace. */
findPackages(): Promise<Array<Package>>;
/**
* Finds and returns a map of packages within the workspace and its
* dependents (i.e. the packages that depend on each of those packages).
* Returns a map of packages within the workspace, its dependencies and
* dependents. The response looks like this:
* {
* "package-path": {
* "dependents": ["dependent1_path", "dependent2_path"],
* "dependencies": ["dependency1_path", "dependency2_path"]
* }
* }
*/
findPackagesAndDependents(): Promise<Record<string, Array<string>>>;
findPackagesWithGraph(): Promise<SerializablePackages>;
mehulkar marked this conversation as resolved.
Show resolved Hide resolved
/**
* Given a set of "changed" files, returns a set of packages that are
* "affected" by the changes. The `files` argument is expected to be a list
Expand Down
81 changes: 63 additions & 18 deletions packages/turbo-repository/rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,20 @@ pub struct Package {
pub relative_path: String,
}

#[derive(Clone)]
type RelativePath = String;

#[napi]
#[derive(Debug)]
pub struct PackageDetails {
#[napi(readonly)]
pub dependencies: Vec<RelativePath>,
#[napi(readonly)]
pub dependents: Vec<RelativePath>,
}
type SerializablePackages = HashMap<RelativePath, PackageDetails>;

#[derive(Clone)]
#[napi]
pub struct PackageManager {
/// The package manager name in lower case.
#[napi(readonly)]
Expand Down Expand Up @@ -73,13 +84,36 @@ impl Package {
workspace_path: &AbsoluteSystemPath,
) -> Vec<Package> {
let node = PackageNode::Workspace(PackageName::Other(self.name.clone()));
let ancestors = match graph.immediate_ancestors(&node) {
Some(ancestors) => ancestors,
let pkgs = match graph.immediate_ancestors(&node) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure you want just immediate_ancestors? What about transitive dependents? If you do in fact just want immediates, maybe a comment or update the method name to indicate that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just immediate for now. We are constructing the graph client side in our use case, we'll keep doing that for now and iterate here. I can't remember why I didn't do full tree on ancestors when I first did it, probably Rust-related hurdles.

Some(pkgs) => pkgs,
None => return vec![],
};

pkgs.iter()
.filter_map(|node| {
let info = graph.package_info(node.as_package_name())?;
// If we don't get a package name back, we'll just skip it.
let name = info.package_name()?;
let anchored_package_path = info.package_path();
let package_path = workspace_path.resolve(anchored_package_path);
Some(Package::new(name, workspace_path, &package_path))
})
.collect()
}

fn dependencies(
&self,
graph: &PackageGraph,
workspace_path: &AbsoluteSystemPath,
) -> Vec<Package> {
let node = PackageNode::Workspace(PackageName::Other(self.name.clone()));
let pkgs = match graph.immediate_dependencies(&node) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, this is getting just immediate dependencies, not the full transitive closure. Might be worth a comment at least.

Some(pkgs) => pkgs,
None => return vec![],
};

ancestors
.iter()
pkgs.iter()
.filter(|node| !matches!(node, PackageNode::Root))
.filter_map(|node| {
let info = graph.package_info(node.as_package_name())?;
// If we don't get a package name back, we'll just skip it.
Expand Down Expand Up @@ -107,29 +141,40 @@ impl Workspace {
self.packages_internal().await.map_err(|e| e.into())
}

/// Finds and returns a map of packages within the workspace and its
/// dependents (i.e. the packages that depend on each of those packages).
/// Returns a map of packages within the workspace, its dependencies and
/// dependents. The response looks like this:
/// {
/// "package-path": {
/// "dependents": ["dependent1_path", "dependent2_path"],
/// "dependencies": ["dependency1_path", "dependency2_path"]
/// }
/// }
#[napi]
pub async fn find_packages_and_dependents(
&self,
) -> Result<HashMap<String, Vec<String>>, Error> {
pub async fn find_packages_with_graph(&self) -> Result<SerializablePackages, Error> {
let packages = self.find_packages().await?;

let workspace_path = match AbsoluteSystemPath::new(self.absolute_path.as_str()) {
Ok(path) => path,
Err(e) => return Err(Error::from_reason(e.to_string())),
};

let map: HashMap<String, Vec<String>> = packages
let map: HashMap<RelativePath, PackageDetails> = packages
.into_iter()
.map(|package| {
let deps = package.dependents(&self.graph, workspace_path);
let dep_names = deps
.into_iter()
.map(|p| p.relative_path)
.collect::<Vec<String>>();

(package.relative_path, dep_names)
let details = PackageDetails {
dependencies: package
.dependencies(&self.graph, workspace_path)
.into_iter()
.map(|p| p.relative_path)
.collect(),
dependents: package
.dependents(&self.graph, workspace_path)
.into_iter()
.map(|p| p.relative_path)
.collect(),
};

(package.relative_path, details)
})
.collect();

Expand Down