Skip to content

Commit 88a6ace

Browse files
huntiemeta-codesync[bot]
authored andcommitted
Fix JS API change false positives for out-of-sync branches (#56871)
Summary: Pull Request resolved: #56871 `diff-js-api-changes` was posting false positive "JavaScript API change detected" warnings on PRs that never modified `ReactNativeApi.d.ts`. This happened because the action ran `git merge-base` against limited local history (`git fetch --depth=500`), which couldn't find the common ancestor for branches that were far behind main — including (often) pick requests to release branches. The fallback wrote an empty "before" snapshot, causing the entire API surface to appear newly added. {F1989977284} #### Changes - Query the GitHub API first — if the PR doesn't touch `ReactNativeApi.d.ts`, skip the diff entirely. - When the snapshot is touched, use the GitHub compare API to resolve the merge base instead of relying on local git history. - Limit the check to PRs targeting `main` or `*-stable` branches. Changelog: [Internal] Reviewed By: christophpurrer Differential Revision: D105572432 fbshipit-source-id: 20e5db01c9e6b51acb55548746c8fd512a363274
1 parent 948835d commit 88a6ace

1 file changed

Lines changed: 35 additions & 14 deletions

File tree

.github/actions/diff-js-api-changes/action.yml

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,30 +7,51 @@ outputs:
77
runs:
88
using: composite
99
steps:
10-
- name: Fetch PR and main, compute merge base
11-
id: merge_base
12-
shell: bash
13-
env:
14-
PR_SHA: ${{ github.event.pull_request.head.sha }}
15-
run: |
16-
git fetch origin main
17-
git fetch origin "$PR_SHA" --depth=500
18-
echo "merge_base=$(git merge-base "$PR_SHA" origin/main)" >> $GITHUB_OUTPUT
10+
- name: Resolve API diff inputs
11+
id: api_diff
12+
uses: actions/github-script@v8
13+
with:
14+
script: |
15+
// Check if we are targeting main or a stable branch
16+
const baseRef = context.payload.pull_request.base.ref;
17+
if (baseRef !== 'main' && !baseRef.endsWith('-stable')) {
18+
core.setOutput('changed', false);
19+
return;
20+
}
21+
22+
// Check if the diff touches ReactNativeApi.d.ts
23+
const files = await github.paginate(github.rest.pulls.listFiles, {
24+
...context.repo,
25+
pull_number: context.payload.pull_request.number,
26+
});
27+
const changed = files.some(f => f.filename === 'packages/react-native/ReactNativeApi.d.ts');
28+
core.setOutput('changed', changed);
29+
if (!changed) return;
30+
31+
// Calculate merge base
32+
const {data} = await github.rest.repos.compareCommits({
33+
...context.repo,
34+
base: context.payload.pull_request.base.ref,
35+
head: context.payload.pull_request.head.sha,
36+
});
37+
core.setOutput('merge_base', data.merge_base_commit.sha);
1938
20-
- name: Extract before and after API snapshots
39+
- name: Fetch commits and extract API snapshots
40+
if: steps.api_diff.outputs.changed == 'true'
2141
shell: bash
2242
env:
2343
SCRATCH_DIR: ${{ runner.temp }}/diff-js-api-changes
24-
MERGE_BASE: ${{ steps.merge_base.outputs.merge_base }}
25-
PR_SHA: ${{ github.event.pull_request.head.sha }}
2644
run: |
2745
mkdir -p $SCRATCH_DIR
28-
git show "$MERGE_BASE":packages/react-native/ReactNativeApi.d.ts > $SCRATCH_DIR/ReactNativeApi-before.d.ts \
46+
git fetch origin "${{ steps.api_diff.outputs.merge_base }}" --depth=1
47+
git fetch origin "${{ github.event.pull_request.head.sha }}" --depth=1
48+
git show "${{ steps.api_diff.outputs.merge_base }}":packages/react-native/ReactNativeApi.d.ts > $SCRATCH_DIR/ReactNativeApi-before.d.ts \
2949
|| echo "" > $SCRATCH_DIR/ReactNativeApi-before.d.ts
30-
git show "$PR_SHA":packages/react-native/ReactNativeApi.d.ts > $SCRATCH_DIR/ReactNativeApi-after.d.ts \
50+
git show "${{ github.event.pull_request.head.sha }}":packages/react-native/ReactNativeApi.d.ts > $SCRATCH_DIR/ReactNativeApi-after.d.ts \
3151
|| echo "" > $SCRATCH_DIR/ReactNativeApi-after.d.ts
3252
3353
- name: Run breaking change detection
54+
if: steps.api_diff.outputs.changed == 'true'
3455
shell: bash
3556
env:
3657
SCRATCH_DIR: ${{ runner.temp }}/diff-js-api-changes

0 commit comments

Comments
 (0)