Implementing package inference #2112
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
9 Ignored Deployments
|
cli/internal/scope/filter/filter.go
Outdated
| selector.namePattern = pi.PackageName | ||
| } | ||
| if selector.parentDir != "" { | ||
| parentDir := pi.DirectoryRoot.UntypedJoin(selector.parentDir).ToStringDuringMigration() |
There was a problem hiding this comment.
Is it possible for parentDir to be an absolute path? From reading the code, it seems possible if, for instance, --cwd is set to an absolute path. In which case, does this do the right thing?
There was a problem hiding this comment.
Good catch. We unfortunately have not plumbed through our paths here yet.
Looking through the code, specifying an absolute path will break currently, or at least not match anything. We join it with the repo root / cwd regardless. But that does mean the above behavior is incorrect, since if we have a parentDir, it is always going to be an absolute path, and the correct behavior for inference is to join what the user specified to the inferred directory.
I'm going to take a stab at using our path types to sort this out and hope it doesn't spiral out too much.
There was a problem hiding this comment.
Ported parentDir on TargetSelector to RelativeSystemPath, and pulled the thread to doing the right thing here.
It wasn't possible for the user input to parentDir to be an absolute path before, but that was not obvious.
c419bff to
422e11d
Compare
aa9e09d to
72cbb82
Compare
🟢 CI successful 🟢Thanks |
Co-authored-by: Chris Olszewski <chris.olszewski@vercel.com>
Benchmark for 2136e79Click to view benchmark
|
Benchmark for 0d4dc97Click to view benchmark
|
Benchmark for af3e613Click to view benchmark
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TURBO_INVOCATION_DIRenv var to global turbo's invocation of local turbo. This allows local turbos that support it to calculate package inference, without passing a bad argument to local turbos that don't yet have this feature.--pkg-inference-rootflag, intended to be passed by the shim after calculating an appropriate value, based on the monorepo root andTURBO_INVOCATION_DIRenv variable.TargetSelectorinstances in the presence of an inferred directory. If there's a single package at or above the directory, use that. Otherwise, consider the directory to be a prefix on a parentDir specifierTargetSelectorinstances by reference so they can be updated in place--cwd