fix: lint:changes command to only lint changed files - #2665
Conversation
|
f69eb89 to
b60b30a
Compare
Visual regression report
Diff images (33)alert.png — baseline no longer producedavatar.png — baseline no longer producedbadge.png — baseline no longer producedbillboard.png — baseline no longer producedbreadcrumb.png — baseline no longer producedbutton-and-derivatives.png — baseline no longer producedbyline.png — baseline no longer producedcalendar.png — baseline no longer producedcheckbox.png — baseline no longer producedcheckboxgroup.png — baseline no longer producedcolorpicker.png — baseline no longer producedcontextview.png — baseline no longer producedcustom-and-lucide-icons.png — baseline no longer produceddateinput-dateinput2.png — baseline no longer produceddatetimeinput.png — baseline no longer produceddiff-demo.png — 6324 pixels differdrilldown.png — baseline no longer producedfiledrop.png — baseline no longer producedform-errors.png — baseline no longer producedheading.png — baseline no longer producedimg.png — baseline no longer producedlink.png — baseline no longer producedmenu.png — baseline no longer producedmetric-pill-tag-timeselect-text.png — baseline no longer producedoptions.png — baseline no longer producedpagination.png — baseline no longer producedprogressbar.png — baseline no longer producedselect-simpleselect.png — baseline no longer producedtable.png — baseline no longer producedtabs.png — baseline no longer producedtooltip.png — baseline no longer producedtreebrowser.png — baseline no longer producedview.png — baseline no longer producedBaselines come from the |
Replace the broken --since flag with proper merge-base diff logic. The command now gets files changed since merge-base with master and passes them directly to oxlint instead of routing through the workspace runner which would lint the entire codebase.
b60b30a to
e5b15d7
Compare
balzss
left a comment
There was a problem hiding this comment.
this command breaks workflows without js/ts changes since oxlint exits with 1. ironically this very PR is a good example
…anged Filter the changed-file list to lintable extensions before handing it to oxlint, so it's only invoked when there's actually something to lint.
| "test:browser-ui": "pnpm run playwright:install && vitest --project browser --browser.headless=false --ui", | ||
| "lint": "pnpm -r --stream lint", | ||
| "lint:changes": "git diff --name-only $(git merge-base HEAD origin/master)..HEAD | xargs -r oxlint --format=unix", | ||
| "lint:changes": "git diff --name-only $(git merge-base HEAD origin/master)..HEAD | { grep -E '\\.(js|jsx|ts|tsx|mjs|cjs)$' || true; } | xargs -r oxlint --format=unix", |
There was a problem hiding this comment.
- git diff: list files changed since branching from master
- grep (+ || true): keep only lintable extensions; don't fail if none match
- xargs -r oxlint: only run oxlint if files remain, so no-match no longer exits 123
balzss
left a comment
There was a problem hiding this comment.
Issue with the current solution: git diff --name-only also lists deleted files, and oxlint exits 1 (No files found to lint) when every path it gets no longer exists — so a PR whose only lintable change is a deletion (e.g. removing an obsolete test) will fail the lint job. The pre-commit hook won't catch it either, since lint-staged skips deleted files.
Suggested fix: add --diff-filter=ACMR to the git diff call so deleted paths never reach oxlint.
please note that the fix was suggested by claude and needs a double check

Summary
--since HEAD^flag withgit merge-baseto properly target changed filesTest Plan
pnpm run lint:changesnow only lints files in the diff since merge-baseFixes INSTUI-5090