documentation updates regarding @defer
#12814
Conversation
|
commit: |
size-limit report 📦
|
🛠️ Docs preview building...The preview is currently being built. Build ID: 55d4a3ff7b8278e012687a1b |
|
!docs set-base-branch main |
|
!docs set-base-branch main |
…hen using `link.simulateResult`
jerelmiller
left a comment
jerelmiller
left a comment
There was a problem hiding this comment.
Had a few comments for you. Most of it is looking good though!
docs/source/data/defer.mdx
Outdated
| }); | ||
|
|
||
| if (loading) return "Loading..."; | ||
| if (dataState === "empty") return "Loading..."; |
There was a problem hiding this comment.
This won't work if the query returns an error on the first chunk since errorPolicy: 'none' sets data to undefined (so dataState will be set to empty). We probably want the conditional that checks error first before we check the dataState.
I definitely agree not using loading here makes sense since its still true while everything in streamed in. Could we update this example to instead show a small spinner to demonstrate that its still loading, but we can start rendering data? Would it make sense to use dataState for that or networkStatus === NetworkStatus.streaming?
There was a problem hiding this comment.
Hmm, what about this?
Moving the next line up, but with a networkStatus error check. The error property will stay present e.g. on a refetch until data arrives, so only checking error would mask the loading/refetching state.
| if (dataState === "empty") return "Loading..."; | |
| if (networkStatus === NetworkStatus.error) return `Error! ${error.message}`; | |
| if (dataState === "empty") return "Loading..."; |
There was a problem hiding this comment.
The
errorproperty will stay present e.g. on a refetch until data arrives
I'm not sure that's true. See this test:
apollo-client/src/core/__tests__/ObservableQuery.ts
Lines 915 to 994 in a832111
You'll see the first emitted value after the error is the loading state without the error property set (source):
await expect(stream).toEmitTypedValue({
data: dataOne,
dataState: "complete",
loading: true,
networkStatus: NetworkStatus.refetch,
partial: false,
});Also verified with this useQuery test. error should only stick around if notifyOnNetworkStatusChange is false since the loading state won't be emitted to clear out the error.
I think we're safe to stick with just checking error for now. Since the rest of our docs use error pretty generally, I think it makes sense to keep this here. Maybe we kick the can down the road a bit further? It might be a good idea in general to use networkStatus more commonly anyways, but that is outside the scope of this PR.
docs/source/data/directives.mdx
Outdated
|
|
||
| <Note> | ||
|
|
||
| In order to use `@defer` directive, you need to choose an incremental delivery format handler . See [choosing an `incrementalHandler`](./defer#setting-up-defer-support-by-choosing-an-incrementalhandler). |
There was a problem hiding this comment.
| In order to use `@defer` directive, you need to choose an incremental delivery format handler . See [choosing an `incrementalHandler`](./defer#setting-up-defer-support-by-choosing-an-incrementalhandler). | |
| To use the `@defer` directive, you need to choose an incremental delivery format handler. See [choosing an `incrementalHandler`](./defer#setting-up-defer-support-by-choosing-an-incrementalhandler). |
I like the AI suggestion here
Co-authored-by: Jerel Miller <jerelmiller@gmail.com>
Co-authored-by: Jerel Miller <jerelmiller@gmail.com>
jerelmiller
left a comment
jerelmiller
left a comment
There was a problem hiding this comment.
Had a couple minor suggestions after reading it through once more, but nothing major. I think the AI review pointed out an error so check that out as well.
Co-authored-by: Jerel Miller <jerelmiller@gmail.com>
Co-authored-by: Jerel Miller <jerelmiller@gmail.com>
❌ Docs preview failedThe preview failed to build. Build ID: 6c31692c55acb3b261f64858 Errorsreact/api/link/apollo-link-base-batch-http
|
!docs set-base-branch main