-
-
Notifications
You must be signed in to change notification settings - Fork 613
feat(Android, Stack v5): allow for nested container pop via JS #3612
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
kkafar
wants to merge
4
commits into
main
Choose a base branch
from
@kkafar/js-pop-nested-stack
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+224
−82
Conversation
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
This allows for cleaner route creation
Previously it has not been possible to pop nested container via JS. When there was only a single screen left in the JS stack state, the reducer would deny removing it and would just ignore the operation printing a warning that pop operation requires at least two screens. Currently, in such scenario the parent `StackContainer` (JS component) will be asked to pop whole screen that hosts the nested container. This is achieved by changes in stack container state reducer. Previously it operated directly on stack, but now I need to somehow communicate that an action has not been performed directly on the container and must be forwarded to parent container. We can not use the `parentNavigation` directly in the reducer, because: 1. reducer should be pure, 2. we can not dispatch an action directly to the parent container during render (and this is when the state reducing takes place), React treats it as an error. Therefore, I see two possibilities here. Either we move all the `StackContainer` state to some centralized place (similarly to what react-navigation does with `NavigationContainer`) or we stay with current approach (each `StackContainer` owns its state) but we emit this intent as an side effect. Right now I went with second option. Reducer, when it find it makes sense, not only updates the stack state, but also produces side effects to be performed. Currently, there is only single side effect: `PopContainerStackNavigationEffect`, but I found it low cost to make it a bit more generic. Reducer produces the side effect, which is later consumed by the new `useParentNavigationEffect` hook. Important thing to note in this implementation is the referential stability of the effects array. The state object id changes every time `state.stack` or `state.effects` change. However, `state.effects` object id stays the same as long as there is no change to prevent excessive rendering. Given a nested stack, with only a single screen in the nested stack, the flow is as follows: 1. `navigation.pop` is dispatched to the nested stack, this triggers render. 2. Reducer detects that it has only a single screen & can not perform the action, therefore it produces new state with new effects array containing the `PopContainerStackNavigationEffect`. 3. The `useParentNavigationEffect` schedules the `useEffect`, which is executed after this render. 4. The `useEffect` is executed triggering render of the outer container and consuming the effect (trigger render of the inner container). 5. New render starts from the outer container -> it performs the navigation action, detaching the screen that hosts nested container 6. During the same render as in 5., nested container clears its `state.effects`. 7. After the dismiss completes, `pop-completed` operation is dispatched by event listeners & hosting screen & nested container are cleared from the state (they no longer render). > [!note] > Thing to note here, is that the nested stack's only screen, when it's > being dismissed it sends `onDismissed(isNativeDismiss=true)` event, > because it is dismissed & it's activityMode has never been changed to > `detached`. I don't think this is causing any problems right now, > but it is definitely something to be aware of. What happens if there is only a single screen in outer stack & it hosts the nested stack? Nothing. Renders are triggered, but there is no one to handle that operation.
t0maboro
approved these changes
Feb 3, 2026
| export function determineFirstRoute( | ||
| function applyPopContainerToEffects( | ||
| effects: StackNavigationEffect[], | ||
| newEffect: StackNavigationEffect, |
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we should be more strict here and use PopContainerStackNavigationEffect ?
| @@ -0,0 +1,39 @@ | |||
| import React from 'react'; | |||
| import { StackNavigationContext } from '../contexts/StackNavigationContext'; | |||
| import { | |||
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: import type
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
Previously it has not been possible to pop nested container via JS.
When there was only a single screen left in the JS stack state, the
reducer would deny removing it and would just ignore the operation
printing a warning that pop operation requires at least two screens.
Currently, in such scenario the parent
StackContainer(JS component)will be asked to pop whole screen that hosts the nested container.
Changes
This is achieved by changes in stack container state reducer.
Previously it operated directly on stack, but now I need to somehow
communicate that an action has not been performed directly on the
container and must be forwarded to parent container. We can
not use the
parentNavigationdirectly in the reducer, because:render (and this is when the state reducing takes place), React
treats it as an error.
Therefore, I see two possibilities here. Either we move all the
StackContainerstate to some centralized place (similarly to whatreact-navigation does with
NavigationContainer) or we stay withcurrent approach (each
StackContainerowns its state) but we emitthis intent as an side effect.
Right now I went with second option. Reducer, when it find it makes
sense, not only updates the stack state, but also produces side effects
to be performed. Currently, there is only single side effect:
PopContainerStackNavigationEffect, but I found it low cost to make ita bit more generic.
Reducer produces the side effect, which is later consumed by the new
useParentNavigationEffecthook.Important thing to note in this implementation is the referential
stability of the effects array. The state object id changes every time
state.stackorstate.effectschange. However,state.effectsobjectid stays the same as long as there is no change to prevent excessive
rendering.
Given a nested stack, with only a single screen in the nested stack,
the flow is as follows:
navigation.popis dispatched to the nested stack, this triggersrender.
the action, therefore it produces new state with new effects array
containing the
PopContainerStackNavigationEffect.useParentNavigationEffectschedules theuseEffect, which isexecuted after this render.
useEffectis executed triggering render of the outer containerand consuming the effect (trigger render of the inner container).
navigation action, detaching the screen that hosts nested container
state.effects.pop-completedoperation is dispatchedby event listeners & hosting screen & nested container are cleared from
the state (they no longer render).
Note
Thing to note here, is that the nested stack's only screen, when it's
being dismissed it sends
onDismissed(isNativeDismiss=true)event,because it is dismissed & it's activityMode has never been changed to
detached. I don't think this is causing any problems right now,but it is definitely something to be aware of.
What happens if there is only a single screen in outer stack & it
hosts the nested stack? Nothing. Renders are triggered, but there is no
one to handle that operation.
Visual documentation
Screen.Recording.2026-02-03.at.16.35.26.mov
Screen.Recording.2026-02-03.at.16.33.15.mov
Test plan
I'm using
TestStackNesting.Just as the video shows.
Checklist