Add a way to specify non-js dependencies for a package#36
Open
brucespang wants to merge 2 commits into
Open
Conversation
…quoting Two related bugs surfaced when packages declare additionalPaths that live outside the npm/yarn/pnpm workspace root (e.g. a workspace nested inside a larger git repo with sibling source directories): 1. gitAllFilesChangedSinceSha resolved the output of `git diff --name-only` against `cwd`, but git always emits paths relative to the repo top-level regardless of cwd. When the workspace root is not the repo root, this produced paths like `<workspace>/<external>/...` that don't exist on disk, so the downstream isUnderPath filter rejected them and the additionalPaths feature appeared to silently do nothing. 2. gitCommitsSince wrapped each pathspec in `"..."`, but the local exec helper splits commands on whitespace via `command.split(/\s+/)` rather than parsing shell quoting. The literal quote characters end up in argv and git matches no files. With `relPath` of a single, unquoted path (the pre-additionalPaths shape) this never tripped, but the new multi-path form was always quoted. Fix (1) by querying `git rev-parse --show-toplevel` once and resolving against that. Fix (2) by joining pathspecs unquoted; users who need whitespace in additionalPaths can fix the exec layer separately. Made-with: Cursor
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
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.
We have a package that uses WASM and depends on some source files that live outside the package directory. Our repo looks something like this:
We'd like a commit that only affects files in
c/to cause a version bump to thewasmpackage. As far as I can tell, previously there was no way to do this since lets-version only looked at git commits within the package's own directory (wasm/).This PR adds a
packagesmap toletsVersion.config.mjswhere you can specifyadditionalPathsper package. When deciding if a version bump is needed, changes to those paths are treated as changes to the package.Another approach I considered was updating package.json to have a list of additional paths. I went with the
letsVersion.config.mjsapproach for vague consistency with turborepo'sinputs.Used Claude Opus 4.6 to do this PR, with review/planning by me. Happy to make any changes/cleanup you'd suggest (and would be thrilled to close it if there's a cleaner way to solve this problem!)
Rest of the PR description is from Claude:
Example config
For the repo structure above, the config at the repo root would be:
Validation
Each path is validated at startup. If a resolved path doesn't exist, you get an error:
Additional fixes
Multiple packages can share the same additional path
The pre-existing code used
Array.find()when matching modified files to packages, since paths were 1:1 with packages. This PR switches toArray.filter(), in case two packages both depend on the same external path.Path matching boundary safety
The pre-existing code used bare
String.startsWith()andString.includes()for matching file paths against package paths. SincepackagePathnever includes a trailing separator (it comes frompath.dirname()), this could produce false positives — e.g.,/repo/src-other/file.tswould match a package rooted at/repo/src. This seems unlikely in practice since package directory names tend to be unique, but it felt worth fixing.This PR introduces an
isUnderPath(filePath, basePath)utility that normalizes the base path with a trailing separator before comparison, and uses it at all path-matching call sites.Quoted paths in git commands
When this PR passes multiple paths to
git log -- <paths>, each path is individually quoted to handle paths containing spaces. (The pre-existing single-path code didn't need this since git handles a single unquoted path fine.)