[ResourceItem] Remove children from the equality check in shouldComponentUpdate

[chloerice]

WHY are these changes introduced?

Fixes #2164

The resource item component does a deep equality check of previous and next props in the shouldComponentUpdate lifecycle method. This causes significant performance issues in production because children are included in that check.

WHAT is this pull request doing?

The ideal way to fix this forward would be to rewrite ResourceItem as a functional component and 🔥shouldComponentUpdate entirely. Even more ideally, it shouldn't be handling its own selected state but instead rely on a selected boolean prop. But we don't have cycles for that effort right now.

This PR hot fixes the problem by stripping children out of the check to reduce complexity of the comparison down to only the explicit props.

• Keeping this a draft until I've tested in a production environment, as I need to verify whether the deep comparison remains laggy due to the fact that the optional media prop expects either an Avatar or a Thumbnail.

How to 🎩

To tophat this change, I made a release candidate and pushed the web upgrade branch to Web staging to enable performance analysis in a production environment.

	Before	After
Render time	5.2sec	2.2sec
Browser freezing	Yes	No
shouldComponentUpdate calls / highest call time	>1000 / 2.2ms	5 / 0.5ms

[github-actions]

🟢 This pull request modifies 2 files and might impact 1 other files.

Details:

All files potentially affected (total: 1)

• /src/components/ResourceList/ResourceList.tsx

📄 UNRELEASED.md (total: 0)

Files potentially affected (total: 0)

🧩 src/components/ResourceItem/ResourceItem.tsx (total: 1)

Files potentially affected (total: 1)

• /src/components/ResourceList/ResourceList.tsx

[jineshshah36]

Hi, we merged this change it broke a bunch of our ResourceItem components. I have put together a codesandbox here which exemplifies the problem.

Essentially, if you attempt to update the children of the resource item asynchronously without making any additional prop changes, it will never re-render. In the example I included, the author is "fetched" asynchronously, and when we attempt to re-render with the author info, it doesn't.

I believe excluding children is simply a cure for the symptom, but the real problem is that the code is using deep equals instead of shallow equals. Is there a reason for the deep equals? Shouldn't all props be immutable, therefore not need a deep equals comparison? It seems like you could solve all problems by just doing a shallow equals across all props? I would argue that if you need deep equals, it would imply that some of your props are not immutable.