relay-tools / relay-hooks

Use Relay as React hooks
https://relay-tools.github.io/relay-hooks/docs/relay-hooks.html
MIT License
540 stars 56 forks source link

useQuery triggers too many re-renders #146

Closed wholenews closed 3 years ago

wholenews commented 3 years ago

My component is rendering five times with useQuery and only once with useQuery commented out (the useQuery parameters are the same, e.g. reference equality). The performance is noticeably impacted. I would like to see only two renders: one with data null, and one with data provided. I wonder if it has something to do with querying lots of fragments in the same query? My previous implementation used multiple QueryRenderer and achieved good performance. I guess now I'll test separate queries with useQuery.

morrys commented 3 years ago

Hi @wholenews, the number of fragments present in the query does not affect the number of renderers caused by the useQuery.

In version 3.7.0 the useQuery makes only two renders as you described it.

In version 4.0.0 for a more complete management of the query the component is rendered 3 times:

It is possible to verify these behaviors also in the tests or in the example projects present in the repository.

To see the cause of the rerenders it is necessary to investigate the component tree.

For curiosity, what version of relay-hooks are you using?

wholenews commented 3 years ago

Hi @morrys, thanks for the quick response and really great package!!

I'm using 4.0.0 and I misattributed two of those renders, (which yes were caused by parent component renders). Now it's three renders as expected. After downgrading to 3.7.0 I only observe two renders!

So I think everything is indeed expected.

morrys commented 3 years ago

@wholenews perfect! I would love to have some feedback on how you feel with version 4.0.0 💯

Lalitha-Iyer commented 1 year ago

Hi @morrys, thanks for explaining the behavior in detail. One follow-up question, is there a way to opt out of partial/incremental rendering? I would prefer having 2 re-renders.

morrys commented 1 year ago

@Lalitha-Iyer There is no way to change this behavior without analyzing and implementing this request.

morrys commented 1 year ago

@Lalitha-Iyer you could try commenting out this line locally: https://github.com/relay-tools/relay-hooks/blob/master/src/QueryFetcher.ts#L134

Lalitha-Iyer commented 1 year ago

Thanks that worked well. If I understand correctly there are 2 scenarios when you can get incremental data 1) when there is partially cached data - For this today we can use renderPolicy to control if partial data should cause a re-render https://github.com/relay-tools/relay-hooks/blob/master/src/FetchResolver.ts#L134 2) when there's a GraphQL "subscription" request, in this case onNext is not optional.

I am wondering if the fetchQuery onNext here is necessary, since this is not a subscription request. This is the one that is causing the second render with data and loading=true. https://github.com/relay-tools/relay-hooks/blob/master/src/FetchResolver.ts#L174

morrys commented 1 year ago

surely there is the scenario of polled queries that never call complete.

It would also be necessary to verify the use of @defer @stream directives (it may not be useful since we are already subscribed to the store)

Lalitha-Iyer commented 1 year ago

Thanks, I hadn't thought about those. I am thinking we could either infer when to re-render onNext ( for example if poll cacheConfig is set), or provide an explicit option to turn off re-renders onNext. This feature would be hugely useful for improving overall performance.