feat(react): useFragment hook (BETA) #3570

JoviDeCroock · 2024-04-22T19:03:37Z

Resolves #1408

Summary

This introduces a new hook to the React package that has two main purposes. On one hand it will mask over the data it gets from its parent component given a fragment, so in the following scenario:

query GetTodo { todo { ...TodoFields completed } }
fragment TodoFields on Todo { id text }

When we'd pass the resulting data from GetTodo.todo into a component and leverage useFragment we wouldn't be able to get to the completed parameter.

The second goal is to establish a loading paradigm for deferred queries, this means that given the following document

query {
  todo {
    id
    text
    author { ...AuthorFields @defer }
  }
}

fragment AuthorFields on Author {
  id
  name
}

When the initial payload comes in we can display the id and text, during that we display this query as stale however we can make this hook in deeper with React paradigms by leveraging suspense when the deferred boundary isn't complete and suspense is enabled we can suspend. Even without suspense this brings the benefit of not having to check stale and being able to render independently of the result.stale flag.

Notes

The heuristic used to assess whether we are using is the presence of undefined basically data can only be undefined when it has not been queried or while it's being streamed in.

I opted for a slightly different approach to useQuery, in useQuery we are subscribed to the request and when the resolved query comes in we call resolve on the promise we threw to React. In this one I opted to purely base it on props, I tested this in this demo.

I opted to create the request with our code we use for operations but replaced variables with data to ensure every entity gets its own cache entry.

A lacking piece at the moment would be directives, if a user leverages @include() with a variable then we'd need access to the parent variables to uphold our heuristic of when to set this to loading. To counter-act this we check whether the field is undefined and when it had one of these directives we accept the absence of the data.

In the future we'll need to find a way to resolve() the promise, this is currently impossible as we have no way of establishing a stable identifier across concurrent invocations of a fragment given unique sets of data.

Todo

support gql.tada FragmentOf dance for data
- Decision: unwrap with readFragment before using useFragment
verify whether TypedDocumentNode types work correctly
handle nested fragments in the FragmentDefinitionNode

changeset-bot · 2024-04-22T19:03:41Z

🦋 Changeset detected

Latest commit: 11727b6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
urql	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

kitten · 2024-04-22T19:12:42Z

packages/react-urql/src/hooks/useFragment.ts

+): { data: Data; fulfilled: boolean } => {
+  const maskedData = {};
+  let isDataComplete = true;
+  selectionSet.selections.forEach(selection => {


I think, this would need (named) fragment spread support in before a merge. It's simply too uncommon not to compose fragments, and that requires named fragments. There's probably something to be said though about that being optimised in the future

(Note: we talked about nested signals/reactive proxies, so that could be an approach to feed information on changes in above maskFragment and to hand info to the next nested maskFragment for a different document; not in scope for the PR anyway)

Question is what full-masking (like in this implementation) would also look like given heuristic/non-heuristic fragment matching.

Last note is that we could align this a bit more to the Graphcache implementation here so it vaguely matches. Just in the overall style, so we have a bit of parity in case we decide to share code. That's optional though and not necessarily actionable, unless this gives you an idea of course

There are a few things bothering me about doing that, we should def do FragmentSpread as alluded to in the todo, however what bothers me is that these spreads could be suspended themselves. I wanted to create the assumption that a FragmentSpread with @defer wouldn't be registered in useFragment but there would be an expectation that this is passed on to a new component.

For InlineFragment I have implemented it similar to Field and defer would trigger internal to the component.

In writing this I realised that we actually can't do include/skip as it stands because we don't have access to the variables the parent-operation was executed with, this could be a limitation for now but weakens the heuristic we use to see whether something is deferred significantly.

RE the graphCache point, I can add heuristic fragment matching to this if that's desired, for now this only works on concrete matches.

EDIT: added an initial heuristic for include/skip in 6c7eb36 and naive heuristic matching in 8ccfba1

packages/react-urql/src/hooks/useFragment.ts

examples/with-defer-stream-directives/src/Songs.jsx

JoviDeCroock · 2024-04-24T10:46:34Z

packages/react-urql/src/hooks/useFragment.ts

+        if (newResult.fulfilled) {
+          return { data: newResult.data, fetching: false };
+        } else if (suspense) {
+          const promise = new Promise(() => {});


Being able to resolve this Promise might be important for streamed-SSR, I am not sure 😅 the issue here being that we can't keep a stable reference to this cache-entry as we face two challenges

This could be a series of useFragment, think of a list of todos. This means we'd be able to create a key for the document, however we'd then share this key across all siblings which could be streamed in out of order.

When we add data to the key this means that when the previous entry is completed due to the missing part streaming in that we update the key to a new state. This becomes problematic as we never actually resolve the promise, storing the previous reference in a ref isn't possible as React tears down the previous state.

Leveraging useId is also not possible as that isn't stable across Suspense invocations

One solution I can see is deriving a data that follows along the document and purposefully omits any include/skip and defer from the data and uses that to derive the key. This would essentially be similar to creating a __typename:id as they key, which could also be relevant here.

EDIT: suggestion implemented in ea665f3

JoviDeCroock · 2024-04-24T12:41:00Z

This proposal has run into kind of a dead-end after trying it out with streamed rendering, the issue resides in the fact that during SSR React does not do any re-rendering which means useFragment will never be re-evaluated.

For this to work we'd have to establish a subscription in our urql-client where this fragment is used, this subscription would need to bring the data to the useFragment, this adds additional complexity to this API proposal because we'd need to be aware on which entity we are listening for changes and hook into the correct query. to evaluate this fragment against.

JoviDeCroock added 5 commits April 22, 2024 20:00

mvp

6e91676

working mvp

be0b26e

improvements

65ae40e

tsdoc

0c722d9

correction

8731db6

JoviDeCroock added 2 commits April 22, 2024 21:06

add inline fragment support

a318dfd

logging

d9ef0cd

kitten reviewed Apr 22, 2024

View reviewed changes

JoviDeCroock added 3 commits April 23, 2024 08:21

add include/skip

6c7eb36

wording change

88b84df

cleanup and heuristic fragment matching

8ccfba1

JoviDeCroock commented Apr 23, 2024

View reviewed changes

packages/react-urql/src/hooks/useFragment.ts Outdated Show resolved Hide resolved

add comments and remove code

dadf926

JoviDeCroock commented Apr 23, 2024

View reviewed changes

examples/with-defer-stream-directives/src/Songs.jsx Show resolved Hide resolved

JoviDeCroock added 3 commits April 23, 2024 14:14

hoist a few things

ec4fd17

update condition

ce0f551

include/skip on fragments

b9a274a

JoviDeCroock marked this pull request as ready for review April 23, 2024 12:39

JoviDeCroock added 3 commits April 23, 2024 14:40

add changeset

1187faf

add defer handling to fragment-spread

4aa9096

resolve promise

17f8336

JoviDeCroock force-pushed the use-fragment-mvp branch from 686f300 to 17f8336 Compare April 23, 2024 12:52

remove comments

d353496

JoviDeCroock requested a review from kitten April 23, 2024 12:53

JoviDeCroock added 4 commits April 23, 2024 15:27

add support for null

892b8c7

fixes

57f1a58

add tests

fb6f98f

add one more test

1e16372

JoviDeCroock commented Apr 24, 2024

View reviewed changes

JoviDeCroock added 3 commits April 24, 2024 12:52

propose solution to keying problem

ea665f3

use the createRequest export instead of keyDocument

85d7ed6

alternative

11727b6

JoviDeCroock closed this Apr 24, 2024

JoviDeCroock deleted the use-fragment-mvp branch April 24, 2024 12:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(react): useFragment hook (BETA) #3570

feat(react): useFragment hook (BETA) #3570

JoviDeCroock commented Apr 22, 2024 •

edited

changeset-bot bot commented Apr 22, 2024 •

edited

kitten Apr 22, 2024

JoviDeCroock Apr 23, 2024 •

edited

JoviDeCroock Apr 24, 2024 •

edited

JoviDeCroock commented Apr 24, 2024

feat(react): useFragment hook (BETA) #3570

feat(react): useFragment hook (BETA) #3570

Conversation

JoviDeCroock commented Apr 22, 2024 • edited

Summary

Notes

Todo

changeset-bot bot commented Apr 22, 2024 • edited

🦋 Changeset detected

kitten Apr 22, 2024

Choose a reason for hiding this comment

JoviDeCroock Apr 23, 2024 • edited

Choose a reason for hiding this comment

JoviDeCroock Apr 24, 2024 • edited

Choose a reason for hiding this comment

JoviDeCroock commented Apr 24, 2024

JoviDeCroock commented Apr 22, 2024 •

edited

changeset-bot bot commented Apr 22, 2024 •

edited

JoviDeCroock Apr 23, 2024 •

edited

JoviDeCroock Apr 24, 2024 •

edited