Truncate @stream arrays only on last chunk by default#13069
Truncate @stream arrays only on last chunk by default#13069phryneas merged 59 commits intorelease-4.1from
@stream arrays only on last chunk by default#13069Conversation
…ils to field function options
|
commit: |
jerelmiller
force-pushed
the
jerel/refetch-maintain
branch
from
b26c7ba to
9b209ac
Compare
jerelmiller
changed the title
@stream arrays only on last chunk by default@stream arrays only on last chunk by default
| ) => mergeObjects(existing, incoming); | ||
| const mergeFalseFn: FieldMergeFunction<any> = (_, incoming) => incoming; | ||
|
|
||
| export const defaultStreamFieldMergeFn: FieldMergeFunction<Array<any>> = ( |
There was a problem hiding this comment.
I'm doing the work to combine/truncate the array in a default merge function instead of the incremental handler because it makes it much easier to implement a custom merge function that takes the truncated array immediately (i.e. you just need (_, incoming) => incoming). This ensures incoming is set to the array emitted from the server in a @stream field rather than the combination of the cached array and streamed field.
|
@phryneas I think I have this in a pretty good spot. I've added comments to both highlight some areas of code with an explainer or to get your opinion. Feel free to make tweaks as you see fit so we can get this out and release an |
| private extensions: Record<string, any> = {}; | ||
| private pending = new Map<string, GraphQL17Alpha9Handler.PendingResult>(); | ||
| private streamDetails = new Trie<{ current: Incremental.StreamFieldDetails }>( | ||
| true, |
There was a problem hiding this comment.
| true, | |
| false, |
Given that path is (string|number)[], this can never be weak, so let's keep it as an explicitly strong Trie from the start.
src/cache/inmemory/policies.ts
Outdated
| * This field is only available in `merge` functions when the `@stream` | ||
| * directive is used on the field. | ||
| */ | ||
| streamFieldDetails?: Incremental.StreamFieldDetails; |
There was a problem hiding this comment.
I prefer streamFieldInfo, renaming :)
| fieldName: field.name.value, | ||
| field, | ||
| variables: context.variables, | ||
| path, |
There was a problem hiding this comment.
I'd say we can add it in the future. It might be useful, but it might also be a foot gun. Let's not do this too quickly without good use cases.
| }) | ||
| ); | ||
| // friend:2 ["one"] | ||
| expect(merge).toHaveBeenNthCalledWith( |
There was a problem hiding this comment.
I like the idea, but in the spirit of getting this published today, I'm not going to add it now. We should look at this again in the new year, though.
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to release-4.1, this PR will be updated.⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ `release-4.1` is currently in **pre mode** so this branch has prereleases rather than normal releases. If you want to exit prereleases, run `changeset pre exit` on `release-4.1`.⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ # Releases ## @apollo/[email protected] ### Patch Changes - [#12884](#12884) [`d329790`](d329790) Thanks [@phryneas](https://github.com/phryneas)! - Ensure that `PreloadedQueryRef` instances are unsubscribed when garbage collected - [#13069](#13069) [`9cad04a`](9cad04a) Thanks [@jerelmiller](https://github.com/jerelmiller)! - Truncate @stream arrays only on last chunk by default Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Finish rename of some variables from `streamDetails` to `streamInfo` from #13069 for consistency --------- Co-authored-by: jerelmiller <[email protected]>
2 participants
Updates the default
@streambehavior to maintain existing cache data as much as possible. Currently the@streamarray is truncated when the first chunk arrives which means you might lose data on screen suddenly while the data is fetched. We want to maintain the truncation, but only when the server completes the stream of data.