Many useless re-renders

[cglacet]

Current behavior

The following code from the documentation will render each tab two times on init and one time on every tab change. This will be the case even if you use React.memo as suggested

import * as React from 'react';
import { View, useWindowDimensions } from 'react-native';
import { TabView, SceneMap } from 'react-native-tab-view';

const FirstRoute = React.memo(() => {
  console.log("Render route 3");
  return <View style={{ flex: 1, backgroundColor: '#ff4081' }} />
});

const SecondRoute = React.memo(() => {
  console.log("Render route 1");
  return <View style={{ flex: 1, backgroundColor: '#673ab7' }} />
});

const renderScene = SceneMap({
  first: FirstRoute,
  second: SecondRoute,
});

export default function TabViewExample() {
  const layout = useWindowDimensions();

  const [index, setIndex] = React.useState(0);
  const [routes] = React.useState([
    { key: 'first', title: 'First' },
    { key: 'second', title: 'Second' },
  ]);

  return (
    <TabView
      navigationState={{ index, routes }}
      renderScene={renderScene}
      onIndexChange={setIndex}
      initialLayout={{ width: layout.width }}
    />
  );
}

This means 2 things:

1) SceneMap doesn't memoize scenes based on the route they depends on. 2) The render function doesn't provide consistent props to scenes (otherwise FirstRoute and SecondRoute would never re-render as we never change the props on our side).

Workaround

I found a way to hack over this issue by extracting our props (only):

const SecondRoute = (renderProps) => {
  return <SecondRouteFix {...renderProps.route} />
}

const SecondRouteFix = React.memo((props: {title: string, key: string}) => {
  console.log("Render route 2");
  return <View style={{ flex: 1, backgroundColor: '#673ab7' }} />
});

Investigating further, I found that the renderProps actually contains what I think is the currently focused tab index as renderProps.position. This is what is causing the issue here.

I think the correct fix would simply be to remove this index (as the user is already asked to handle it by himself const [index, setIndex] = React.useState(0);). Since the user controls the index there no reason to inject if in the props (he already has it).

One other solution is to fix the documentation.

Issues with the documentation

The solutions proposed in the documentation do not work:

const renderScene = ({ route }) => {
  switch (route.key) {
    case 'home':
      return <HomeComponent />;
    default:
      return null;
  }
};

function HomeComponent() {
  return (
    <View style={styles.page}>
      <Avatar />
      <NewsFeed />
    </View>
  );
}

export default React.memo(HomeComponent);

If you use this, HomeComponent WILL re-render on every tab change. Only Avatar and NewsFeed will not. This is not only because of the React.memo but because the props are not propagated to these two components. If you were to write this:

const renderScene = (renderProps) => {
  switch (route.key) {
    case 'home':
      return <HomeComponent {...renderProps} />;
    default:
      return null;
  }
};

function HomeComponent(renderProps) {
  return (
    <View style={styles.page}>
      <Avatar {...renderProps}  />
      <NewsFeed {...renderProps} />
    </View>
  );
}

export default React.memo(HomeComponent);

Both Avatar and NewsFeed would re-render (because of the changing tab index). That's the reason why, when you use SceneMap you will need to both: 1) use React.memo on your scene, but also 2) remove injected props to only keep the route one.

Documentation fix

I think the documentation should Include an example of the component lifecycle to highlight the fact that props injected by the lib are changing over time (even if the user's props do not).

For now, the only reference to this problematic prop is not even clear "position: animated node which represents the current position". From what I saw, it's not a node it's a number, it's not an Animated.value as you might also expect from the "animated" bit. It's also not the current scene's index. I suspect its the current tab index, but I'm not sure, what I'm sure is "it changes when we modify the navigationState.index.

In the end I'm convinced that the better option is to guarantee the user that the injected props will remain stable over time as long as they are untouched on the user side. If the user requires the current tab index for some reason, he cas include it in the route props. (jumpTo is not a problem because its reference remain unchanged over the component lifecycle).

Expected behavior

Scenes should only re-render when the route it depends on change. All re-renders should be controled explicitely by the user. In other words, no injected prop should change unless the user changes the input routes props.

Reproduction

https://snack.expo.dev/PaH0-4H0O

Platform

• [X] Android
• [X] iOS
• [X] Web
• [ ] Windows
• [ ] MacOS

Environment

package	version
react-native-tab-view
react-native-pager-view
react-native
expo
node
npm or yarn

[github-actions[bot]]

Couldn't find version numbers for the following packages in the issue:

• react-native
• react-native-tab-view
• react-native-pager-view
• expo

Can you update the issue to include version numbers for those packages? The version numbers must match the format 1.2.3.

[okwasniewski]

@cglacet Thanks for reporting this issue, this should be fixed in the upcoming release. Once #1392 gets merged.