这是indexloc提供的服务,不要输入任何密码
Skip to content

feat: add Workspace.find_package_by_path #10493

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

Merged
merged 6 commits into from
May 20, 2025

Conversation

scubbo
Copy link
Contributor

@scubbo scubbo commented May 20, 2025

Description

As raised here.

Depending on the call-pattern that we want to support, it might be worthwhile to introduce a cache for ancestor-paths so that subsequent calls only need to traverse-up to a first-common-ancestor.

Testing Instructions

pnpm build && pnpm test within packages/turbo-repository passes.

Note

I'm very much a Rust newbie, so please don't hold back on stylistic nits or comments - I have no strong opinions on "correct" Rust!

As
[here](https://linear.app/vercel/issue/TURBO-4724/expose-package-path-in-turborepository).

Depending on the call-pattern that we want to support, it might be
worthwhile to introduce a cache for ancestor-paths so that subsequent
calls only need to traverse-up to a first-common-ancestor.
@turbo-orchestrator turbo-orchestrator bot added needs: triage New issues get this label. Remove it after triage pkg: turbo-repository labels May 20, 2025
Copy link

vercel bot commented May 20, 2025

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

Name Status Preview Comments Updated (UTC)
examples-basic-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 20, 2025 6:46pm
examples-designsystem-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 20, 2025 6:46pm
examples-gatsby-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 20, 2025 6:46pm
examples-kitchensink-blog ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 20, 2025 6:46pm
examples-native-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 20, 2025 6:46pm
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 20, 2025 6:46pm
examples-svelte-web ❌ Failed (Inspect) May 20, 2025 6:46pm
examples-tailwind-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 20, 2025 6:46pm
examples-vite-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 20, 2025 6:46pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
turbo-site ⬜️ Skipped (Inspect) May 20, 2025 6:46pm

@scubbo
Copy link
Contributor Author

scubbo commented May 20, 2025

AFAICT that deployment failure is unrelated to this code, though I don't know for sure.

@scubbo scubbo marked this pull request as ready for review May 20, 2025 00:56
@scubbo scubbo requested a review from a team as a code owner May 20, 2025 00:56
@scubbo scubbo requested review from tknickman and dimitropoulos May 20, 2025 00:56
Copy link
Member

@chris-olszewski chris-olszewski left a comment

Choose a reason for hiding this comment

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

Only concerns are around beefing up the tests.

// every ancestor.
#[napi]
pub async fn find_package_by_path(&self, path: String) -> Result<Package, Error> {
let package_mapper = DefaultPackageChangeMapper::new(&self.graph);
Copy link
Member

Choose a reason for hiding this comment

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

This is something that I should've given you a code pointer on. This is the structure we use to determine which files affect which packages. We can use it raw here to do the same purpose of mapping files to packages.

Happy to chat a bit more about this!

match package_mapper.detect_package(&anchored_path) {
turborepo_repository::change_mapper::PackageMapping::All(
_all_package_change_reason,
) => Err(Error::from_reason("file belongs to many packages")),
Copy link
Member

Choose a reason for hiding this comment

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

An alternative here would be to possibly return the root package or return that the file change is considered "global".

const workspace = await Workspace.find(MONOREPO_PATH);

for (const [filePath, result] of [
["apps/app/src/util/useful-file.ts", "app-a"],
Copy link
Member

Choose a reason for hiding this comment

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

We should have some tests for root files e.g. a root tsconfig.json, pnpm-lock.yaml, pnpm-workspace.yaml

scubbo and others added 2 commits May 20, 2025 09:56
Co-authored-by: Chris Olszewski <chris.olszewski@vercel.com>
@scubbo scubbo requested a review from chris-olszewski May 20, 2025 17:12
@chris-olszewski chris-olszewski merged commit 649cf04 into main May 20, 2025
43 of 44 checks passed
@chris-olszewski chris-olszewski deleted the scubbo/find-package-by-path branch May 20, 2025 20:13
chris-olszewski added a commit that referenced this pull request May 20, 2025
### Description

Bump version to release #10493

### Testing Instructions

👀
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: triage New issues get this label. Remove it after triage pkg: turbo-repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants