react-native-skeleton-content-nonexpo icon indicating copy to clipboard operation
react-native-skeleton-content-nonexpo copied to clipboard

WIP perf: add `PureSkeletonContent` component to improve performance by reducing re-rendering of component

Open mernxl opened this issue 4 years ago • 5 comments

This is a proof of concept as well as a final code to support this issue #28 i opened earlier.

As a guide, this section has been added to the README file.

PureSkeletonContent

If you are really concerned about performance, or you don't want your components mounting being runned while isLoading is false, then you may need to consider using PureSkeletonContent.

Point to note

All props passed to PureSkeletonContent should be a constant prop, memoize it if it will change sometime. Otherwise, you should consider using the good old SkeletonContent.

This point does not apply to componentProps as it is shallow checked to know if we should re-render the Skeleton or not.

import { FunctionComponent } from 'react';
import { Text, TextStyle, StyleSheet } from 'react-native';
import { PureSkeletonContent } from 'react-native-skeleton-content-nonexpo'; 

const Greetings: FunctionComponent<{ name: string }> = ({ name }) => 
  (<Text>Hello {name}</Text>);

const GreetingsSC = [{ height: 40, width: 200, paddingVertical: 2 }];

const SomeComponent: FunctionComponent<{name: string}> = ({ name }) => {
  const [loading, setLoading] = useState(true);

  return (
    <PureSkeletonContent 
      isLoading={isLoading}
      layout={GreetingsSC}
      component={Greetings} 
      componentProps={{ name }} // will be shallow checked, you don't need to memoize this
      containerStyle={styles.container} // notice we using styles from styleSheet
    />
  )
}

const styles = StyleSheet({
  container: {
    flex: 1, width: 300
  }
})

mernxl avatar Nov 19 '20 14:11 mernxl

@alexZajac @mernxl Is there an update here? Would be very interested in this feature. Thanks for your effort!

reichhartd avatar Dec 06 '21 11:12 reichhartd

Thanks for the PR! We would need the checks to pass for it to be merged!

alexZajac avatar Sep 05 '22 23:09 alexZajac

Hi, this MR will need some more review and probably some talks. If possible @alexZajac could you test on a local project? It's been a while I worked on such a project.

I am available to help drive this towards a positive direction.

My initial change was we use Animated from react-native and ditch reanimated. But then it's been a while, it's possible there's been some updates that fixes a lot. Let's just take the time to review and see how to get to a good point.

mernxl avatar Sep 16 '22 03:09 mernxl

Hi!

Yeah, this one slipped over my hands with the amount of work I had on the side, not sure if having a different component is the way to go here. My rationale is that if this one is "more performant" than the other SkeletonContent, then why don't we replace the implementation of this one instead? Maybe there is a functional difference I didn't catch there.

Thoughts?

alexZajac avatar Sep 18 '22 21:09 alexZajac

Actually replacing without deprecating the current will make the community unstable. It will take time to migrate, most people will move away from the library. I would say I actually use them both within my project, for huge components, I will go with Pure, when I have small things to display within, I go with the current.

The primary function of the PureSkeletonContent is to allow you to pass in the "child component", it will only be initialized when isLoading is false and destroyed when true. I think a performance gain for navigating to new screens actually, where it does not create a component when it's not needed yet.

The current SkeletonContent, takes in an already initialized "child component", and so sometimes you need to check isLoading here and there especially when the data to be displayed drives the isLoading variable.

mernxl avatar Sep 19 '22 02:09 mernxl