screenIsActive prop / componentDidFocus event for TabNavigator items about react-navigation HOT 208 CLOSED

It probably makes sense to add lifecycle hooks to screens. So on your screen component you could declare:

• componentWillFocus - this happens after component did mount, before the screen starts animating in
• componentDidFocus - screen is done animating in
• componentWillBlur - screen is about to animate out
• componentDidBlur - screen is done animating out

What do people think about that?

from react-navigation.

Use componentDidUpdate and compare the active route on navigation.

from react-navigation.

Quick, ready to use solution using HOC to expose props.isFocused. No Redux needed.

Installation
Install the latest version of react-navigation-is-focused-hoc from npm

yarn add react-navigation-is-focused-hoc

or

npm install --save react-navigation-is-focused-hoc

USAGE EXAMPLE

app.js

import React from 'react'
import { StackNavigator } from 'react-navigation'
import { updateFocus } from 'react-navigation-is-focused-hoc'

import MyScreenView from './screens/myScreenView'

// navigation
const AppNavigator = StackNavigator({
  MyScreenView: { screen: MyScreenView },
}, {
  initialRouteName: 'MyScreenView',
})

export default class App extends React.Component {

  render() {
    return (
      <AppNavigator
        onNavigationStateChange={(prevState, currentState) => {
          updateFocus(currentState)
        }}
      />
    )
  }
}

myScreenView.js

import React from 'react'
import {
  View,
  Text,
} from 'react-native'
import { withNavigationFocus } from 'react-navigation-is-focused-hoc'

class MyScreenView extends React.Component {

  render() {
    return (
      <View>
        {this.props.isFocused
          ? <Text>I am focused</Text>
          : <Text>I am not focused</Text>
        }
      </View>
    )
  }
}

// second argument is the route name specified during StackNavigator initialization.
export default withNavigationFocus(MyScreenView, 'MyScreenView')

from react-navigation.

https://reactnavigation.org/docs/navigators/navigation-prop

from react-navigation.

I'd like just to chime in here coming from extensive usage of React Router v4 where they explicitly backed away from lifecycle hooks. If we can at all avoid adding lifecycle hooks, I'm all for it. These lifecycle hooks add complexity and, IMHO, are not part of React's basic philosophy. Not that my opinion really matters ;-)

Why not use what we already have -- props to signal a component's current state? Can't the usual lifecycle hooks cover most or all cases at that point? I might be missing some stuff here.

@satya164 can you show me a quick example or provide more detail of the creating vs rendering concern here?

from react-navigation.

I know it's probably going to happen but just wanted to add my +1 for lifecycle hooks

from react-navigation.

class SettingsScreen extends React.Component {
  render () {
    console.log('state: ', this.props.navigation.state)
    return (
      <Text>
        {JSON.stringify(this.props, null, 2)}
      </Text>
    )
  }
}

will log:
state: Object {key: "Settings", routeName: "Settings"}
even if the currentTab is Home or Chat

from react-navigation.

@JohannesKaufmann have you checked the tab navigator docs? there's an option called lazyLoad which does exactly what you want https://reactnavigation.org/docs/navigators/tab

from react-navigation.

@mkonicek withNavigationFocus is just an HOC that pass focused prop to the wrapped component so that the component doesn't have to deal with imperative API.

function MyComponent(props) {
   console.log(props.focused); // check if I'm focused
}

export default withNavigationFocus(MyComponent);

It's a much better API than lifecycles or events and the user is explicit when he wants to receive the prop, so we don't accidentally re-render things unnecessarily.

Also user can use it on any component down the children to know focus status as opposed to only on the direct child, similar ti withNavigation HOC.

from react-navigation.

Also, in addition to these hooks, we can add an HOC withNavigationFocus or something, which uses these hooks to pass a focused prop to the component, so that the components can use it declaratively and don't need to use the imperative API.

from react-navigation.

FWIW if anyone is using redux, you can do:

function getCurrentRoute (state) {
    if (state.routes) {
        return getCurrentRoute(state.routes[state.index]);
    }
    return state;
}

const navReducer = (state, action) => {
    const newState = MainNavigator.router.getStateForAction(action, state);
    newState.currentRoute =  getCurrentRoute(newState).routeName;
    return newState;
};

And then wrap your containers in an HoC that checks the navigation prop routeName vs currentRoute in the navReducer. This way you can implement your own componentIsActive/componentIsHidden lifecycle methods. It's not great but works for me until something more official is fleshed out.

from react-navigation.

Hey guys,

Any progress on this?
Thanks,

+1

from react-navigation.

I think following will be a better API,

navigation.addListener('animateIn', doStuff);
navigation.addListener('animateOut', doStuff);
navigation.addListener('focus', doStuff);
navigation.addListener('blur', doStuff);

Lifecycle hooks will break when using HOCs and you always need to be the direct child of the navigator to use the lifecycle hooks.

Events will work everywhere, even inside the components of navigationOptions, and with withNavigation HOC.

from react-navigation.

Unfortunately I think this API won't be very easy to use, because you'll need to re-subscribe every time the navigation prop changes, by listening to lifecycle hooks. Also you'll need to unsubscribe.

So I'm afraid the developer experience is worse this way, but I can still be convinced otherwise. Maybe you can post both examples so we can see them side-by-side?

I do agree the HOC problem is real, but I think there are ways to work around it.

from react-navigation.

@ericvicenti I think those might be useful. since the only thing wrong about them is the names don't match what it does, how about,

componentWillAnimateIn
componentWillAnimateOut
componentDidFocus
componentDidBlur

from react-navigation.

Quick and dirty hack for those who don't want to use redux:

<StackRouter
  onNavigationStateChange={(prevState, currentState) => {
    const getCurrentRouteName = (navigationState) => {
      if (!navigationState) return null;
      const route = navigationState.routes[navigationState.index];
      if (route.routes) return getCurrentRouteName(route);
      return route.routeName;
    };
    global.currentRoute = getCurrentRouteName(currentState);
  }}
/>

Then, simply use global.currentRoute wherever you need it.

from react-navigation.

lazyLoad slows down the animation a lot

It won't if you don't to expensive work during the animation

componentDidMount() {
  InteractionManager.runAfterInteractions(() => this.setState({ ready: true });
}

render() {
  if (!this.state.ready) {
    return <PlaceholderWithIndicator />;
  }

  ...
}

from react-navigation.

@hilkeheremans I do appreciate that lifecycle hooks should be used sparingly. We got this far and we haven't needed them, but there are often things in real-world apps that demand this use case. Just like most things with react, the declarative API is what you almost always want to use, but occasionally, imperative lifecycle hooks are needed. One of the greatest things about react is that it gives you a great declarative interface, but it also provides the component lifecycle methods for times when the declarative API is insufficient and you need an "escape hatch" to imperative programming. The handling of focus is one of these examples.

@satya164, the easiest way to handle the "viewWillAppear race condition is to skip that hook and only provide viewDidAppear and viewDidDisappear

from react-navigation.

Using @dmhood comment above I've written an HOC to give me a IsFocused prop. This relies on redux and setting up the navigation reducer as above.

NavigationIsFocused.js

import { connect } from 'react-redux';
import React, { Component } from 'react';

function withNavigationIsFocused(WrappedContainer) {

    return class extends React.Component {
        static navigationOptions = {
            ...WrappedContainer.navigationOptions
        };

        constructor(props) {
            super(props);
            this.state = {
                isFocused: false,
            }
        }

        _checkRoute = (props) => {
            if (this.props.navigation.state.routeName === props.currentRoute) {
                this.setState({isFocused:true});
            }
            else {
                this.setState({isFocused: false});
            }
        };

        componentDidMount() {
            this._checkRoute(this.props);
        }

        componentWillReceiveProps(newProps) {
            this._checkRoute(newProps);
        }

        render() {
            return <WrappedContainer isFocused={this.state.isFocused} {...this.props} />;
        }
    }
}

export function connectWithNavigationIsFocused(mapStateToProps, mapDispatchToProps, navigator) {
    return function (container) {

        let addNavigationMapStateToProps = function (state) {
            let map = mapStateToProps(state);
            return {
                ...map,
                currentRoute: state[navigator].currentRoute
            }
        };

        return connect(addNavigationMapStateToProps, mapDispatchToProps)(withNavigationIsFocused(container));
    }
}

Usage

import React, { Component } from 'react';

import {
    View,
    Text,
} from 'react-native';

import { connectWithNavigationIsFocused } from '../HOC/NavigationIsFocused'

class IsFocusedExample extends Component {

    static navigationOptions = {
        header: {
            visible: false,
        }
    };

    componentWillReceiveProps(newProps) {

        if (newProps.isFocused !== this.props.isFocused) {
            if (newProps.isFocused) {
                console.log("We are focused");
            }
            else {
                console.log("We are not focused");
            }
        }
    }
    render() {
        return (
            <View>
                <Text>This is a isFocused test</Text>
            </View>
        )
    }
}

function mapStateToProps(state) {
    return {};
}

function mapDispatchToProps(dispatch) {
    return {};
}

export default connectWithNavigationIsFocused(mapStateToProps, mapDispatchToProps, 'navReducer')(IsFocusedExample);

where the third parameter to connectWithNavigationIsFocused is the name of the navigation reducer.

Comments are welcome!

from react-navigation.

I kinda like the naming conventions used in ReactTransitionGroup

componentWillAppear()
componentDidAppear()
componentWillEnter()
componentDidEnter()
componentWillLeave()
componentDidLeave()

from react-navigation.

Hey guys,

Any progress on this?
Thanks,

It can be found here, but it's experimental: https://github.com/satya164/react-navigation-addons

from react-navigation.

lazyLoad slows down the animation a lot and you can't just activate it for one tab. And I would rather show a View with the correct background colour and a ActivityIndicator while the user is not viewing the tab so that the animation is smooth.

What about a higher order component that gives you more information about the current routing state? Or maybe some way to register to the Navigation Dispatch that you see in the console.

from react-navigation.

with lazyLoad: true

The first time i access the Tab Game it takes quite some time for the animation (i assume the animation starts as soon as the component is rendered?). The second time its fast as usual.

from react-navigation.

because you'll need to re-subscribe every time the navigation prop changes

I believe this can be easily solved. it's only a problem if the navigation prop keeps the listeners. But if the navigator keeps the listeners, then this won't be a problem. Even with lifecycle methods, the navigator needs to store a ref, so it's not much different.

So I'm afraid the developer experience is worse this way, but I can still be convinced otherwise.

It's just a little bit worse because you always have to explicitly un-subscribe. With life-cycle hooks, the developer experience is much worse when you're using HOCs such as Redux's connect. In most of my apps, most of the screens use connect.

Though the worse dev experience in events approach is easy to solve using a HOC we'll provide, which manages the subscription under the hood.

Personally I think most people should use withNavigationFocus HOC we provide instead of using this API directly. This API will act as a low-level API, and I think events provide much more flexibility than lifecycle hooks, which is nice for a low-level API. Also it's very easy to build a life-cycle hooks API with it if someone really wants it.

his would alleviate the HOC issue raised above

We cannot provide a focused prop to all screens because it'll trigger a re-render every-time it changes. Components should explicitly tell us that they want this prop. I think withNavigationFocus HOC is the way to go.

Lifecycle Hooks

Pros

• Easy to use, no need to manually subscribe and unsubscribe
• Can use withNavigationFocus HOC to wrap which passes a focused prop to get a declarative API

Cons

• Won't work when your screen is wrapped in HOC, can use withNavigationFocus HOC to workaround
• If a child component wants to know about focus state, you've to pass it down, re-rendering everything when state changes
• withNavigationFocus HOC only works for the screen component (we can do extra work to make it work for components down the tree by implementing a subscription system internally)

Events

Pros

• Works fine with HOCs
• Can pass down to child in tree without worrying about re-render, or use withNavigationFocus HOC deep in tree to know when screen is focused
• Can use HOC to wrap the component which passes a focused prop, which solves manual subscription woes, and get a declarative API
• withNavigationFocus HOC works for any component down the tree
• We can automatically cleanup listeners if the screen unmounts, or warn the developer
• A small DX improvement is that flow will work with it

Cons

• Developers have to explicitly subscribe and unsubscribe from Events, can use HOC to avoid manual work

from react-navigation.

Re-rendering a screen on focus change may not necessarily be that bad, if each screen is built responsibly with pure components

from react-navigation.

I believe there was conversation at some point but is there a potential to just use React lifecycle methods to accomplish all of these vs having to explicitly add this stuff?

from react-navigation.

Why we need invent something new? Maybe better take the solution from ReactRouterV4? We already have great hooks: componentDidMount, componentWillUnmount, ...

from react-navigation.

@janzenz cool, I've updated my answer accordingly.

from react-navigation.

this.props.navigation.state.routeName is the name of the rendered tab even if its not selected. And if you change the tab its not changed.

from react-navigation.

The API that React has for passing information from a parent to a child is props. If a child screen needs to know if it is focused, it should get an isFocused boolean prop.

I suspect that this is a reasonably rare use case and most screens don't want to re-render in that situation. Imperative methods are the standard way to do that.

@browniefed, we can support connect and most other HOCs by calling getWrappedInstance

@satya164, won't that leave a dangling handler and cause leaks?

from react-navigation.

@pmachowski you beat me to it! thanks again!

from react-navigation.

@pmachowski actually this worked better for me:

    static navigationOptions = (props) => {
      if (typeof WrappedComponent.navigationOptions === 'function') {
        return WrappedComponent.navigationOptions(props)
      }

      return { ...WrappedComponent.navigationOptions }
    }

from react-navigation.

@pmachowski Hero!!!

from react-navigation.

@mkonicek the hooks/events should be the low level API on which higher level abstractions like withNavigationFocus can be built. We cannot build it without the low level APIs.

Unless we make passing the prop the only API, of course, which would suck, because it'll re-render screens every time. It's particularly bad for things like swiping tabs since it'll introduce a jank when swiping.

from react-navigation.

One small thing about HOC - I don't think I've ever used them, they might be a very advanced concept for a typical developer. If we can make the API super easy to use without people having to thing about what a HOC is that might help.

from react-navigation.

@ericvicenti One simple example would be when a component inside a listview needs to know the focus state. For example, say a video/gif which starts playing when the component comes into view, or a card which wants to lazyload online user list,

It's pretty difficult to achieve this with lifecycle hooks/passing props. You cannot just do setState and a pass down the state as a prop to the child row, because ListView's rowHasChanged will prevent the component from updating, now you can do few things, both of which are tedious

• Add the property to row data by modifying original data array
• Keep a list of components which would need to know the focus state, and update rowHasChanged

Both are tedious. Also, you re-render listview even when only few of the components might have needed to re-render, which is pretty bad for perf.

Even without a listview, it'd be bad for perf to re-render components which don't need to re-render. Although usage is simpler.

This is a solvable issue, but the solution gets quite tedious for such a simple case.

Now, React's lifecycle hooks don't have such a problem, because they are pretty different from the hooks we're talking about here, which look similar, but behave very differently. React will call lifecycle hooks recursively down to the deepest child. So when you need to do side effects, child component can manage it by itself without having the parent component having to manage it. It's pretty different case here, where the parent always has to manage everything.

I think it may be fine for it to be explicit

Yeah, explicit is fine. The HOC doesn't make it non-explicit either, it just makes it easier. The problem here is that parent needs to manage everything. Child should be able to manage it's own behaviour.

from react-navigation.

The new ScrollView that Spencer is working on has better utilities for dealing with the lifecycle of a row and knowing when it is in view.

The current listview also allows me to know when something is in the viewport with onChangeVisibleRows

So I think this whole 'focus' situation is still an area that needs innovation and we should leave it up to the user on how deal with focus within their screen components.

Totally fine with leaving it upto the user. But why make it more tedious than it has to be?

I don't want to bikeshed on this anymore, but I just don't see why we want to go with lifecycles despite it's disadvantages,

• They are fine for simple use cases, but tedious to use when complex use cases
• They aren't really lifecycle hooks, React's lifecycle hooks work very differently
• They aren't flexible. I can't build things on top of them

Compared to events,

• Easy enough for simple use cases, even easier than classic events because the user doesn't have to manage the subscription, and stay easy for complex use cases too
• Very flexible, you can easily build an HOC which provides a prop, or calls lifecycle hooks, or anything
• Can be used for a lot more things in the library, for example an event on animation position change, event on tab press etc.

from react-navigation.

An additional thing to consider is that it would be helpful for this to work for nested navigation. For example, a stack navigator nested inside a tab navigator would probably like to know when a tab is switched and the stack navigator is no longer visible.

from react-navigation.

@dantman You mean rerendered, right?
I am pretty sure no matter how you optimize the app your HOCs componentWillMount, componentWillUnmount, render will be called at least once unless mounted at all.

from react-navigation.

@ericvicenti what are the arguments against events? the only con which is that the dev has to unsubscribe manually can be easily addressed since we know when screen unmounts, so we can cleanup the events.

from react-navigation.

I think idiomatic solution would be to add isFocused flag to navigation object, and then users would check for changes inside componentwillreceiveprops (like we always do). everything else seems to me like a overengineerig

As I said above, we cannot do that because it'll re-render screens everytime when their focused state changes.

And that's why I proposed withNavigationFocused HOC since this should be the API.

from react-navigation.

Re-rendering a screen on focus change may not necessarily be that bad, if each screen is built responsibly with pure components

@ericvicenti What if it's not built responsibly?

from react-navigation.

We either need these hooks, or props on navigation (isFocused), correct? When would I check these props to know I should send the network request? Would I do that in componentWillReceiveProps and compare isFocused?

Ideally you would do something like this,

class MyComponent extends React.Component {
  componentDidFocus() {
    this._fetchSomeData();
  }
  ...
}

Or, with events,

class MyComponent extends React.Component {
  componentDidMount() {
    this.props.navigation.addListener('focus', this._fetchSomeData);
  }
  ...
}

And if you want it to affect the rendered UI, you need to store it in state by calling this.setState, which the withNavigationFocus HOC aims to address.

from react-navigation.

Usually I've seen that it is fine to have the screen component be the owner of the focus and blur behavior. Do you have a few examples of when your deep-down components need to know about the screen's focus and blur? Depending on the use cases, I think it may be fine for it to be explicit.

from react-navigation.

@JohannesKaufmann @satya164 can confirm lazyLoad: true animation is janky in prod

from react-navigation.

Hey guys,

Any progress on this?
Thanks,

from react-navigation.

1. Performance.
   I don't know how React's rendering works in low level and how optimized it is, but I am worried that it can get pretty slow in performance when all my components' render() methods are nested like the following in multiple levels due to nested HOCs:

You're completely optimizing the wrong thing. If HOCs hurt the performance of your app then it's because you're not making proper use of PureComponent/shouldComponentUpdate in your components.

The fundamental way React works is that if the props/state haven't changed (according to your shouldComponentUpdate implementation) react doesn't even bother calling render().

Also a reminder, every single <SomeComponent /> in your render() is a function call to React.createElement(SomeComponent, props, ...) that creates a new ReactElement. There's a huge amount of new function calls and object creation that happens every single render() and is then just discarded. HOCs don't add too much to that. Calls and objects aren't too heavy in JS and React's pattern for optimization is to make it so that whole sections of your tree just don't get re-rendered when they don't need to be.

Also lifecycle methods feel weird to me. The built-in component* ones are handled internally by React and are tied in with the whole renderer which is responsible for creating the objects and even does this separately for every layer of a component and its HOCs. Rather than lifecycle methods I'd prefer events, which are also already a pattern than react-native makes use of all over.

from react-navigation.

@joonhocho

You don't need to use HOC with either APIs. It's just a possible convenience to have if you need.

There are probably a lot of other things which will cause perf bottlenecks before HOCs. But we're not telling you to use HOC. That's upto you.

Also HOC is not the primary API I proposed.

Lifecycles are great for actual lifecycle. They get called automatically by React recursively. Regardless of if you are using a HOC and regardless of where your component is in the tree. Adding these methods for navigation events provide the illusion of lifecycle methods, but they work nothing like them. They don't match how React works at all.

The whole point of the API I proposed was to have good compatibility with HOCs and third party libs, and remove unnecessary confusion because the original API doesn't work like lifecycle methods at all. The original API will break if you use an HOC which doesn't have a method to get the original ref. And it will also break if your component is not the top-most component in the screen.

from react-navigation.

@pmachowski Thanks, that's how I'm using your HOC already, and it did resolve the issue with the Camera continuing to scan QR codes after I'd switched screens.

I did some more debugging, and it looks like render() is being called multiple times on my screen/Component when I use navigate() to switch to the screen/Component, which is causing the sound I'm playing via react-native-sound when that Component renders to play multiple times. It looks like this is a different issue. Do you know how I can ensure that render() is only called once on a Component when its screen is invoked via navigate()?

from react-navigation.

but how do you get the active route?

from react-navigation.

@ericvicenti sounds good, though we need to think about it a bit more. this is probably clear for stack navigators, but

• what will happen for navigators that don't do animation?
• what if componentWillFocus fires, but componentDidFocus didn't (can happen for tabview when you're swiping fast), then should componentWillBlur fire?

from react-navigation.

We need to pass props to the TabView but we don't or has that changed?

from react-navigation.

@browniefed nope. I think we need to discuss it first, since it'd be weird to have some props passed when rendering, and some props passed when creating the navigator. I'll open an issue regarding that.

from react-navigation.

@satya164 can you show me a quick example or provide more detail of the creating vs rendering concern here?

Which concern are you talking about?

from react-navigation.

The first time i access the Tab Game it takes quite some time for the animation

Pretty sure this won't be noticeable when you're in production mode

from react-navigation.

@satya164 In __DEV__ = false mode its still noticeable so lazyLoad: true is not an option. (but I haven't tried building for production)

As I'm currently using wix/react-native-navigation with smooth animations this is something that is super important for me...

from react-navigation.

If it's noticeable in production mode, then it's something we need to fix.

As I'm currently using wix/react-native-navigation with smooth animations this is something that is super important for me...

Animation performance is sure important, regardless of which navigator you use. We use native animations where we can, so animation performance should be good, if you see animation is janky somewhere, please file an issue. Remember that this library is still in beta.

from react-navigation.

@satya164 do you have idea when something like this will be implemented?

from react-navigation.

@ivpusic feel free to send a PR to implement it

from react-navigation.

I'm not too convinced about the pure listener approach either. I like what @satya164 mentioned before: what if we implement BOTH the good old imperative AND a declarative approach, ie:

// lifecycle methods
componentWillAnimateIn
componentWillAnimateOut
componentDidFocus
componentDidBlur

// props on navigation (possible props:)
focused
isFocusing
isBlurring

This would alleviate the HOC issue raised above, wouldn't it? Not saying this is a great idea, just throwing it in here to get to better solutions.

from react-navigation.

@browniefed do you have a link?

from react-navigation.

Just @hilkeheremans comment above actually. RRv4 removed them in favor of just using React life cycle methods.

from react-navigation.

@browniefed RR4 removed custom lifecycle methods in favor of React's lifecycle methods. We're talking about adding custom lifecycle methods.

from react-navigation.

I am in favor of lifecycle hooks- it is what we do internally at FB, and it is very comparable to React's component lifecycle API. The tradeoffs seem very minor to me.

There must be a way to get lifecycle hooks to work with flow. @skevy, do you know how to do that?

from react-navigation.

They also have to re-subscribe when the navigation prop changes, right?

I like lifecycle hooks because it matches React's API, it matches our internal hybrid navigation code, and because the ergonomics are generally cleaner. Sure HOCs can be a pain, but the HOC authors can deal with it. I would rather than place the burden on the HOC developers, rather than the developers of each screen.

from react-navigation.

What about libs that have nothing to do with React Native? How exactly is the HOC going to deal with it?
Exporting a simple redux connected Component will now not be able to use navigation life cycle methods?

from react-navigation.

They also have to re-subscribe when the navigation prop changes, right?

We can store the callbacks the same way we store the component ref, in the navigator. So they don't get cleared. So user only subscribes once, and doesn't even need to unsubscribe.

Even if they were stored on the navigation object, the library can take care of re-subscribing. Users don't need to.

it matches React's API, it matches our internal hybrid navigation code

Familiarity is good, but people are familiar with events also. I think it's not a strong enough reason given that all the cons with this approach. If we're going with lifecycle approach, I think we first need to come up with good solutions for the cons.

but the HOC authors can deal with it.

I don't think it's fair to offload this to HOC authors. Correct me if I'm wrong, but I don't think if I'm writing a generic HOC I'll be able to address it in anyway without having knowledge of React Navigation. React Redux shouldn't have to know about React Navigation. This has to be fixed by users of the HOC. Which is not very nice.

from react-navigation.

This is the only thing users need to do, doesn't look that bad,

class MyComponent extends React.Component {
  componentDidMount() {
    this.props.navigation.addListener('focus', (status: boolean) => {
      // do something;
    });
  };

  render() {
    // render stuff
  }
}

from react-navigation.

The API that React has for passing information from a parent to a child is props. If a child screen needs to know if it is focused, it should get an isFocused boolean prop.

Yeah, that's why I suggested a withNavigationFocus HOC, regardless of which approach we go with.

won't that leave a dangling handler and cause leaks?

We know when a screen unmounts, we can clear listeners automatically.

we can support connect and most other HOCs by calling getWrappedInstance

Didn't know this was a thing. Though not all HOCs will have this.

from react-navigation.

@satya164 with all due respect but your proposal would confuse the hell out of me as a developer. Proper reasoning about the various lifetimes of objects in react is challenging as is. Adding automatically deregistering event listeners that then live on the navigator instead of the object you call addListener on is very confusing to say the least.

from react-navigation.

Adding automatically deregistering event listeners that then live on the navigator instead of the object you call addListener on is very confusing to say the least.

As a user, why do you need to care about where the library keeps the listeners? I can keep it in the closure, on a different object, and where-ever, why does it matter?

Regarding automatically removing the listener, when the screen is unmounted, it'll never get any new event. There's no the point in keeping the listener in memory. Of course you can unsubscribe manually if you want. But there's no reason the library should keep it internally. Again as a usr you don't need to care about it.

from react-navigation.

I think idiomatic solution would be to add isFocused flag to navigation object, and then users would check for changes inside componentwillreceiveprops (like we always do). everything else seems to me like a overengineerig

from react-navigation.

(late to the party :)) Yes lazyLoad is currently broken, see #214 (edit: I tried the react-navigation from npm which is very old, will try master). When fixed, I agree that's it's not powerful enough as it can't be (currently) enabled for one tab only.

from react-navigation.

Proper reasoning about the various lifetimes of objects in react is challenging as is.

Just a +1 on this. I still find the React components lifecycle confusing with all those events, thinking of in which order they'll get called for my component..

from react-navigation.

To summarize the long discussion, what's the leading proposal now? Is it:

componentWillAnimateIn
componentWillAnimateOut
componentDidFocus
componentDidBlur

We either need these hooks, or props on navigation (isFocused), correct? When would I check these props to know I should send the network request? Would I do that in componentWillReceiveProps and compare isFocused? Seems like a lot of work, probably I'd find componentWillAnimateIn easier to use.

I don't understand what withNavigationFocus is or why it's needed. An code snippet on how to use it would help.

from react-navigation.

Thanks for the explanation. Do I understand it correctly we need either:

1. Hooks:

class MyComponent extends React.Component {
  async componentWillFocus() {
    const data = await fetchData(...);
    this.setState({ data });
  }
  ...
}

Or:

2. isFocused and isFocusing props:

/*async*/ function MyComponent(props) {  // async won't work here :/
   // start the network request as soon as possible - when animation starts
   if (props.isFocusing) {
     const data = await fetchData(...);
     this.setState({ data });
   }
}

We don't need both 1 and 2, correct?

from react-navigation.

Unless we make passing the prop the only API, of course, which would suck, because it'll re-render screens every time. It's particularly bad for things like swiping tabs since it'll introduce a jank when swiping.

Ahh I get it about the re-rendering now, thanks a lot!

OK so looks like we'll want something like this, @ericvicenti does that work for you too?

class MyComponent extends React.Component {
  async componentWillFocus() {
    const data = await fetchData(...);
    this.setState({ data });
  }
  ...
}

from react-navigation.

@mkonicek Agree that people might not know what an HOC is, but most people already use it everyday, e.g. React Redux's connect HOC. I think it should be fine as long as it is documented.

from react-navigation.

OK, last basic question. Is it possible to make it work without people having to type the following in their code?

export default withNavigationFocus(MyComponent);

People would declare a screen as usual:

export default MyComponent;

And react-navigation would call the functions componentWillFocus etc. on the component if they exist.

from react-navigation.

Not sure if we already talked this through, but I'd like to see it implemented for stack navigator as well, according to request in #298

from react-navigation.

but I'd like to see it implemented for stack navigator

The title is misleading, but we're definitely not talking about adding these just for tabs. These will be for all navigators. Tabs, Stack, Drawer etc.

from react-navigation.

I agree with @mkonicek and I think we can do this without the HOC

from react-navigation.

@ericvicenti focused prop?

from react-navigation.

One problem with prop like focused(coming from exnavigation) is that everytime you pop a scene and the focus changes, the entire scene would re-render. it's painful for screens that doesn't care about focus to optimize their renders for focus. Refer: expo/ex-navigation#15.

from react-navigation.

if people really want to re-render their screen when the focus changes, then yes a withFocused HOC would make sense. But I think that use case is probably not very common. We may want to just implement the lifecycle hooks, and add the HOC if people file issues for it.

from react-navigation.

Makes sense. The lifecycle hooks provide the maximum kind of flexibility we need, and people who want the props can just use a HOC.

Once we all agree I wouldn't mind throwing a first PR at the agreed on solution, but having never implemented those before I could use a few rough pointers in the right direction there [i.e. good practice examples from other projects].

from react-navigation.

and people who want the props can just use a HOC.

I can't even build an HOC with just lifecycle hooks. I'm totally fine with this being in userland, but If I were to build this HOC, I have to wrap the screen component in an HOC, and then wrap the child which wants to know the state again. There's is no way for a child component to know the focus state without having to pass a prop all way down. Which is very inconvenient and expensive in a medium to large app.

from react-navigation.

Part of the issue it seems is that the list view is pretty tightly coupled to the concept of focus. As one example, you don't want to auto-play every video in the list, just the video that you are scrolled to. The tricky part here is the scroll view, because if there was only one video to start playing, then it would be pretty easy.

The new ScrollView that Spencer is working on has better utilities for dealing with the lifecycle of a row and knowing when it is in view.

So I think this whole 'focus' situation is still an area that needs innovation and we should leave it up to the user on how deal with focus within their screen components.

from react-navigation.

I think we should be cautious about prematurely abstracting the API, which often introduces complexity and doesn't add much benefit.

If you want to see an example of this, see the way the responders and interpolators are set up on CardStack 😆 That stuff was introduced by one of the core contributors to NavigationExperimental, and was meant to enhance easy of customization. But after just a year of code rot it already feels like strange extra complexity, because there is just one remaining interpolator/responder, and everything under CardStack is quite tightly coupled.

Sorry for the tangential rant, but this stuff builds up over time so I want to avoid adding features we think we will need but actually don't wind up being so valuable. Technically the component lifecycle hooks aren't even necessary, but this is a super common feature request and we have this in our internal hybrid apps at Fb. Anything beyond these basic hooks is something I'm likely to push back on, until we get another wave of issues requesting that feature.

from react-navigation.

not elegant, but here is a reasonable solution (without using a fully custom getStateForAction) if you are using redux:

newNavigatorState.routes = newNavigatorState.routes.map((route, i) => {
    const isActive = i === newNavigatorState.index
    route.params = route.params || {}
    if (route.params.active === isActive) return route

    return {
      ...route,
      params: {
        ...route.params,
        active: isActive,
      }
    }
  })

from react-navigation.

@ericvicenti @satya164

Just in case that API for this issue has not been completely settled yet, I just want to throw my opinions out there.
I'm personally not a fan of HOC approach in general. I know that many libraries out there implements HOC approach, and it's somewhat popular, but it gets pretty ugly when I have to do it with multiple libraries like so:

libA(libB(connect(MyComponent)))

There are few issues that grind my gears with HOC in general:

1. Performance.
   I don't know how React's rendering works in low level and how optimized it is, but I am worried that it can get pretty slow in performance when all my components' render() methods are nested like the following in multiple levels due to nested HOCs:

        render() {
            return <WrappedContainer isFocused={this.state.isFocused} {...this.props} />;
        }

For example, redux does this with its connect and it wraps almost all my components and I don't want other libraries following this step. Library authors, please stop this trend...

2. Static properties are not copied over unless done manually by HOC implementation.
   I've had many debugging sessions due to HOCs not copying over static methods/properties of the wrapped class.
   When you define some property to your component like so, MyComponent.myStaticValue = 1;, you lose this property when wrapped with HOC. Most HOC libraries don't copy over static properties, and I had to do work around them as some libraries work with static properties defined on classes. It's painful and not straightforward to debug sometimes.

3. Not only static properties, but access to member methods/properties are lost as well.
   For example, sometimes you do this with ref of your components.

onSomeEvent() {
  this.input.focus();
}

render() {
<MyInput ref={(ref) => this.input = ref} />;
}

This works when MyInput has focus method, but when wrapped by HOC, you can't do this anymore. Because they obviously don't copy them.

If possible, please avoid HOC approach. It creates many side effects and not necessary in most cases.

I personally prefer lifecycles as suggested by @MarkMurphy as they are declarative and is what react does with their component lifecycles. Plus, it doesn't mess with your component class. It leaves your classes as they are, clean and beautiful.

How about
screenDidBecomeActive(), screenDidBecomeInactive(),
or
screenDidEnterForeground(), screenDidEnterBackground()?

Adding these to all nested components will surely lead to performance issues, so I suggest only adding them to screen-level components. If nested components need them, then they may get those via redux or as props manually.

I think lifecycles are the cleanest and best approach as they match how react works with their component lifecycles. Also, you don't have to worry about adding and removing listeners yourself as they are done automatically by library.

Just my two cents.

from react-navigation.

@dantman

You're completely optimizing the wrong thing. If HOCs hurt the performance of your app then it's because you're not making proper use of PureComponent/shouldComponentUpdate in your components.

I am already using shouldComponentUpdate for all my components, but react-native is still somewhat slow compared to native app. Using PureComponent/shouldComponentUpdate for your components doesn't magically solve all your performance issues you can find in react native. Thus, being careful over how to design library API is important and is not completely optimizing the wrong thing, especially for react navigation, that is so fundamental and touches every aspect of your app.

Also a reminder, every single in your render() is a function call to React.createElement(SomeComponent, props, ...) that creates a new ReactElement. There's a huge amount of new function calls and object creation that happens every single render() and is then just discarded. HOCs don't add too much to that. Calls and objects aren't too heavy in JS and React's pattern for optimization is to make it so that whole sections of your tree just don't get re-rendered when they don't need to be.

In HOC pattern, HOCs render() does call <WrappedComponent ... />. So in your words, it's a huge amount of new function calls and object creation that happens every single render().
I just wanted to say that HOC patterns can sometimes be confusing and may have unwanted side effects if not implemented carefully.

I am totally fine with registering events. I am just against HOCs in general. that's all.

@satya164
I agree with you that events are more flexible and can work better with different libraries. Thanks for clarifying.

from react-navigation.

In HOC pattern, HOCs render() does call <WrappedComponent ... />. So in your words, it's a huge amount of new function calls and object creation that happens every single render().
I just wanted to say that HOC patterns can sometimes be confusing and may have unwanted side effects if not implemented carefully.

Each element is one call and one ReactElement. A HOC has one element, thus one call and one ReactElement. The "huge" calls I describe are due to the fact that a normal non-HOC component will have dozens of react elements in them for all the views, text, native UI, and custom components. Hence what a HOC does in its render is trivial in comparison.

And in that same paragraph:

Calls and objects aren't too heavy in JS and React's pattern for optimization is to make it so that whole sections of your tree just don't get re-rendered when they don't need to be.

from react-navigation.

Yes, it may be not that big of a deal at first site because it's only one element.
But, let's say that every library chooses to use HOC approach because it's only one call.
You may easily end up doing what I said before:

libA(libB(libC(connect(YourComponentThatIsInTheList))))

I am pretty sure this will have some impact in performance of your app if the component gets rendered 30 times per screen.
Don't forget that each of your HOC layer also has react lifecycles too as well as render(). That's a lot of unnecessary work that can be easily avoided by not using HOC.

from react-navigation.

@joonhocho Not if the app is properly optimized and props/state in the pure parent component using your component haven't changed, then 0% of those HOCs and your component will be rendered or receive any lifecycle calls.

from react-navigation.

but react-native is still somewhat slow compared to native app. Thus, being careful over how to design library API is important and is not completely optimizing the wrong thing

Whatever slowness you encounter with React Native is due to the overhead of the native bridge most of the time.

it's a huge amount of new function calls and object creation that happens every single render()

It's likely that you'll have bigger issues in the app if you encounter this at all. It's very unlikely that you'll have a huge number of navigation related HOCs in a single tree. In fact the number of Redux HOCs will be probably several times more than navigation related HOC.

I am just against HOCs in general. that's all.

I'd say it's premature optimization. Like avoiding extra function calls to optimize perf. Sure it'll improve perf, but how much? Always measure first.

Regardless, nor harm for a HOC being there for people who want to use it. It doesn't affect you if you don't use it.

The discussion here is about the API for focus detection. I only proposed additional HOC because it can be convenient. Let's decide whether to add HOC or not later. HOC isn't important to my proposal.

from react-navigation.

@pmachowski thanks for sharing. I tried it but I lost the ability to setup the navigationOptions, now it's blank.

from react-navigation.

@janzenz my bad. I've edited my post to fix it. Just update withNavigationFocus.js with the updated code.

from react-navigation.

@pmachowski, thank you so much for sharing your solution, it's the only thing that has allowed me to continue building my app with React Native (the camera was continuing to scan QR codes even after the app navigated away from the camera to a different screen).

Your solution works great if I rely exclusively on the back arrow in the navigation header. However, the app seems to get confused about which screen has focus if I change screens via another mechanism, e.g. a button press

<Button
onPress={() => navigate('ScanScreen')}
title="Continue"
/>

Is there anything I can add to navigate() calls to ensure they respect/use the focus mechanism the same way the navigation header does?

I'm extremely grateful for your solution. Does anyone know if there are plans to make it or something like it a part of react-native-navigation? If not, how are people suppressing components like the camera when they navigate to different screens of their apps?

from react-navigation.

@gititon I've the same use case (real time processing of camera feed).
Back button functionality does not need my HOC. When you press back the view is unloaded (destroyed) by React Navigation.

My HOC is useful when you use navigate() to navigate away from your ScanScreen. Make sure you have something like along the below lines in your ScanScreen

  render() {
    return (
      <View>
        { this.props.isFocused
          ? <Camera />
          : null
        }
      </View>
    )
  }

where <Camera /> is your camera component doing QR code detection. That way it will be unloaded properly.

from react-navigation.