Relay: Regression: Wasteful rerender during Pagination

[cvle]

Regression: https://github.com/facebook/relay/issues/2562

Pagination Containers rerender every single item on every pagination.
I've verified the regression starting with version 5.0.0 up to the newest version 10.1.2.
Versions between 2.0.0-4.0.0 are working as expected.

I've upgraded the example at https://github.com/cvle/relay-wasted-render to the newest version 10.1.2. There is also a branch working that runs on 4.0.0 without said issue.

I have done some performance analysis and just realised that during a pagination using the createPaginationContainer there is a lot of wasteful rerender that impacts the performance the larger the list gets.

I have created a minimal example here: https://github.com/cvle/relay-wasted-render

You can see on the screenshot that whenever I load more, the whole list gets rerendered and I haven't found a way yet to optimise rendering using the shouldComponentUpdate approach. It seems like all fragment data has changed even though e.g. the me fragment has not been mutated / changed.

[cvle]

An oddity is, that every pagination triggers two render commits:

The first render looks correct, and actually only renders the new list item.

The second render commit that follows immediately renders everything again and is completely wasted.

[cvle]

Versions 2.0.0-4.0.0 only trigger one optimal render:

[cvle]

We have a huge performance penalty because of this, and no luck finding a workaround so far :-(

[josephsavona]

Thanks for reporting this and for the repro, this is super helpful for investigating. In general we have not seen this type of wasted re-rendering w PaginationContainer, so I do wonder if there is something particular about the setup that is causing this.

We'll prioritize looking at this in our next review of OSS issues. In the meantime, if anyone in the community is interested we would appreciate if anyone could investigate a bit and try to confirm what's happening here.

[cvle]

From my investigation so far:

The first state update comes from a regular subscription through RelayModernFragmentSpecResolver when the relay cache changes.

The second state update comes from the onNext handeling: https://github.com/facebook/relay/blob/5d555a4db0ed6e28cc0003cc5b37912cf2d294c1/packages/react-relay/ReactRelayPaginationContainer.js#L742 which sets variables and the request on the resolver, and takes the updated data.

In my cases it doesn't cause any data to change except for __fragmentOwner which changes every iteration.

First render:

After 1st pagination:

After 2nd pagination:

This would make sense that introducing __fragmentOwner caused this regression as this happened during 4.0.0 -> 5.0.0.

The question here is: Do I need the onNext handling at all? As the first state update seems to work already, what's the onNext handling for? (Is the __fragmentOwner supposed to change at all?)

Removing the onNext handler seems to resolve the issue for me.

[josephsavona]

Thanks for the detailed debugging, that makes sense. We'll have to see if there is a way to avoid this second render, ideally the onNext handling can be merged into the first update.