relay-tools / relay-hooks

Use Relay as React hooks

https://relay-tools.github.io/relay-hooks/docs/relay-hooks.html

MIT License

542 stars 56 forks source link

[proposal] improve rendering for query without incremental data #241

Closed morrys closed 1 year ago

morrys commented 1 year ago

238

to adapt the check to this PR and defer https://github.com/facebook/relay/pull/3121#issue-652496136 https://github.com/facebook/relay/blob/9ecd6195d83c3f9127067d4c4199979ad4e5f66e/packages/relay-runtime/store/OperationExecutor.js#L1690

morrys commented 1 year ago

https://github.com/relay-tools/relay-hooks/issues/146#issuecomment-764923422

In version 4.0.0 for a more complete management of the query the component is rendered 3 times:
data null, isLoading to true
data provided, isLoading a true (waiting for possible incremental data)
data provided, isLoading a false (the query is resolved)

with this PR, renderer change:

when the response doesn't have the hasNext field set to true

data null, isLoading to true
data provided, isLoading a false (the query is resolved) (the re-render is called in onComplete function)

when the responsehave the hasNext field set to true, so the response waiting

data null, isLoading to true
data provided, isLoading a true (waiting for possible incremental data) (the re-render is called in onNext function)
data provided, isLoading a false (the query is resolved) (the re-render is called in onComplete function)

morrys commented 1 year ago

I would like to consider releasing both this PR and the solution to the issue https://github.com/relay-tools/relay-hooks/issues/233#issuecomment-1496417927 with version 8.0.0.

@Lalitha-Iyer @alex-statsig Could you help me to test it also in your applications? tomorrow I may release version 8.0.0-rc1 to make testing easier for you

morrys commented 1 year ago

@Lalitha-Iyer @alex-statsig released 8.0.0-rc.1 with https://github.com/relay-tools/relay-hooks/issues/233#issuecomment-1496417927 and this PR.

morrys commented 1 year ago

released 8.0.0-rc.2, with a small fix for incremental data

morrys commented 1 year ago

released 8.0.0-rc.3, with a fix for https://github.com/relay-tools/relay-hooks/issues/159, now i added the test for it

alex-statsig commented 1 year ago

@Lalitha-Iyer @alex-statsig released 8.0.0-rc.1 with #233 (comment) and this PR.

Looks like the warning is fixed, and it no longer spams queries during the suspense. It does still seem to send the query twice (one time on initial mount, one time when the suspense finishes), but that's probably semi-expected with "store-and-network".

Lalitha-Iyer commented 1 year ago

Hi, @morrys thanks for the proposal and for implementing this. I verified when data is null, we now render only two times ( data=null & isLoading=true, data provided & isLoading=false ) 👍

However, when some fields are already cached, and with UNSTABLE_renderPolicy: 'partial', I still see an extra render. ( data=partialData & isLoading=true, data=completeData & isLoading=true, data=completeData & isLoading=false). Is this expected?

morrys commented 1 year ago

@Lalitha-Iyer @alex-statsig released version 8.0.0 https://github.com/relay-tools/relay-hooks/releases/tag/v8.0.0

Rendering is optimized when the query does not fetch data from the store with the store-and-network policy.

From a first analysis, the third render is essential to avoid possible update problems that do not derive from the execution of the query which is still pending.

To be evaluated if it is possible, without modifying the relay-runtime, to identify that the subscription was triggered by the query response.