feat!: --stage-record-file now requires files list; add config file support
#21
Conversation
![doist-bot[bot]](https://avatars.githubusercontent.com/in/2345791?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR successfully introduces the required file list argument for --stage-record-file and adds configuration file support with monorepo-friendly path normalization. The logic for path normalization and config loading appears generally sound. There are a few robustness improvements to consider regarding configuration type safety, shared state in default values, and the handling of deleted files in the documented manual git hook.
There was a problem hiding this comment.
This PR correctly implements the required changes to the --stage-record-file flag and introduces configuration file support with appropriate precedence logic. The integration tests cover the new functionality well. I have identified one issue regarding error handling during configuration loading.
There was a problem hiding this comment.
The PR correctly implements the required changes for explicit file staging and configuration support. The changes are verified by tests. However, the configuration loading logic in src/config.mts handles errors too leniently by falling back to defaults, which can mask configuration issues in CI environments. Also, the validation logic allows arrays as config objects.
Add support for `.react-compiler-tracker.config.json` to configure: - `recordsFile`: Path to the records file - `sourceGlob`: Glob pattern for source files Change `--stage-record-file` to accept files as arguments instead of reading from git, making it compatible with lint-staged in monorepos. Remove redundant `getStagedFromGit()` and `filterSupportedFiles()` functions since the glob pattern already filters by extension and callers now provide the file list directly. Closes #16 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Normalize file paths passed to --stage-record-file and --check-files by stripping the git prefix when running from a package subdirectory. This allows lint-staged to pass repo-root-relative paths that get correctly converted to cwd-relative paths. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Return { ...DEFAULT_CONFIG } instead of DEFAULT_CONFIG to avoid
shared mutable state across multiple loadConfig() calls
- Add isValidConfig type guard to validate parsed JSON config,
warning and falling back to defaults if validation fails
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add --diff-filter=ACMR to git diff command to only include Added, Copied, Modified, and Renamed files, excluding deleted files that would cause the tracker to fail. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Move getGitPrefix() and normalizeFilePaths() from git-utils.mts - Add absolute path handling to normalizeFilePaths() - Delete git-utils.mts and git-utils.test.mts Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Explicit file lists from --stage-record-file and --check-files are now filtered against the sourceGlob config, ensuring only matching files are processed. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Files that don't exist now throw an error instead of being silently ignored. This provides clear feedback when lint-staged passes a path to a file that was deleted. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Previously, JSON parsing errors in the config file were silently swallowed, falling back to defaults without any indication to the user. Now a warning is logged so users are aware of the issue. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…defaults Invalid config files now throw errors immediately rather than logging a warning and falling back to defaults. This makes configuration issues more visible. Also improved test cases with more realistic invalid config examples: - Trailing comma (common JSON editing mistake) - Object wrapper instead of string value Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
JSON.parse can return an array, which has typeof 'object' and would bypass the existing check. This could result in an empty config being returned since property access on arrays returns undefined. Added Array.isArray check and corresponding test case. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
frankieyan
force-pushed
the
frankie/staged-changes
branch
from
43602b4 to
34a1041
Compare
| function isValidConfig(config: unknown): config is Partial<Config> { | ||
| if (typeof config !== 'object' || config === null || Array.isArray(config)) { | ||
| return false | ||
| } | ||
| const obj = config as Record<string, unknown> | ||
| if (obj.recordsFile !== undefined && typeof obj.recordsFile !== 'string') { | ||
| return false | ||
| } | ||
| if (obj.sourceGlob !== undefined && typeof obj.sourceGlob !== 'string') { | ||
| return false | ||
| } | ||
| return true | ||
| } |
There was a problem hiding this comment.
💭 Perhaps we should just use zod here?
There was a problem hiding this comment.
Maybe if we have more config options to add in the future?
Previously,
--stage-record-fileautomatically looked for staged files to test against. This was brittle, and would duplicate what users were likely already doing with lint-staged or shell scripts. This PR changes this mode so that it requires a list of files to test, like--check-files, while still checking them against the source glob pattern.Additionally, this also handles absolute file paths, as well as those not relative to the current working directory. For example, when this is installed under a specific package in a monorepo, lint-staged or git will provide paths that start from the repo's root. This will attempt to strip out the base path by using
git rev-parse --show-prefixto look up the cwd relative to the repo root.Lastly, this adds support for a
.react-compiler-tracker.config.json, which allows the records file's name, and the source glob pattern to be customized.Closes #16
Test plan
I used yalc to test this out with a monorepo:
npx yalc publishin this repo@doist/react-compiler-trackerin a package in a monorepoyalc add @doist/react-compiler-trackerin your target repo. You do have to run this from the root, because AFAIK, dependencies are normally hoisted, and yalc does not seem to mimic the same behaviourchmod +xyournode_modules/.bin/react-compiler-trackerref.currentduring rendernpm exec -w={yoour package} @doist/react-compiler-tracker -- --check-files apps/frontend/src/components/badge.tsx