zobeirhamid / react-native-scrollable-navigation-bar

Respecting navigation bar for scrolling screens.
https://zobeirhamid.github.io/react-native-scrollable-navigation-bar/
MIT License
267 stars 29 forks source link

Fix state updates on unmounted components #16

Closed hirbod closed 4 years ago

hirbod commented 4 years ago

Fixes #11

@zobeirhamid So, HeaderNavigationBar and HeaderBorder register listeners on componentDidMount while in my case (after testing), componentWillUnmount triggers immediately. So the component gets created and destroyed instantly, but keeps adding listeners, which still call fire().

This is a a memory leak, as the listener array grows and grows but also throws warnings since the component is already unmounted. Memory usage of my app shrinked after my adjustments.

I added types and a removeListener function and some extra checks. On componentWillUnmount, I remove the listener. Since they've been passed as anonymous function, I stored it to this.listener and use them for listening and removing.

I just tested this for my case, using following code:

                <ScrollableNavigationBar
                    transitionPoint={180}
                    title="Profil"
                    StatusBar={() => (
                        <StatusBarComponent
                            barStyle="light-content"
                            backgroundColor="transparent"
                        />
                    )}
                    titleStyle={{color: 'white'}}
                    headerBackgroundColor="#b7cc23"
                    headerTitle={"Profil"}
                    showsVerticalScrollIndicator={true}
                    barStyle="light-content"
                    scrollIndicatorInsets={{
                        top: 135,
                        left: 0,
                        bottom: 0,
                        right: 0
                    }}
                    snapToInterval={84}
                    snapToAlignment={"start"}
                >

No side effects, no issues. It also looks like, that components get mounted/unmounted on scroll, so this handling should be fine now.

I would kindly ask you to review and test this on your branch, maybe against your demos and merge this + create a new release anytime soon. Thanks!