Skip to content

Fix JS API breaking change detection to compare against merge base#54819

Closed
emily8rown wants to merge 3 commits intofacebook:mainfrom
emily8rown:fix-false-positive-breaking-reactNativeApi-changes
Closed

Fix JS API breaking change detection to compare against merge base#54819
emily8rown wants to merge 3 commits intofacebook:mainfrom
emily8rown:fix-false-positive-breaking-reactNativeApi-changes

Conversation

@emily8rown
Copy link
Copy Markdown
Contributor

@emily8rown emily8rown commented Dec 8, 2025

Summary

Updates the diff-js-api-breaking-changes action to compare the PR's changes against the merge base (where the branch diverged from main) instead of the current tip of main.

Problem

The previous implementation compared the current state of main to the PR head. This could produce false positive results when main had new commits to reactNativeApi.d.ts reporting breaking changes from main as if they were introduced by the PR.

Solution

  • Calculate the merge base between the PR head and origin/main
  • Compare the API snapshot at the merge base to the snapshot at the PR head

Changelog:

[GENERAL] [FIXED] - Updates the diff-js-api-breaking-changes action to compare the PR's changes against the merge base (where the branch diverged from main) instead of the current tip of main.

Test Plan:

(outputs shown for #54815)
Tested locally on branch that previously got false positive:
Mirror old approach:

mkdir -p /tmp/api-diff-old
git show origin/main:packages/react-native/ReactNativeApi.d.ts > /tmp/api-diff-old/before.d.ts
git show HEAD:packages/react-native/ReactNativeApi.d.ts > /tmp/api-diff-old/after.d.ts
node ./scripts/js-api/diff-api-snapshot /tmp/api-diff-old/before.d.ts /tmp/api-diff-old/after.d.ts

output:

{
  "result": "BREAKING",
  "changedApis": [
    "ActivityIndicatorProps",
    "Animated",
    "DrawerLayoutAndroidProps",
    "FlatList",
    "FlatListProps",
    "ImageBackground",
    "ImageBackgroundProps",
    "ImageProps",
    "ImagePropsBase",
    "KeyDownEvent",
    "KeyEvent",
    "KeyUpEvent",
    "KeyboardAvoidingView",
    "KeyboardAvoidingViewProps",
    "ModalProps",
    "PressableProps",
    "ProgressBarAndroidProps",
    "RefreshControl",
    "RefreshControlProps",
    "ScrollViewProps",
    "SectionList",
    "SectionListProps",
    "SwitchProps",
    "TextInputProps",
    "ViewProps",
    "VirtualizedListProps",
    "VirtualizedSectionListProps"
  ]
}

Mirror new approach:

git fetch origin main
MERGE_BASE=$(git merge-base HEAD origin/main)
echo "Merge base: $MERGE_BASE"
mkdir -p /tmp/api-diff
git show $MERGE_BASE:packages/react-native/ReactNativeApi.d.ts > /tmp/api-diff/before.d.ts
git show HEAD:packages/react-native/ReactNativeApi.d.ts > /tmp/api-diff/after.d.ts
node ./scripts/js-api/diff-api-snapshot /tmp/api-diff/before.d.ts /tmp/api-diff/after.d.ts 

output:

{
  "result": "NON_BREAKING",
  "changedApis": []
}

…st the branch base instead of the head of main
@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 8, 2025
@emily8rown emily8rown marked this pull request as ready for review December 8, 2025 17:35
@emily8rown emily8rown requested a review from huntie December 8, 2025 17:35
@meta-codesync
Copy link
Copy Markdown

meta-codesync bot commented Dec 8, 2025

@huntie has imported this pull request. If you are a Meta employee, you can view this in D88654826.

@meta-codesync meta-codesync bot closed this in e7f5618 Dec 10, 2025
@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Dec 10, 2025
@meta-codesync
Copy link
Copy Markdown

meta-codesync bot commented Dec 10, 2025

@emily8rown merged this pull request in e7f5618.

@react-native-bot
Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @emily8rown in e7f5618

When will my fix make it into a release? | How to file a pick request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Facebook Partner: Facebook Partner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants