Make sure style does not regenerate between renders if it is the same.

[larkox]

Description

This PR deals with one performance issue generated by creating a new style object on every render.

Any re-rendering of the parent component may make this component to re-render, and it will generate a brand new style property. This calls the re-rendering from the whole tree below this. This can be specially troublesome with components like VirtualizedLists (which have right now performance problems of their own facebook/react-native#31327 ).

Changes

• Added a new property to store the last sent style of the component.
• Added checks to see if the new resulting style has changed from the last style sent.
• Added another check to make sure any removed variable from the style is also not present in the last sent style.

Test code and steps to reproduce

Flipper can be used to track the re-renders and the reasons. Any simple three level design should be able to reproduce this error. For example:

const Level3 = React.memo(({style}) => (<View style={style} />))
const Level2 = ({style}) => (<Level3 style={style} />)
const AnimatedLevel2 = createAnimatedComponent(View)
const Level1 = () => (<AnimatedLevel2 style={{height:100}} />)

If we re-render the Level1, Level3 should not be re-rendered.

Checklist

• Included code example that can be used to test this change
• Updated TS types
• Added TS types tests
• Added unit / integration tests
• Updated documentation
• Ensured that CI passes

[larkox]

Not sure if it is safe enough to test only for the lengths. The case that bothers me is:

inputStyle = {
  var1: 1,
  var2: 2,
  var3: undefined,
}
lastStyle = {
   var1: 1,
   var2: 2,
   var4: 5,
}

[AlexanderEggers]

@piaskowyk Would you consider this PR for the next release? I think the changes will be quite helpful to improve app performance.

[piaskowyk]

@AlexanderEggers I will try, but I see here a potential issue with the race condition of the update on the UI thread (mappers). I need to prepare some tests for this PR.

[terrysahaidak]

I would at least make it a for of loop with key in inputStyle so we won't have a loop (includes) for every loop iteration (every) for no reason.

[larkox]

I added some code to avoid the double loop.
We have to iterate through the lastStyle, since we already know all relevant information from the input style is present in lastStyle. What we have to make sure is that no extra information is being sent through lastStyle (no extra key in lastStyle that is not present on inputStyle).

I could populate the Set in the previous for, but not sure what is faster (initializing the set directly with an array, or adding one by one the elements).

[derekstavis]

Just stumbled upon this issue myself. This is generating significant re-renders on my app since I started using Layout Animations in various core components. I am happy to have found this PR so I can at least patch locally :)

[larkox]

@terrysahaidak Any news here?

[larkox]

@piaskowyk Any news regarding this PR?

[piaskowyk]

Hey, I see problem that you don't handle nested properties for example transform: [{}, {}]. Did you test what happens if a render is triggered during animation running?

[larkox]

After the changes on createAnimatedComponent, the changes on this PR no longer make sense. I am not sure if they are still needed (I think the style is being recreated in _filterNonAnimatedProps, but I would like to check this version working in flipper before trying to fix it).