Merged
Conversation
yedidyak
reviewed
Apr 19, 2026
On programmatic pop, snapshot the outgoing screen and tear down its React view immediately so componentWillUnmount fires before the next screen mounts. This prevents shared-state races when a screen is popped and re-pushed in quick succession. For interactive pops (swipe-back), skip the early teardown so the React view stays alive if the gesture is cancelled, falling back to the existing delegate cleanup path. Also guards against no-op pops (e.g. popping root) leaving a stuck snapshot, and avoids duplicate componentDidDisappear in multi-pop flows by only emitting it for the previously visible screen.
yedidyak
approved these changes
Apr 20, 2026
Contributor
yedidyak
left a comment
There was a problem hiding this comment.
Approving per request. I still have one follow-up concern on the custom iOS pop-animation path, but treating it as a non-blocking risk for this review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
When navigating between screens of the same component type (pop + push in quick succession), the new screen loads data into a shared store on mount, but the old screen's
componentWillUnmountfires after the new screen has already mounted and appeared — clearing the store and leaving the new screen stuck in a loading/empty state.Root Cause
When a screen is popped on iOS, RNN relies on ARC deallocation of the
UIViewControllerto tear down the React surface (viaRCTSurfaceHostingView.dealloc→[_surface stop]). This deallocation happens asynchronously — after the pop animation completes and UIKit releases its internal references to the view controller.If a new screen is pushed immediately after the pop (especially the same component type), the new screen's React surface mounts before the old one is destroyed. This means
componentDidMountof the new screen fires beforecomponentWillUnmountof the old screen. Any shared state that is loaded on mount and cleared on unmount gets corrupted — the old screen's late unmount wipes data the new screen just loaded.Timeline observed (from the video):
Fix
Modified
RNNStackController.popViewControllerAnimated:to explicitly tear down the React surface during the pop, rather than waiting for ARC:componentDidDisappearthrough the React view while it's still alive, so the JS component receives the event before unmountdestroyReactView) immediately after[super popViewControllerAnimated:]returns — this stops the Fabric surface synchronously, ensuringcomponentWillUnmountfires before any subsequent push creates a new surfaceThe existing
destroyReactViewcall inStackControllerDelegate.didShowViewController:becomes a safe no-op sincereactViewis already nil.Test
Added an E2E test (
"pop and re-push same component should not have stale unmount") that reproduces the exact race condition:pop+pushin the same JS tickWithout the fix the test fails (shows
"stale_unmount"), with the fix it passes. All 156 existing iOS E2E tests continue to pass with no regressions.