Skip to content

[No QA] Make -changed scripts check the working tree, not just committed changes #99764

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.

Sign up for GitHub

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
roryabraham merged 9 commits into from
Sep 1, 2026
Merged

[No QA] Make -changed scripts check the working tree, not just committed changes #99764

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
670f5e2
fix: check working tree, not just HEAD, in lint-changed and check-cha…
JakubKorytko Aug 28, 2026
b71be98
fix: standardize knip-changed and spell-changed on shared merge-base/…
JakubKorytko Aug 28, 2026
2b4d390
fix: send shellUtils log helpers to stderr, fix stale spell-changed d…
JakubKorytko Aug 28, 2026
a0447f3
Merge branch 'main' into korytko/99113-lint-changed-working-tree
JakubKorytko Aug 31, 2026
85ea9aa
fix: address PR review comments on -changed scripts
JakubKorytko Aug 31, 2026
86eb399
chore: rebuild gh actions bundle
JakubKorytko Aug 31, 2026
412511b
fix: bash 3 compat, preserve quoted paths, fix ls-files cwd-relative bug
JakubKorytko Aug 31, 2026
77e9e47
fix: exclude binaries from spell-changed, route all log helpers to st…
JakubKorytko Sep 1, 2026
8abbc61
fix: address PR review comments on -changed scripts
JakubKorytko Sep 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions .github/actions/javascript/getPullRequestIncrementalChanges/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26147,10 +26147,11 @@ var Git = class _Git {
* @param fromRef - The starting reference (commit, branch, tag, etc.)
* @param toRef - The ending reference (defaults to working directory if not provided)
* @param filePaths - Optional specific file path(s) to diff (relative to git repo root)
* @param untrackedFileExtensions - Optional extensions (e.g. ['.ts', '.tsx']) to restrict which untracked files get read; avoids reading unrelated untracked files (media, snapshots, etc.) in full
* @returns Structured diff result with line numbers and change information
* @throws Error when git command fails (invalid refs, not a git repo, file not found, etc.)
*/
static diff(fromRef, toRef, filePaths, shouldIncludeUntrackedFiles = false) {
static diff(fromRef, toRef, filePaths, shouldIncludeUntrackedFiles = false, untrackedFileExtensions) {
let command = `git diff -U0 -M ${fromRef}`;
if (toRef) {
command += ` ${toRef}`;
Expand All @@ -26163,7 +26164,10 @@ var Git = class _Git {
const diffOutput = execSync(command);
const diffResult = _Git.parseDiff(diffOutput);
if (!toRef && shouldIncludeUntrackedFiles) {
const untrackedFiles = _Git.getUntrackedFiles(filePaths);
let untrackedFiles = _Git.getUntrackedFiles(filePaths);
if (untrackedFileExtensions) {
untrackedFiles = untrackedFiles.filter((file) => untrackedFileExtensions.some((ext) => file.endsWith(ext)));
}
const untrackedFileDiffs = _Git.createFileDiffsForUntrackedFiles(untrackedFiles);
if (untrackedFileDiffs.length > 0) {
diffResult.files.push(...untrackedFileDiffs);
Expand Down Expand Up @@ -26451,7 +26455,7 @@ var Git = class _Git {
* In CI, uses the GitHub API with pagination for accuracy.
* Locally, uses git diff against the provided ref.
*/
static async getChangedFilesWithStatus(fromRef, toRef, shouldIncludeUntrackedFiles = false) {
static async getChangedFilesWithStatus(fromRef, toRef, shouldIncludeUntrackedFiles = false, untrackedFileExtensions) {
if (IS_CI) {
const files = await GithubUtils_default.paginate(GithubUtils_default.octokit.pulls.listFiles, {
owner: CONST_default.GITHUB_OWNER,
Expand All @@ -26467,7 +26471,7 @@ var Git = class _Git {
previousFilename: file.previous_filename
}));
}
const diffResult = this.diff(fromRef, toRef, void 0, shouldIncludeUntrackedFiles);
const diffResult = this.diff(fromRef, toRef, void 0, shouldIncludeUntrackedFiles, untrackedFileExtensions);
return diffResult.files.map((file) => ({
filename: file.filePath,
status: file.diffType,
Expand All @@ -26486,13 +26490,13 @@ var Git = class _Git {
*/
static getUntrackedFiles(filePaths) {
try {
const untrackedOutput = execSync("git ls-files --others --exclude-standard", {
const untrackedOutput = execSync("git ls-files -z --others --exclude-standard", {
stdio: "pipe"
});
if (!untrackedOutput.trim()) {
if (!untrackedOutput) {
return [];
}
let untrackedFiles = untrackedOutput.trim().split("\n").filter((file) => file.length > 0);
let untrackedFiles = untrackedOutput.split("\0").filter((file) => file.length > 0);
if (filePaths) {
const pathsArray = Array.isArray(filePaths) ? filePaths : [filePaths];
const normalizedPaths = pathsArray.map((p) => path.normalize(p));
Expand Down
2 changes: 1 addition & 1 deletion CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ Do not use `useMemo`, `useCallback`, or `React.memo` in components or hooks that
1. **ESLint**: Run `npm run lint-changed` to catch lint errors early.
2. **TypeScript**: Run `npm run typecheck` after changes that may affect typing (types, interfaces, or function signatures). It runs the TypeScript 7 native compiler and is the required merge gate in CI.
3. **React Compiler**: If you added new React components/hooks or modified existing ones, run `npm run react-compiler-compliance-check check-changed` to verify they compile with React Compiler. This applies the same rules as CI, evaluated against BOTH the Babel and OXC compilers: new components/hooks must compile, existing compiled files must not regress, and changes must not introduce new memoization divergence (one compiler memoizing a file while the other does not). See `contributingGuides/REACT_COMPILER.md` for details and common fixes.
4. **Spelling**: Run `npm run spell-changed -- <files you changed>` to catch spelling errors (cspell needs an explicit file list). CI validates with cspell, which remains the required merge gate.
4. **Spelling**: Run `npm run spell-changed` to catch spelling errors (it discovers changed files itself; pass an explicit file list only if you want to check specific files instead). CI validates with cspell, which remains the required merge gate.

### Testing

Expand Down
3 changes: 3 additions & 0 deletions cspell.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@
"HSBCSGS",
"Handelsbanken",
"Handtool",
"Heapsnapshot",
"Heathrow",
"HiBob",
"Highfive",
Expand Down Expand Up @@ -439,6 +440,7 @@
"WDYR",
"Wallester",
"Warchoł",
"Webp",
"Wintrust",
"Woohoo",
"Wooo",
Expand Down Expand Up @@ -910,6 +912,7 @@
"QAPR",
"QBWC",
"qrcode",
"quotepath",
"rach",
"reactnative",
"reactnativebackgroundtask",
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
"knip-changed": "./scripts/knip-changed.sh",
"shellcheck": "./scripts/shellCheck.sh",
"spell": "cspell --color **/*",
"spell-changed": "cspell --color --no-must-find-files",
"spell-changed": "./scripts/spellChanged.sh",
"fmt": "oxfmt --write .",
"fmt-watch": "onchange \"**/*.{js,mjs,ts,tsx,yml,yaml}\" -- oxfmt --write {{changed}}",
"print-version": "echo $npm_package_version",
Expand Down
22 changes: 14 additions & 8 deletions scripts/knip-changed.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
#!/bin/bash
#
# Run the same knip delta check the CI workflow runs, against your local main.
# Generates knip reports for the current branch and main, then compares them
# with scripts/compareKnipReports.ts.
# Run the same knip delta check the CI workflow runs, against the merge base
# with origin/main. Generates knip reports for the current branch and the
# merge base, then compares them with scripts/compareKnipReports.ts.
#
# Uses a temporary git worktree so your working directory is untouched.
# Reuses your current node_modules (symlinked) — fine for static analysis as
# long as dependencies haven't changed dramatically. If main has drifted
# locally, run `git pull --rebase origin main` first; this script will not
# fetch on your behalf.
# long as dependencies haven't changed dramatically.
#
set -euo pipefail

TOP="$(realpath "$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" &>/dev/null && pwd)/..")"
readonly TOP
source "${TOP}/scripts/shellUtils.sh"

WORKTREE_DIR=$(mktemp -d /tmp/knip-main-worktree.XXXXXX)
CURRENT_REPORT=$(mktemp /tmp/knip-current.XXXXXX.json)
MAIN_REPORT=$(mktemp /tmp/knip-main.XXXXXX.json)
Expand All @@ -22,11 +24,15 @@ cleanup() {
}
trap cleanup EXIT

info "Fetching origin/main"
MERGE_BASE_SHA_HASH="$(get_merge_base_with_main)"
readonly MERGE_BASE_SHA_HASH

echo "Running knip on current branch..."
npm run knip:json > "$CURRENT_REPORT"

echo "Running knip on main..."
git worktree add --detach "$WORKTREE_DIR" main >/dev/null
echo "Running knip on merge base (${MERGE_BASE_SHA_HASH})..."
git worktree add --detach "$WORKTREE_DIR" "$MERGE_BASE_SHA_HASH" >/dev/null
ln -s "$PWD/node_modules" "$WORKTREE_DIR/node_modules"
(cd "$WORKTREE_DIR" && npm run knip:json) > "$MAIN_REPORT"

Expand Down
28 changes: 11 additions & 17 deletions scripts/lintChanged.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,23 @@ TOP="$(realpath "$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" &>/dev/null && pwd)
readonly TOP
source "${TOP}/scripts/shellUtils.sh"

# Fetch the commit history to include the merge-base commit
info "Fetching origin/main"
git fetch origin main --no-tags

MERGE_BASE_SHA_HASH="$(git merge-base origin/main HEAD)"
MERGE_BASE_SHA_HASH="$(get_merge_base_with_main)"
readonly MERGE_BASE_SHA_HASH

# Check if output is empty or malformed
if [[ -z "$MERGE_BASE_SHA_HASH" ]] || ! [[ "$MERGE_BASE_SHA_HASH" =~ ^[a-fA-F0-9]{40}$ ]]; then
error "git merge-base returned unexpected output: $MERGE_BASE_SHA_HASH"
exit 1
fi

# Get the diff output and check status
if ! GIT_DIFF_OUTPUT="$(git diff --diff-filter=AMR --name-only "$MERGE_BASE_SHA_HASH" HEAD -- '*.js' '*.jsx' '*.ts' '*.tsx' '*.mjs' '*.cjs')"; then
error "git diff failed - output: $GIT_DIFF_OUTPUT"
exit 1
# Diffs against the working tree (not HEAD) and includes untracked files, so
# committed, staged, unstaged and untracked changes are all linted
CHANGED_FILES_OUTPUT="$(get_changed_files "$MERGE_BASE_SHA_HASH" '*.js' '*.jsx' '*.ts' '*.tsx' '*.mjs' '*.cjs')"
declare -a ALL_CHANGED_FILES=()
if [[ -n "$CHANGED_FILES_OUTPUT" ]]; then
while IFS= read -r file; do
ALL_CHANGED_FILES+=("$file")
done <<< "$CHANGED_FILES_OUTPUT"
fi

# Run eslint on the changed files, forwarding any user-provided flags
if [[ -n "$GIT_DIFF_OUTPUT" ]] ; then
# shellcheck disable=SC2086 # For multiple files in variable
exec bun "${TOP}/scripts/lint.ts" "$@" $GIT_DIFF_OUTPUT
if [[ "${#ALL_CHANGED_FILES[@]}" -gt 0 ]]; then
exec bun "${TOP}/scripts/lint.ts" "$@" "${ALL_CHANGED_FILES[@]}"
else
info "No lintable files changed"
fi
2 changes: 1 addition & 1 deletion scripts/react-compiler-compliance-check.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ function getMainSource(ref: string, mainPath: string): string | undefined {
*/
async function checkChangedFiles(remote: string, verbose: boolean, checkOxc: OxcChecker): Promise<boolean> {
const mainBaseCommitHash = await Git.getMainBranchCommitHash(remote);
const changedFiles = await Git.getChangedFilesWithStatus(mainBaseCommitHash);
const changedFiles = await Git.getChangedFilesWithStatus(mainBaseCommitHash, undefined, true, FILE_EXTENSIONS);

const reactFiles = changedFiles.filter((f) => FILE_EXTENSIONS.some((ext) => f.filename.endsWith(ext)) && f.status !== 'removed');

Expand Down
58 changes: 50 additions & 8 deletions scripts/shellUtils.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,24 +26,24 @@ if [ -z "${RESET+x}" ]; then
fi

function success {
echo -e "🎉 $GREEN$1$RESET"
echo -e "🎉 $GREEN$1$RESET" >&2
}

function error {
echo -e "💥 $RED$1$RESET"
echo -e "💥 $RED$1$RESET" >&2
}

function info {
echo -e "$BLUE$1$RESET"
echo -e "$BLUE$1$RESET" >&2
}

function title {
printf "\n%s%s%s\n" "$TITLE" "$1" "$RESET"
printf "\n%s%s%s\n" "$TITLE" "$1" "$RESET" >&2
}

# Function to clear the last printed line
clear_last_line() {
echo -ne "\033[1A\033[K"
echo -ne "\033[1A\033[K" >&2
}

# Function to check if Cloudflare WARP is installed and running
Expand Down Expand Up @@ -126,16 +126,58 @@ get_abs_path() {
echo "$abs_path"
}

# Function to read lines from standard input into an array using a temporary file.
# This is a bash 3 polyfill for readarray.
# Fetches origin/main and prints the merge-base SHA between it and HEAD.
# This is the single definition of "base" shared by the *-changed scripts
# (lintChanged.sh, knip-changed.sh, spellChanged.sh) so they all agree on
# what "changed" means.
# Usage: get_merge_base_with_main
get_merge_base_with_main() {
if ! git fetch origin main --no-tags >&2; then
error "git fetch origin main failed"
return 1
fi

local merge_base_sha_hash
merge_base_sha_hash="$(git merge-base origin/main HEAD)" || {
error "git merge-base failed"
return 1
}

if ! [[ "$merge_base_sha_hash" =~ ^[a-fA-F0-9]{40}$ ]]; then
error "git merge-base returned unexpected output: $merge_base_sha_hash"
return 1
fi
Comment on lines +140 to +149

@situchan situchan Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The merge-base validation checks the output instead of the exit status

The -z arm is dead: the empty string already fails the 40-hex regex, and both arms produce the same message.

@JakubKorytko JakubKorytko Aug 31, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

dropped the dead -z arm


echo "$merge_base_sha_hash"
}

# Prints files changed relative to a base commit, diffed against the working
# tree (so committed, staged and unstaged changes are all included), plus
# any untracked files. Excludes deletions.
# Usage: get_changed_files <base_sha> [path spec...]
get_changed_files() {
local base_sha="$1"
shift

git -c core.quotepath=false -c diff.relative=false diff --diff-filter=AMR --name-only "$base_sha" -- "$@" || return 1
git -c core.quotepath=false ls-files --full-name --others --exclude-standard -- "$@"
}

# Function to read lines from standard input into an array.
# This is a bash 3 polyfill for readarray. Uses printf -v (not eval) on each
# line so special shell characters in the input (e.g. from git-derived filenames)
# aren't executed.
# Arguments:
# $1: Name of the array variable to store the lines
# Usage:
# read_lines_into_array array_name
read_lines_into_array() {
local array_name="$1"
local line
local index
eval "index=\${#${array_name}[@]}"
while IFS= read -r line || [ -n "$line" ]; do
eval "$array_name+=(\"$line\")"
printf -v "${array_name}[$index]" '%s' "$line"
index=$((index + 1))
done
}
38 changes: 38 additions & 0 deletions scripts/spellChanged.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#!/bin/bash

# Spell-checks files that have changed in this branch. If file paths are
# passed as arguments (e.g. by CI, which gets its file list from the PR
# API), those are checked instead and no change discovery happens.

set -eu

TOP="$(realpath "$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" &>/dev/null && pwd)/..")"
readonly TOP
source "${TOP}/scripts/shellUtils.sh"

if [[ "$#" -gt 0 ]]; then
exec "${TOP}/node_modules/.bin/cspell" --color --no-must-find-files "$@"
fi

info "Fetching origin/main"
MERGE_BASE_SHA_HASH="$(get_merge_base_with_main)"
readonly MERGE_BASE_SHA_HASH

# Excludes common binary/media file types since spell-checking them is pointless and wasteful
CHANGED_FILES_OUTPUT="$(get_changed_files "$MERGE_BASE_SHA_HASH" ':!*.png' ':!*.jpg' ':!*.jpeg' ':!*.gif' ':!*.webp' ':!*.ico' ':!*.mp4' ':!*.mov' ':!*.zip' ':!*.tar.gz' ':!*.heapsnapshot' ':!*.pdf')"
declare -a ALL_CHANGED_FILES=()
if [[ -n "$CHANGED_FILES_OUTPUT" ]]; then
# Excludes any path starting with "." (dotfiles and top-level dot-directories like .github/), matching CI's filter. A nested dot-directory, e.g. docs/.hidden/config.ts, is still checked.
while IFS= read -r file; do
if [[ "$file" != .* ]]; then
ALL_CHANGED_FILES+=("$file")
fi
done <<< "$CHANGED_FILES_OUTPUT"
fi
readonly -a ALL_CHANGED_FILES

if [[ "${#ALL_CHANGED_FILES[@]}" -gt 0 ]]; then
exec "${TOP}/node_modules/.bin/cspell" --color --no-must-find-files "${ALL_CHANGED_FILES[@]}"
else
info "No changed files to spell check"
fi

@situchan situchan Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This code has the same structure with lintChanged.sh:11-26.
The PR extracted the first two steps into helpers but left the last two duplicated

@JakubKorytko JakubKorytko Aug 31, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

got your point but leaving duplicated - the two tails exec different tools with different args, a shared helper would just be an if/else around two one-liners

23 changes: 12 additions & 11 deletions scripts/utils/Git.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,11 @@ class Git {
* @param fromRef - The starting reference (commit, branch, tag, etc.)
* @param toRef - The ending reference (defaults to working directory if not provided)
* @param filePaths - Optional specific file path(s) to diff (relative to git repo root)
* @param untrackedFileExtensions - Optional extensions (e.g. ['.ts', '.tsx']) to restrict which untracked files get read; avoids reading unrelated untracked files (media, snapshots, etc.) in full
* @returns Structured diff result with line numbers and change information
* @throws Error when git command fails (invalid refs, not a git repo, file not found, etc.)
*/
static diff(fromRef: string, toRef?: string, filePaths?: string | string[], shouldIncludeUntrackedFiles = false): DiffResult {
static diff(fromRef: string, toRef?: string, filePaths?: string | string[], shouldIncludeUntrackedFiles = false, untrackedFileExtensions?: string[]): DiffResult {
// Build git diff command (with 0 context lines for easier parsing, -M for rename detection)
let command = `git diff -U0 -M ${fromRef}`;
if (toRef) {
Expand All @@ -139,7 +140,10 @@ class Git {

// Include untracked files when diffing against working directory
if (!toRef && shouldIncludeUntrackedFiles) {
const untrackedFiles = Git.getUntrackedFiles(filePaths);
let untrackedFiles = Git.getUntrackedFiles(filePaths);
if (untrackedFileExtensions) {
untrackedFiles = untrackedFiles.filter((file) => untrackedFileExtensions.some((ext) => file.endsWith(ext)));
Comment thread
JakubKorytko marked this conversation as resolved.
}
const untrackedFileDiffs = Git.createFileDiffsForUntrackedFiles(untrackedFiles);

// Merge untracked files into the diff result
Expand Down Expand Up @@ -502,7 +506,7 @@ class Git {
* In CI, uses the GitHub API with pagination for accuracy.
* Locally, uses git diff against the provided ref.
*/
static async getChangedFilesWithStatus(fromRef: string, toRef?: string, shouldIncludeUntrackedFiles = false): Promise<ChangedFile[]> {
static async getChangedFilesWithStatus(fromRef: string, toRef?: string, shouldIncludeUntrackedFiles = false, untrackedFileExtensions?: string[]): Promise<ChangedFile[]> {
if (IS_CI) {
const files = await GitHubUtils.paginate(GitHubUtils.octokit.pulls.listFiles, {
owner: CONST.GITHUB_OWNER,
Expand All @@ -520,7 +524,7 @@ class Git {
}));
}

const diffResult = this.diff(fromRef, toRef, undefined, shouldIncludeUntrackedFiles);
const diffResult = this.diff(fromRef, toRef, undefined, shouldIncludeUntrackedFiles, untrackedFileExtensions);
return diffResult.files.map((file) => ({
filename: file.filePath,
status: file.diffType,
Expand All @@ -541,19 +545,16 @@ class Git {
*/
static getUntrackedFiles(filePaths?: string | string[]): string[] {
try {
// Get all untracked files
const untrackedOutput = execSync('git ls-files --others --exclude-standard', {
// -z avoids git C-quoting non-ASCII/special-character paths, so the raw path is preserved for later filtering
const untrackedOutput = execSync('git ls-files -z --others --exclude-standard', {
stdio: 'pipe',
});

if (!untrackedOutput.trim()) {
if (!untrackedOutput) {
return [];
}

let untrackedFiles = untrackedOutput
.trim()
.split('\n')
.filter((file) => file.length > 0);
let untrackedFiles = untrackedOutput.split('\0').filter((file) => file.length > 0);

// Filter by filePaths if provided
if (filePaths) {
Expand Down
Loading
Loading