Add navigation.carefullyGetParent() function about rfcs

[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.

🤔

[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

[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.

[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.

[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