Bulk navigation action dispatch

[dantman]

Sometimes it is necessary to dispatch more than one type of navigation action at once. In my case I found a situation where I needed to trigger a setParams and navigate action at the same time (navigating to a new scene at the same time as updating the current scene's params to close the menu with the button that triggered the navigation).

However you can't actually just do navigation.setParams(...); navigation.navigate('...'); because from what I can tell dispatches trigger a setState and you don't get the new state you need till the next tick.

So I believe it makes sense for us to have way to trigger multiple actions at the same time, at least at the low level something like:

navigation.dispatch(NavigationActions.bulk({
  actions: [
    NavigationActions.setParams({
      key: navigation.state.key,
      params: {...},
    }),
    NavigationActions.navigate({
      routeName: '...'
    })
  ]
}));

[ericvicenti]

Yeah I think this would be a good feature to have

[nihgwu]

would batch be more semantic?

[ericvicenti]

I'm not sure what we should name it, but I do prefer batch over bulk

[matthamil]

Would it not be more semantic to name it NavigationActions.all()? The idea being that native Promises with Promise.all take an array of promises and then resolve all of them. Two cents.

[grabbou]

My +1 goes to batch

[svicalifornia]

Why not make navigation actions return Promises to enable chaining? I don't see a need for batch actions if each navigation action becomes thennable.

[dantman]

@svicalifornia That sounds nice in principle, however I believe that will result in undesirable overlapping transitions in some situations.

[svicalifornia]

@dantman Actually, it would help remove transition overlap problems, assuming that each Promise would be fulfilled only after its corresponding Transition completed. (Thus Transitions should return Promises as well, if they don't already.)

If navigation actions returned promises, then it would be easy to chain together two navigation transitions, and it would also be simple to run multiple navigations simultaneously with any combination of Promise.all and other sequencing functions.

[dantman]

@svicalifornia That's even worse. Besides being much more complex to implement; That means that 2+ navigation actions in bulk will result in watching one navigation transition after another instead of the desired transition directly to the final state.

[svicalifornia]

@dantman I already covered simultaneous actions: "it would also be simple to run multiple navigations simultaneously with any combination of Promise.all…"

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/all

[svicalifornia]

Here's how several common use cases would be handled with Promises:

• Sequence two transitions with no overlap:

  navigate('X').then(() => navigate('Y'))

• Sequence three or more transitions with no overlap:

  navigate('X').then(() => navigate('Y')).then(() => navigate('Z')).then...

• Run multiple transitions simultaneously:

  Promise.all([ navigate('X'), navigate('Y'), ... ])

• Set params before navigating:

  setParams(...).then(() => navigate('X'))

• Run some other code after navigating:

  navigate('X').then(() => { / your code here / })

You can sequence or parallelize Promises however you want. This is far better than a simple batch action. There's no reason to add a batch action if the existing navigation actions used Promises.

[svicalifornia]

@dantman Back to your specific issue that raised above, the main problem seems to be that state is not updated synchronously with navigate actions. Your code should work as it is. React Navigation should be fixed to update state after each navigation action, so that the state is always consistent with the sequence of navigation actions that have been started.

[dantman]

@svicalifornia That won't work and still leaves out one use case.

Your examples have:

• 2 sequential transitions
• 3 sequential transitions
• 2 parallel transitions
• 2 sequential transitions

A navigation.dispatch(?bulk?(NavigationActions.navigate(...), NavigationActions.navigate(...)) is one dispatch and should only have one transition. There should be no parallel or sequential transitions. This is still not possible with what you describe.

Promises like you describe would be nice, but they are a solution to a different problem (part of #135) not this issue.

Additionally the parallel versions and exact patterns you describe are impossible or undesirable from the react standpoint.

The current problem is that navigation is effectively similar to an immutable object that doesn't update to the next tick or return itself. ie: You need access to the new object in order to run a second action on it.

And from a react standpoint the immutable part of that is desirable, because it's necessary for the PureComponent pattern to work and react-navigation to perform well. So I'm opposed to the idea of "fixing" things so you're able to navigate using the old navigator object instead of the new navigator.

Sequential promises would still work in this context, they'd just effectively give you access to the new navigator.

navigator.navigate('X').then((navigator) => navigator.navigate('Y');

The parallel Promise.all([ navigate('X'), navigate('Y'), ... ]) pattern however wont work and has other problems with it.

Firstly, this is the exact same as writing navigation.navigate('X'); navigation.navigate('Y'); which does not work for the previously described reason.

Secondly, if this were in fact parallel (which it's not) you'd instead have a race condition where navigations dependent on each other an be executed out of order.

[svicalifornia]

1. Parallel transitions definitely do exist. For example, I'm working on an app that opens a modal from a drawer. The drawer slides closed while a modal opens from the bottom, with the drawer animation happening behind that. It's a weird pattern, I know, but it works.
2. Parallel promises would work fine for the kind of parallel transitions I described in #1, when you need to sequence some other code after the completion of both transitions. Admittedly this seems like it would be rare, but you were complaining about having to sequence transitions.
3. I agree that Promise.all would not solve your original problem above, which I addressed in a separate comment that you might have missed while you were writing your novella. Here again:

Back to your specific issue that raised above, the main problem seems to be that state is not updated synchronously with navigate actions. Your code should work as it is. React Navigation should be fixed to update state after each navigation action, so that the state is always consistent with the sequence of navigation actions that have been started.

Batch actions won't solve your issue either. How would the batch action know when to start your second action? How long should it wait to know that the state has been updated? The real problem is that state is not being updated immediately. Any delay at all then forces the question of how long to wait to catch the update before firing off another action.

[svicalifornia]

The current problem is that navigation is effectively similar to an immutable object that doesn't update to the next tick or return itself

Since when are immutable JS functions asynchronous? If that's actually true for you, then you might want to get a better immutable library.

[svicalifornia]

And from a react standpoint the immutable part of that is desirable, because it's necessary for the PureComponent pattern to work and react-navigation to perform well.

PureComponents can work just fine with mutable data. Apparently you've never used MobX.

Of course, React Navigation needs to support immutable data as well, but I still don't see how that has anything to do with the observed delay you're seeing in updating the state.

[matthamil]

@svicalifornia It has been a common issue that navigation does not update nav state synchronously. See the following where devs have had to wrap navigation actions in setTimeout:

https://github.com/react-community/react-navigation/issues/1127#issuecomment-297513213 https://github.com/react-community/react-navigation/issues/288#issuecomment-279125934 https://github.com/react-community/react-navigation/issues/288#issuecomment-291187699 https://github.com/react-community/react-navigation/issues/978#issuecomment-293233932

I agree with you that state should be updated after every action. It appears that state is updating after every animation completion, which is why some members of the community have found that setTimeout resolves this issue for them (which is an ugly hack). I could be wrong, but I think this is the current behavior because, technically, you haven't navigated to the next screen until the animation is complete (meaning state should be updated after the animation not before it).

[dantman]

Batch actions won't solve your issue either. How would the batch action know when to start your second action? How long should it wait to know that the state has been updated? The real problem is that state is not being updated immediately. Any delay at all then forces the question of how long to wait to catch the update before firing off another action.

Wait? Delay? I've been describing batch/bulk dispatch from my first comment: navigation.dispatch(?bulk?(actionA, actionB)). There is no delay and your comment about how it should know when the state is updated makes no sense.

A bulk dispatch happens immediately. The navigation actions are immediately combined in order (the mutations they apply to the navigation state are applied synchronously to the state without waiting a tick) and then the dispatch happens. There are no transitions, ticks, or delays until the new state has been resolved.

---

The current problem is that navigation is effectively similar to an immutable object that doesn't update to the next tick or return itself Since when are immutable JS functions asynchronous? If that's actually true for you, then you might want to get a better immutable library.

You completely misread that. Immutable objects/mutations are not asynchronous. navigation being immutable is desirable. The problem is that in react-navigation's case the mutation does not return the result and you can't get ahold of the result until the next tick when you get the new props.navigation.

[matthamil]

I think immediate multiple batched actions would be a good addition to the library. I think this is a simpler mental model than managing multiple async navigations.

On Fri, Apr 28, 2017 at 11:00 PM Daniel Friesen notifications@github.com wrote:

Batch actions won't solve your issue either. How would the batch action know when to start your second action? How long should it wait to know that the state has been updated? The real problem is that state is not being updated immediately. Any delay at all then forces the question of how long to wait to catch the update before firing off another action.

Wait? Delay? I've been describing batch/bulk dispatch from my first comment: navigation.dispatch(?bulk?(actionA, actionB)). There is no delay and your comment about how it should know when the state is updated makes no sense.

A bulk dispatch happens immediately. The navigation actions are immediately combined in order (the mutations they apply to the navigation state are applied synchronously to the state without waiting a tick) and then the dispatch happens. There are no transitions, ticks, or delays until the new state has been resolved.

The current problem is that navigation is effectively similar to an immutable object that doesn't update to the next tick or return itself Since when are immutable JS functions asynchronous? If that's actually true for you, then you might want to get a better immutable library.

You completely misread that. Immutable objects/mutations are not asynchronous. navigation being immutable is desirable. The problem is that in react-navigation's case the mutation does not return the result and you can't get ahold of the result until the next tick when you get the new props.navigation.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/react-community/react-navigation/issues/1206#issuecomment-298145268, or mute the thread https://github.com/notifications/unsubscribe-auth/ALQV3lJ4rVQHnP7SJSuuwGOzskNl5wbDks5r0rXMgaJpZM4NGUtk .

[svicalifornia]

@matthamil Thanks. It figures that lots of people are reporting this issue, since it's really not good for the state to be inconsistent with the actions that have been executed. Regardless of whether the animations have completed, the state should change when the action functions return, or the action functions should return Promises. But to cover both state and animations, it would be best for the state to change immediately, and the action function to return a Promise that fulfills when the animation finishes.

However, batch actions will not solve this problem. Either they run immediately, in which case they won't pick up the state change, or they run… when? How do we know when the state will change, so that we can run the next action in the batch?

@dantman OK, but don't blame me for misreading your ambiguous writing.

Anyway, can we all agree that React Navigation should handle state changes immediately upon each navigation action? Neither batch actions nor promises adequately solve the issue at hand.

[matthamil]

It will be interesting to see how this is implemented. Probably best to continue discussion once a PR is open :)

[dantman]

Here are a few relevant facts from examining the react-nativation internals:

• navigation.dispatch already returns a boolean to indicate if the dispatch happened; so any change to that api will be a breaking change
• dispatches very much appear to work like immutable data:
  • dispatch is implemented on a react component
  • This component stores the raw current navigation state data in state.nav
  • On navigation, dispatch passes the action data and state.nav to getStateForAction to get the new state data and then use this.setState({ nav: ... }) to update state.nav with that new data
  • render() uses addNavigationHelpers to create the navigation prop based on that state data. It is careful to cache it in this._navigation so it's not re-created every render, update it only when state.nav is not === to the previous state.nav (ie: treating it as immutable), and use addNavigationHelpers to create a whole new object when updating instead of modifying the existing this._navigation. ie: dispatch/NavigationContainer is very careful to treat navigation like an immutable object intended to work well with react's built-in PureComponent pattern.
  • This means that currently state.nav cannot be accessed at all until the next tick because it's only updated via setState and the new navigation object with the updated state also cannot be accessed until the render() happens and updates this._navigation.
  • This also means that when you run 2 dispatches in the same tick this is what happens:
    1. dispatch runs action#1 on the original nav data (A) to result in the new state (B)
    2. dispatch uses setState to set state.nav to (B)
    3. The second dispatch runs action#2 on state.nav which because setState hasn't updated state.nav yet is still the original state (A) and results in the new state (C)
    4. dispatch uses setState to set state.nav to (C)
    5. setState resolves, the final state is either (B) or (C) (depending on how setState decides to handle multiple calls in the same tick), but both are based directly (A) so one of the dispatches are effectively discarded

I am opposed to:

• Anything that breaks the current immutable pattern dispatch/NavigationContainer uses
• Abusing forceUpdate to completely eliminate this.state.nav

I am in favour of: (not exclusively, we can reasonably have more than one these when they don't conflict with each other)

• Some way of waiting for transitions/animations to complete (to fix #135) whether this is a promise returned by dispatch (breaking change) or a separate module/method like InteractionManager.
• Making dispatch accept multiple actions (whether we use an array, ...rest params, or a NavigationActions.{bulk,batch,...}) this would effectively just mean that dispatch's existing const nav = Component.router.getStateForAction(action, state.nav); would just be updated to getStateForAction one action then pass the result (if not undefined) to the next getStateForAction(action, result), and so on.
• Temporarily storing state.nav somewhere besides state.nav during a tick so that following calls to dispatch during the same tick can update the navigation state
• Trying to use this.setState's callback mode to make calls to dispatch build on each other
• Making dispatch work immutably, ie: returning the new navigation state so you can navigation.navigate('A').navigate('B') and also have access to the new navigation.state value before your component is re-rendered with the new value
• If we don't make sync navigation.dispatch(...); navigation.dispatch(...); calls work: During __DEV__ mode, tracking whether a dispatch happened this tick and emitting a warning if multiple dispatches happened and one is being discarded

[svicalifornia]

@dantman Thanks for the deeper investigation and clear and thorough writeup.

In response to your proposals:

1. Waiting for animations to finish isn't always desirable. Animation promises have their own merits, but they aren't the optimal solution for the current issue.

2. The official docs encourage apps to provide their own dispatchers (for integration with Redux or other state management). Rewriting the built-in dispatch function to handle batch-action chaining would require all custom dispatchers to implement batch-action chaining as well. It seems like we would be forcing significant costs upon some developers by not handling this issue at another level.

3. Caching state.nav somewhere seems like a hack. You'd have to give that intermediate state to any custom dispatchers, and they might be confused to receive intermediate state, which violates the React convention of asynchronous state changes.

4. This one (using this.setState's callback mode) seems like it could be a winner.

5. I like this idea too, but I don't know how you'd do this without changing the return signature of dispatch, which would potentially break apps with custom dispatchers.

Did you know that React Navigation already supports compound actions? You can chain actions together yourself like this:

{
  "type": "Navigation/NAVIGATE",
  "routeName": "Somewhere",
  "action": {
    "type": "Navigation/NAVIGATE",
    "routeName": "SomewhereElse"
  }
}

Make the first one type "Navigation/SET_PARAMS", and maybe that will work for you.

[dantman]

Waiting for animations to finish isn't always desirable. Animation promises have their own merits, but they aren't the optimal solution for the current issue.

I know. This was not an exclusive list (see: "(not exclusively, we can reasonably have more than one these when they don't conflict with each other)"), no one item on the list solves every use case. I put it in the list because I'm not opposed to it, and it solves some use cases while we will need a different solution for others.

3. Caching state.nav somewhere seems like a hack. You'd have to give that intermediate state to any custom dispatchers, and they might be confused to receive intermediate state, which violates the React convention of asynchronous state changes.
4. This one (using this.setState's callback mode) seems like it could be a winner.

These two are actually pretty much the same thing: Both of change the dispatch method a bit. Both of them are limited in scope to the NavigationContainer (4. attempts to use setState's callback to get the state modified during the tick while 3. uses a temporary this._nav to store changes made during the tick, basically the same way that this._navigation is used), neither violates the react pattern or makes changes out of NavigationContainer.

The only difference is in calling setState vs. temporarily using this._nav for holding changes that setState have not yet resolved. 4. theoretically looks cleaner if it'll work, 3. is the backup if it doesn't.

The theoretical problems with 4. are:

• It involves always calling this.setState (because we can't make use of this.state, we have to use the state arg provided by setState as the input to getStateForAction). Because NavigationContainer is not pure itself, it's possible that this could result in render() being called in cases where the state has not changed and it currently is not called (ie: action is invalid, getStateForAction returns undefined or the same state it was passed) which would result in all non-pure descendants being pointlessly re-rendered
• The callback form of setState may be asynchronous, which would break dispatch's ability to synchronously return a boolean indicating whether the navigation action was accepted

I like this idea too, but I don't know how you'd do this without changing the return signature of dispatch, which would potentially break apps with custom dispatchers.

Sure. Although I should note, from what I can tell it look like the return value of dispatchers is already not standardized. Ours returns a boolean. Looking at the redux example, the react-redux code, and the redux code. It appears as if that pattern ditches NavigationContainer and it's dispatcher entirely, doesn't implement it's own dispatcher, and instead just passes redux's dispatcher to addNavigationHelpers.

Judging by the redux source code, that dispatch returns the action you passed it instead of a success boolean. And it seems like middleware can wrap that dispatch method in variations that support Promises, etc...

Did you know that React Navigation already supports compound actions? You can chain actions together yourself like this:

This has nothing to do with the multiple simultaneous navigation actions we are talking about. The "action" parameter to NavigationActions.navigate is to provide a navigation action to a child router. ie: If you have a StackNavigator with a TabNavigator in one item (StackNavigator({ FooTabs: TabNavigator({TabA:..., TabB: ...}) })), the action parameter lets you make it open on a tab other than the default one NavigationActions.navigate({ routeName: 'FooTabs', action: NavigationActions.navigate({ routeName: 'TabB' }) }).

We're talking about how to dispatch 2+ navigation actions to that same top-level StackNavigator in the same tick.

[svicalifornia]

Ah, yeah, you're right about the child actions.

All of these options appear to present various challenges or drawbacks. While we consider them, you may wish to implement a custom router. I have been using custom routers to change the state however I wish, which results in all of the changes being applied simultaneously.

See the example here: https://reactnavigation.org/docs/guides/redux

Along with my proposed code example change here: #1311.

You don't have to use Redux, though. I'm using MobX as described here: https://hackernoon.com/react-navigation-with-mobx-2064fcdaa25b

[dantman]

1313 I've fixed navigation.dispatch so it can be called multiple times per-tick.

Some way of waiting for a transition to finish before dispatching another navigation would also be nice, but I'll leave that out of this since #135 should cover that.

[dzpt]

Have you guys solve it? I want to dispatch two actions to two tabs a time

        const setParamsAction = NavigationActions.setParams(
            {
                params: { refresh: true },
                key: 'Main'
            }
        )
        this.props.navigation.dispatch(setParamsAction)

        const setParamsAction1 = NavigationActions.setParams(
            {
                params: { refresh: true },
                key: 'Profile'
            }
        )
        this.props.navigation.dispatch(setParamsAction1)

But didn't work.