Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the iOS tabs JS-driven navigation update flow by introducing a dedicated “state update request” type so the controller can propagate the request’s actionOrigin rather than hardcoding it.
Changes:
- Introduces
RNSTabsNavigationStateUpdateRequest(selected key, base provenance, action origin) and threads it through the JS-driven update pipeline. - Retypes pending updates and rejection plumbing (controller ↔ component view ↔ event emitter) to use the new request type, including rejection payload fields.
- Minor formatting-only changes in
RNSScreenStackHeaderConfig.mm.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| ios/tabs/host/RNSTabsNavigationState.h | Adds the new request type and organizes related enums/types. |
| ios/tabs/host/RNSTabsNavigationState.mm | Implements RNSTabsNavigationStateUpdateRequest utilities (init, cloneRequest, factory). |
| ios/tabs/host/RNSTabBarController.h | Retypes delegate + pending-update API to accept the request type. |
| ios/tabs/host/RNSTabBarController.mm | Stores pending updates as requests; forwards request actionOrigin on success; uses baseProvenance for staleness checks and rejection callbacks. |
| ios/tabs/host/RNSTabsHostComponentView.h | Retypes navStateRequest to the request type. |
| ios/tabs/host/RNSTabsHostComponentView.mm | Constructs requests from props (ProgrammaticJs) and forwards cloned requests; updates rejection delegate signature/payload. |
| ios/tabs/host/RNSTabsHostEventEmitter.h | Renames/retypes rejected payload field to rejectedRequest. |
| ios/tabs/host/RNSTabsHostEventEmitter.mm | Emits rejection payload using rejectedRequest.selectedScreenKey / baseProvenance. |
| ios/RNSScreenStackHeaderConfig.mm | Formatting-only line wrapping/indentation changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
kkafar
added a commit
that referenced
this pull request
Apr 29, 2026
Mirrors the iOS split landed in PR #3950. `TabsNavState` previously served as both the authoritative current state and the JS-driven update request, with `provenance` doing double duty as both "current generation" and "base provenance the request was derived from". Splits out `TabsNavStateUpdateRequest(selectedScreenKey, baseProvenance, actionOrigin)` as a sibling type. Origin assignment moves from the container to the request-construction site (`TabsHostViewManager.setNavStateRequest`). The container becomes origin-agnostic on the JS-driven path: `onMenuItemSelected` reads `actionOrigin` off the buffered `TabSelectOp.request` instead of hardcoding `PROGRAMMATIC_JS`. Today only `PROGRAMMATIC_JS` flows through, so runtime behavior is unchanged — but future native-programmatic origins can land here without touching the container. Also bundles a `TabsNavState.selectedKey -> selectedScreenKey` rename for consistency with the JS contract and the new request type. JS-facing WritableMap keys (`selectedScreenKey`, `rejectedScreenKey`) were already named correctly; this only aligns the Kotlin-internal field name.
10 tasks
t0maboro
approved these changes
Apr 29, 2026
kligarski
approved these changes
Apr 29, 2026
The JS-driven update path was using a single type, RNSTabsNavigationState, to represent both the controller's authoritative state (selectedScreenKey + provenance) and a JS request to change it (target screenKey + baseProvenance). With actionOrigin now part of the public event API and the design needing room for more origins (notably native-programmatic in the future), the request needs to carry its own origin alongside its base provenance. Flesh out the previously-stub `RNSTabsNavigationStateUpdateRequest` with screenKey, baseProvenance, and actionOrigin, plus a utility surface (designated init, cloneRequest, static factory) mirroring RNSTabsNavigationState. Thread it through: - `RNSTabsHostComponentView` constructs the request from props with `actionOrigin = ProgrammaticJs` and forwards it to the controller. - `RNSTabBarController.setPendingNavigationStateUpdate:` and the ivar (renamed `_pendingOperation` → `_pendingStateUpdate`) take the new type. The controller's success-path context now reads `actionOrigin` from the pending request instead of hardcoding `ProgrammaticJs`, so the controller becomes origin-agnostic on this path. - `isNavigationStateUpdateStale:` reads `baseProvenance` from the request. - The rejection event payload (`OnTabSelectionRejectedPayload`) and the corresponding delegate parameter are retyped from `RNSTabsNavigationState *rejectedNavState` to `RNSTabsNavigationStateUpdateRequest *rejectedRequest`. The JSI emit reads `screenKey` / `baseProvenance` from the request. Scope is iOS only; no JS / Android / codegen-spec changes. The runtime value of `actionOrigin` on the public event is unchanged today (still `'programmatic-js'` for JS-driven updates) — this is purely an internal architectural shift that opens the door to distinguishing native-programmatic origins later.
Align `RNSTabsNavigationStateUpdateRequest`'s field name with both the public JS contract (`TabsHostNavStateRequest.selectedScreenKey`) and the sibling `RNSTabsNavigationState.selectedScreenKey`. The inconsistency was a leftover from when the request type was first sketched in isolation. Touches the property, the designated initializer + static factory selector labels, the ivar, and the three call sites that read it (component view rejection assertion, controller pending-update read, event emitter JSI emit). No semantic change.
I've decided to check this in, because it reappears after every single commit.
Mirrors the iOS split landed in PR #3950. `TabsNavState` previously served as both the authoritative current state and the JS-driven update request, with `provenance` doing double duty as both "current generation" and "base provenance the request was derived from". Splits out `TabsNavStateUpdateRequest(selectedScreenKey, baseProvenance, actionOrigin)` as a sibling type. Origin assignment moves from the container to the request-construction site (`TabsHostViewManager.setNavStateRequest`). The container becomes origin-agnostic on the JS-driven path: `onMenuItemSelected` reads `actionOrigin` off the buffered `TabSelectOp.request` instead of hardcoding `PROGRAMMATIC_JS`. Today only `PROGRAMMATIC_JS` flows through, so runtime behavior is unchanged — but future native-programmatic origins can land here without touching the container. Also bundles a `TabsNavState.selectedKey -> selectedScreenKey` rename for consistency with the JS contract and the new request type. JS-facing WritableMap keys (`selectedScreenKey`, `rejectedScreenKey`) were already named correctly; this only aligns the Kotlin-internal field name.
fefd025 to
0d7d7fb
Compare
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.
Description
Note
This PR stacks on top of #3949 — please review/land that one first. The base branch here is
@kkafar/rename-is-native-action, notmain.The native tabs JS-driven nav state update path was using a single type —
RNSTabsNavigationStateon iOS andTabsNavStateon Android — to represent two distinct concepts:(selectedScreenKey, provenance).navStateRequestprop:(selectedScreenKey, baseProvenance).This conflation was tolerable while we only had one origin (
ProgrammaticJs/PROGRAMMATIC_JS), but with the new publicactionOriginenum and the need to support more origins in the future (notably native-programmatic), the request needs to carry its own origin alongside its base provenance. The state type is the wrong place for that field — it describes a result, not a request.This PR introduces a sibling
…UpdateRequesttype on both platforms and threads it through the JS-driven update path. The architectural consequence is that the native container becomes origin-agnostic on its success path: instead of hardcodingactionOrigin = ProgrammaticJswhen the update succeeds, it forwards whatever the incoming request carried. The view manager / component view (which knows the origin isProgrammaticJsfor prop-driven updates) is now the place that decides.The runtime value of
actionOriginon the public event is unchanged today — JS-driven updates still surface'programmatic-js'. This is purely an internal architectural shift.Changes
iOS
RNSTabsNavigationStateUpdateRequesttype inRNSTabsNavigationState.h/.mmwithselectedScreenKey,baseProvenance,actionOrigin. MirrorsRNSTabsNavigationState's utility surface (designated init,cloneRequest, static factory).RNSTabsHostComponentView:navStateRequestproperty retyped toRNSTabsNavigationStateUpdateRequest *.updateProps:oldProps:constructs the request directly from C++ props withactionOrigin = RNSTabsActionOriginProgrammaticJsand forwards acloneRequestto the controller.rejectedStateUpdateTo:delegate impl readsrequest.selectedScreenKey/request.baseProvenancefor the rejection event payload.RNSTabBarController:setPendingNavigationStateUpdate:retyped to take the request._pendingOperation→_pendingStateUpdateand retyped.RNSTabsNavigationStateUpdateContextnow uses_pendingStateUpdate.actionOrigininstead of hardcodedProgrammaticJs.isNavigationStateUpdateStale:retyped; reads.baseProvenance.rejectedStateUpdateTo:parameter retyped to the request.OnTabSelectionRejectedPayload.rejectedNavState(inRNSTabsHostEventEmitter.h) renamed/retyped torejectedRequest: RNSTabsNavigationStateUpdateRequest *. The JSI emit readsselectedScreenKey/baseProvenancefrom the request.Android
TabsNavStateUpdateRequestdata class inTabsNavState.ktwithselectedScreenKey,baseProvenance,actionOrigin.TabsHostViewManager.setNavStateRequestconstructs the request withactionOrigin = TabsActionOrigin.PROGRAMMATIC_JSbaked in (instead of just aTabsNavState).TabsHost.jsNavStateRequestretyped toTabsNavStateUpdateRequest?.updateJSNavStateRequestretyped.TabSelectOpretyped to wrap aTabsNavStateUpdateRequest(fieldnavState→request).TabsContainer:onMenuItemSelectedno longer hardcodesPROGRAMMATIC_JSin the external-context branch — it reads(pendingOperation as TabSelectOp).request.actionOrigin. TheisInExternalOperationContextflag stays (it still suppresses the prevent-native-selection check and thelastUINavStatecapture).isNavStateStaleretyped; readsrequest.baseProvenance.performOperationreads updated totabSelectOp.request.….TabsContainerDelegate.onNavStateUpdateRejectedparameterrejectedNavState: TabsNavStateretyped torejectedRequest: TabsNavStateUpdateRequest.TabsHostEventEmitterandTabsHostTabSelectionRejectedEventfollow.TabsNavState.selectedKey→selectedScreenKeyrename for cross-platform consistency with the JS contract and the new request type. JS-facingWritableMapkeys (selectedScreenKey,rejectedScreenKey) were already named correctly — only the Kotlin-internal field name moves.No JS, codegen-spec, or conversion-helper changes — public contract untouched.
Test plan
No new automated tests; this is an internal architectural refactor with no behavioral change on the user/JS-driven paths. Manual verification via
FabricExample:iOS
actionOrigin === 'user'(unchanged path).navStateRequestupdate →actionOrigin === 'programmatic-js'(now sourced from the request, not hardcoded in the controller).actionOrigin === 'implicit'(unchanged path).rejectStaleNavStateUpdates: true, mid-flight tap) →onTabSelectionRejectedfires withrejectedScreenKeyandrejectedProvenancematching the request'sselectedScreenKeyandbaseProvenance.Android
actionOrigin === 'user'(unchanged path).navStateRequestupdate →actionOrigin === 'programmatic-js'(now sourced from the request, not hardcoded in the container).rejectStaleNavStateUpdates: true, mid-flight tap) →onTabSelectionRejectedfires withrejectedScreenKeyandrejectedProvenancematching the request'sselectedScreenKeyandbaseProvenance.Reproduction lives in the
FabricExampleTabs test screens; loge.nativeEvent.actionOriginfromonTabSelectedande.nativeEvent.rejectedScreenKey/rejectedProvenancefromonTabSelectionRejected.Checklist