refactor: Bump @react-navigation/core to 7.21.12 and drop the now-upstream beforeRemove patch - #98097
Conversation
…beforeRemove patch
…-changes-modal-doesnt-show-up-when-going-back-with-the-browser-back-button
|
@ChavdaSachin Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
|
|
Can you also link the upstream PR for future reference? |
|
Sure, updated the description |
|
@TaduJR Do you think we should include QA testing for this - since other things would have also changed between the library versions? |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2026-08-10.at.3.32.13.PM.mov |
MonilBhavsar
left a comment
There was a problem hiding this comment.
Are we adding a new patch here patches/react-navigation/@react-navigation+core+7.21.12+002+platform-navigation-stack-types.patch - why?
Julesssss
left a comment
There was a problem hiding this comment.
Could you add tests for review and QA please
…-changes-modal-doesnt-show-up-when-going-back-with-the-browser-back-button
…-changes-modal-doesnt-show-up-when-going-back-with-the-browser-back-button
|
Not a new patch. This is the existing patch-package matches a patch to a package by the version in its filename, so bumping core to 7.21.12 means every core patch has to carry |
Done. |
Explanation of Change
The discard-changes fix from #93268 was shipped as a patch to
@react-navigation/core. That fix is now merged and released upstream in version 7.21.12 (react-navigation/react-navigation#13153), so we no longer need to carry the patch.This PR bumps
@react-navigation/coreto 7.21.12 and deletes that patch. The bump also pulled@react-navigation/routersup to 7.6.4.No new patch is being introduced. patch-package keys a patch to a package by the version in its filename, so the existing patches had to be renamed to the new versions:
core …003 propagate-beforeremove— deleted (now upstream)core …001 fix-failing-jest7.16.1 → 7.21.12— rename onlyrouters …001 fix-failing-jest7.5.3 → 7.6.4— rename onlycore …001 platform-navigation-stack-types7.16.1 → 7.21.12 (+002)— rename + regenerateThe
platform-navigation-stack-typesone appears in the diff as a deleted file plus an added file rather than a rename, because 7.21.12 changed the code it patches (RouteProp→ScreenRoutePropin the callbacks,.jsimport extensions), so it had to be regenerated. Its purpose is unchanged (customScreenOptionstyping).No behavior change. The only removed fix is the beforeRemove one, which now comes from the library.
Fixed Issues
$ #84246
PROPOSAL: #84246 (comment)
Tests
Test 1 — Prompt appears when leaving with unsaved input
50).Test 2 — Cancel keeps your input
Test 3 — Discard leaves the screen
Test 4 — No prompt when there is nothing to discard
Test 5 — No prompt after moving forward
Test 6 — Prompt works for other input fields
For each field below: type something, then go back, and confirm the "Discard changes?" prompt appears.
Expected each time: the prompt appears.
Test 7 — Switching tabs, then going back
Test 8 — General navigation sanity (because a core library was updated)
Offline tests
Same as tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Mac-Chrome.mp4