[proposal] improve rendering for query without incremental data

[morrys]

#238

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

[morrys]

deepening the proposed solution for defer facebook/relay#3121 (comment)

prefer to change the logic using hasNext (spec-complaint) instead of is_final (FB specific):

const isIncremental = responses.some((x) => x != null && x.hasNext === true);
if (fetchHasReturned && isIncremental) {
    this.forceUpdate();
}

In this way, no code modification is needed to optimize the render in case an incremental query is not used but it could be a breaking change for those using it with other specifications

[morrys]

#146 (comment)

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]

I would like to consider releasing both this PR and the solution to the issue #233 (comment) 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]

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

[morrys]

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

[morrys]

released 8.0.0-rc.3, with a fix for #159, now i added the test for it

[alex-statsig]

@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]

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]

@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.