Add navigation.carefullyGetParent() function

[brentvatne]

I think people tend to get confused about what state refers to on the navigation object. For example, when we emit focus and blur events for a tab, people get the state for their route but not the state for the navigator. Usually the state for the route doesn't change. I think there could be many cases where you might want to know the state of the parent navigator. For example, if I'm on tab C and someone just swiped over to tab B, then maybe I want to start fetching data required to render my tab. Another use case came to mind while reading over this: react-navigation/react-navigation#3554. I'd like to be able solve this issue with the following:

const modalStackConfig = {
  navigationOptions({ navigation }) {
    const { routeState, navigatorState } = navigation;

    if (routeState.key === navigatorState.routes[0].key) {
      return {
        headerLeft: (
          <Button title="X" onPress={() => navigation.goBack(null)} />
        ),
      };
    }
  },
};

export default StackNavigator(
  {
    Main: StackNavigator({
      MainScreen: Main,
    }),
    Modal: StackNavigator(
      {
        ModalMainScreen: ModalMain,
        ModalDetailScreen: ModalDetail,
      },
      modalStackConfig
    ),
  },
  {
    mode: 'modal',
    headerMode: 'none',
  }
);

If we have multiple modal stack navigators, we can just pass that config in and the back button appears wherever expected. It feels like the navigator state is information that we should have access to and I feel a bit handcuffed without it. In the above case I'd have to set the navigationOptions on the ModalMainScreen route, and probably would want to be explicit about the initialRouteName, which would be workable but I believe not quite as clean or intuitive.

We could make this change on master with this code in createNavigator.js:

const childNavigation = addNavigationHelpers({
  dispatch,
  get state() {
    console.warn('navigation.state is deprecated, please use navigation.routeState instead');
    return route;
  },
  routeState: route,
  navigatorState: state,
  addListener: getChildEventSubscriber(addListener, route.key),
});

I believe we'd need to change some other places for events. But before I embark on a formal RFC for this I wanted to get some thoughts.

• Why didn't we do this in the first place?
• Are there any implications around optimizing re-rendering we need to account for here?

[dantman]

I sympathize with wanting to make it clear what state is for, but I have two problems with this.

The state of the route is the most important thing to the route, and it's already pretty verbose to get something like params from it (this.props.navigation.state.params). I don't like the idea that we're going to rename the property for the most important type of state to make it longer to type and require refactoring of apps, just to differentiate it from less important types of state.

My second problem, is that we're bordering on ruining the performance of PureComponent. Previously it wasn't clear to me if navigation would ruin PureComponent's optimization entirely. But to my understanding by the code in NavigationContainer, there is a level of effort that React Navigation goes to to avoid creating a new navigation object when the state has not changed. And I realized that state only contains things relevant to the route.

In other words, currently in theory if you use PureComponent on a route then navigation will not cause a re-render() if there is no direct change to the state of the route (i.e. params, etc...). Doing a navigate/push/goBack action elsewhere dozens of routes deeper in the stack will not cause the component to re-render.

However if we add navigatorState to a route's props.navigation. Well, then every single update to the state of the Navigator will cause every single route in the navigator to re-render() in order to get the update to props.navigation.navigatorState. Even if it's utterly useless to every single one of them and they aren't even active to display anything. Without every single react-navigation user going and refactoring their app to use an explicit shouldComponentUpdate to ignore the parts of the state they don't care about, this will tank performance.

[satya164]

To access parent navigation, you could use something like this.props.navigation.getParent() and to access its state, this.props.navigation.getParent().state.

It'll also be nice to have this.props.navigation contain the methods to manipulate the state and this.props.route to have the route state. Then you can access params as this.props.route.params. But not sure what's the best way to get parent route here or maybe we don't call the parent a route since it's a navigator anyway. So this.props.navigation.getParent().state still makes sense.

🤔

[brentvatne]

all good points @dantman. i think @satya164's suggested alternative of making it a function should address most concerns. i'm not sure we want to go as far as enabling the possibility of people traversing their parents -- this.props.navigation.getParent().getParent() would be 😱. for actions we want to perform on it, that could be handled the modular action creator api and/or combining action creators with key params and pass in this.props.navigation.getParentState().key

[dantman]

I do think it would be reasonable to give the route access to the navigator's state. But not as a part of props.

this.props.navigation.getParent().state does eliminate the concerns about re-render() happening on every navigation. However, then you never get a re-render. So if you do care about one piece of data on the navigator, you don't have a way to know when the data you got previously has changed.

Instead since this is state that belongs to the navigator, not to the route, why don't we consider this a type of context rather than a type of prop. And give a context style API to access it.

One option which would be familiar to the community would be to add an event to the event emitter. Whenever the state of the navigator changes we'd emit an event and the route could choose to subscribe to that information and only set state based off data they care about.

Alternatively, as I kind of prefer, we could treat this similar to actual context. React 16.3 is getting a new context api to replace the old unstable one. They chose to use a render prop for this, because render props are simple and you can also implement HOCs or other APIs using the render prop.

So I'd recommend implementing navigator state as a render prop. And then using it to implement a HOC/decorator.

<NavigatorState>{(navigatorState) => <RenderSomethingUsingNavigatorState navigatorState={navigatorState}</NavigatorState>

Personally I'd turn it into a redux style decorator to map navigator state to data I actually care about. Which would be extremely optimized and automatically re-render whenever state I care about actually changes.

@navigatorState((navigatorState, routeState) => ({
  isActiveTabNearby: calculateDistanceOfActiveTabFromCurrentTab(navigatorState, routeState.key) < 1,
})
const MyScreen extends PureComponent {
  render() {
    const {navigation, isActiveTabNearby} = this.props;
    const {id} = navigation.state.params;
    // ...
  }
} 

My props would not change on every navigate. But isActiveTabNearby would change when you navigate from a route 2 tabs a away to a route 1 tab away. And once more if you leave.

Or maybe I'd create an alternative type of optimized render-prop like I created yesterday for a different project. Which would allow re-render to only happen where I actually need it.

const MyScreen extends PureComponent {
  render() {
    const routeState = this.props.navigation.state;

    return {
      <View>
        // ...
          <MyNavigatorState
            get={navigatorState => navigatorState.index}
            render={index => <Text>Current index is: {index}</Text>} />
        // ...
      </View>
    };
  }

[satya164]

Maybe this.props.navigation.getParentState() + this.props.navigation.addListener('parentState') will solve the issues.

API with context also sounds nice, but it'll probably just be a HOC based API. It's a pain to use the new context API when you need to do stuff outside of render, where I put most of my methods. Keeping that in mind, it doesn't really matter if it's context based or not, since the HOC can be built on top of both.

[brentvatne]

i like the listener idea because using that and getParentState you could build the same api that @dantman proposed by creating a HOC with render prop or child render fn. what do you think @dantman?

[dantman]

@brentvatne I thought that too. But then realized that would only work for a HOC around the route. My later example wouldn't work for a render prop not wrapping the route unless you explicitly passed navigate to it.

Because you can only use getParentState if you have access to navigate. But only the Route or a component wrapped around the Route has access to navigate.

<MyNavigatorState
  navigation={navigation}
  get={navigatorState => navigatorState.index}
  render={index => <Text>Current index is: {index}</Text>} />

[dantman]

@satya164 Where are you suggesting this.props.navigation.getParentState() be used separate from the listener?

I'm assuming you are thinking of the common pattern. Where you use this.setState({...}) to set an initial value in componentWillMount and in either WillMount or DidMount registering the listener.

I just thought I should point out that React is deprecating and removing componentWillMount along with componentWillReceiveProps. They're adding a static getDerivedStateFromProps to replace them.

[satya164]

But then realized that would only work for a HOC around the route

navigation is already on context, you can get it with withNavigation, so it can work anywhere

Where are you suggesting this.props.navigation.getParentState() be used separate from the listener

What do you mean by that?

React is deprecating and removing componentWillMount

componentWillMount is basically constructor, so I don't think it affects anything

[brentvatne]

react-navigation/react-navigation#779 (comment) related

[brentvatne]

discussed further with @ericvicenti and we realized that since we are talking about escape hatches here (the alternative being to create a navigator that exposes the functionality requiring inspecting parent state) we should just expose getParent() and people can do getParent.addListener('action', () => {/* */}) to listen to state changes. we might want to name it something a little less inviting than getParent() so as to not encourage people to getParent().getParent() etc, which we know from building larger apps can cause frustrating bugs when screens and nav structure shuffle around