Skip to content

fix(ios): pop only the topmost React Native screen on popToNative - #470

Merged
hurali97 merged 2 commits into
callstack:mainfrom
KisaneNeko:fix/ios-pop-to-native-topmost-surface
Sep 24, 2026
Merged

hurali97 merged 2 commits into
callstack:mainfrom
KisaneNeko:fix/ios-pop-to-native-topmost-surface

Conversation

@KisaneNeko

@KisaneNeko KisaneNeko commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #354.

popToNative() posts an app-wide NotificationCenter notification with no sender, so every live React Native surface acted on it — including ones sitting underneath other screens. With a stack like RN → native → RN, one call popped the lot and took the native screen in the middle with it.

There was a second, smaller problem in the same path: a SwiftUI-hosted surface handled the notification twice, once in ReactNativeView via dismiss() and again in the ReactNativeViewController it wraps via popViewController. So even a single RN screen popped one level too far.

BrownfieldSurfaceRegistry now elects one surface — the most recently shown one that's actually on screen — and only that surface acts, once per host. This matches what Android already does implicitly, since its back dispatcher only ever delivers to the top callback.

A couple of things worth knowing while reviewing:

  • Visibility is read live (view.window) rather than cached from the appearance callbacks. SwiftUI recreates hosted controllers, so a cached flag is stale exactly when the notification arrives — I had it the other way round first and it broke.
  • SwiftUI owns its navigation state and reverts an imperative popViewController, so the SwiftUI host still does the dismissing. The controller stands down and only steps in if the dismiss clearly did nothing, which is the case for a ReactNativeView inside a UIHostingController pushed onto a UIKit stack.

Two behaviour changes are called out in the changeset: popToNative() is now a no-op (with a log) when no RN surface is on screen, and it no longer dismisses a modally presented native screen that merely contains an RN surface.

E2E coverage and a demo screen to exercise this are in a follow-up PR stacked on this one, to keep the fix reviewable on its own.

🤖 Generated with Claude Code

popToNative() is posted as an app-wide NotificationCenter notification with
no sender, so every live React Native surface acted on it — including ones
buried under other screens. In a stack like RN -> native -> RN, one call tore
down everything, taking the native screen in between with it.

On top of that, a SwiftUI-hosted surface handled the notification twice: once
in ReactNativeView via dismiss(), and again in the ReactNativeViewController
it wraps via popViewController.

BrownfieldSurfaceRegistry now picks a single surface — the most recently shown
one that is currently on screen — and only that surface acts, once. Visibility
is read live (view.window) rather than cached from viewWillAppear, because
SwiftUI recreates hosted controllers and a cached flag is stale exactly when
the notification arrives.

The registry has no UIKit dependency, so it runs under `swift test` on macOS in
the existing iOS native tests CI job.

Fixes callstack#354

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks like an unusual place to put this and related files. Let's put this at packages/react-native-brownfield/ios/.

We can then discard the Package.swift change

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I followed the BrownfieldBundleURLResolver example. SPM can't reach outside its package root, so moving the file to ios will delete the unit tests. I can do that but just making sure we're fine with that.

For what it's worth they only cover the surface election logic, which is the simple part. The bugs I actually hit were all in the wiring and the e2e caught those, so losing them isn't a big deal.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah let's ignore this test.

…ources

Per review, the registry sits in ios/ alongside ReactNativeBrownfield.swift
rather than under ios/swiftpm/Sources, so the Package.swift change is no
longer needed.

SPM targets cannot reference sources outside the package root, so the
registry's unit tests go with it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@hurali97
hurali97 merged commit 09162c2 into callstack:main Sep 24, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

popToNative closes all React Native screens

2 participants