New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Modern] Deferred queries #2194

Open
edvinerikson opened this Issue Nov 15, 2017 · 13 comments

Comments

Projects
None yet
7 participants
@edvinerikson
Contributor

edvinerikson commented Nov 15, 2017

Hello everyone!

I very recently started thinking about using deferred queries for making a progressive react app with Relay. My idea to use deferred queries for this is to be able to split data requirements the same way we can split react components. This is kind of possible by having query renderers.. but it quickly becomes very bloated and hard to work with.

From a conversation on Twitter I am creating this issue to track the progress and share thoughts on adding deferred queries into Relay Modern. I have myself started experimenting with it, so far I've only been working on a basic query transformer in the compiler.

I heard that you @leebyron actually are working on this internally right now. Is it possible for you to share your work and issues here on Github? I think it would be really good if this can be developed in the open behind some experimental/unstable flag (like React does).

It would also be very interesting to hear what thoughts the rest of the Relay team and others have on this! @josephsavona "confirmed" on Twitter that it is possible to solve. @wincent Maybe you have some ideas on this since you worked on it for Relay Classic.

I am imaging the relay compiler building these extra queries just like normal queries and have the runtime linking them together somehow. Or maybe it makes sense to introduce a list/tree on the concrete batch for deferred queries.

Main issues I see

I see some problems right now... I have not had time to dig into them yet though...

  • Variables
    Figuring out what variables the deferred queries needs and does not need. Maybe we need some sort filtering of variables at runtime.

  • store selectors
    Do we need to switch the selectors when starting to execute deferred queries or can Relay handle the initial selector including all fields from the deferred responses? Since the initial query response will miss those fields I assume Relay will throw errors about fields being undefined in the normalize phase.

  • Refetch containers / pagination containers
    I don't know if this will be a problem.. but I have feeling these will require some extra logic around this.

@josephsavona

This comment has been minimized.

Contributor

josephsavona commented Nov 15, 2017

Here's the approach I imagine:

  • Compiler allows @defer annotation on fragment spreads, the type of the fragment must implement Node such that the object has an id.
  • These fragment spreads are replaced with a handled field, and a new node query is synthesized.
  • All of the queries split off from within a root query are grouped together into a ConcreteBatch (it was intended to eventually contain multiple queries for this exact purpose). This will have to track interdependencies of queries (possibly the root query plus a possibly-empty array/tree of dependents each with an id, query, and parent id).
  • The network layer continues to accept a ConcreteBatch, but it now has to handle the possibility of multiple queries. The network API was designed to support multiple (N) responses for a given query so the API for returning multiple payloads over time is all set up. The main change is that we'll probably want (e.g. for open source) a utility that executes query batches on the client - executing parent queries until the root id of sub-queries is known, at which point it can send those queries.
  • Users will want to know if a deferred fragment is available or not - this is where the synthesized "deferred fragment" handle field comes in - in a parent query the processing of the handle field sets a flag in the store that fragment "X" is pending on node Y. The synthesized deferred query would also contain a handler field, which when processed toggles the flag to mark fragment "X" as resolved for node Y.
@edvinerikson

This comment has been minimized.

Contributor

edvinerikson commented Nov 16, 2017

Thank you for the detailed write-up @josephsavona!

I have some questions though

Compiler allows @defer annotation on fragment spreads, the type of the fragment must implement Node such that the object has an id.

If you add the node dependency on the fragment spread type, how do you know which one to fetch later? Will the generate id transform add the id field to the parent query?

What if we put the responsibillity on the parent type instead?

If I understand your suggestion the queries will roughly look like this:

fragment LiveData on LiveData {
  id
  seatsAvailable 
}

query OriginalA {
  game(id: "roulette") {
   tableName
    liveData {
      ...LiveData @defer
    }
  }
}

query GeneratedA {
  game(id: "roulette") {
    tableName
    liveData {
      id
    }
  }
}

query ADeferred(id: ID!) {
  node(id: $id) {
    ...LiveData
  }
}

However, the issue I see with this is that what if even requesting the id of the liveData object is expensive too?

I imagine that pushing the fetching up to the parent node would allow for easier usage. However, as seen below the queries becomes more complex.

fragment LiveData on LiveData {
  id
  seatsAvailable 
}

query OriginalA {
  game(id: "roulette") {
   tableName
    liveData {
      ...LiveData @defer
    }
  }
}

query GeneratedA {
  game(id: "roulette") {
    id
    tableName
  }
}

query ADeferred(id: ID!) {
  node(id: $id) {
    ... on Game {
       liveData {
          ...LiveData
       }
    }
  }
}


Users will want to know if a deferred fragment is available or not

How do we handle network failures or server errors?

@soneymathew

This comment has been minimized.

soneymathew commented Mar 27, 2018

I notice that this is in-progress

https://twitter.com/leeb/status/930654028798631936

@leebyron is there a plan to support @defer in Relay Modern or would you recommend the query authors handle this via @include or @skip directives instead.

For example wrt to the above query, client could manage the variable state to handle like so until @defer is supported

...LiveData @include(if:$isRefetchTime)
or
...LiveData @skip(if:$isFirstTime)

@edvinerikson

This comment has been minimized.

Contributor

edvinerikson commented Mar 27, 2018

AFAIK this is actually already working, I haven't tested it though. I know that the runtime and compile is capable of handling some sort of deferred fragments at least.

I plan to try it in my app within a few weeks.

@johntran

This comment has been minimized.

Contributor

johntran commented Apr 18, 2018

@edvinerikson Are there code examples of deferred fragments?

@edvinerikson

This comment has been minimized.

Contributor

edvinerikson commented Apr 23, 2018

Not what I know, though you can annotate a fragment whose type uses the Node interface. But you need to implement your own network interface that supports the batching request that Relay generates for deferred queries. You can have a look at the deferred tests if you want to see more about the annotation. https://github.com/facebook/relay/tree/475585b78db7690e4c7364d32a30b9b34cf31916/packages/relay-compiler/transforms/__tests__/fixtures/deferrable-fragment-transform
@relay(deferrable: true)

@jstejada

This comment has been minimized.

Contributor

jstejada commented Apr 30, 2018

@sibelius

This comment has been minimized.

Collaborator

sibelius commented Jul 18, 2018

For anybody trying to implement this

You need to use Observable in your executeFunction of Network

take this as an example #2174 (comment)

@sibelius

This comment has been minimized.

Collaborator

sibelius commented Jul 23, 2018

I've managed to implemented it

It's open source here: https://github.com/sibelius/relay-modern-network-deep-dive

There is also a medium post about it: https://medium.com/@sibelius/relay-modern-network-deep-dive-ec187629dfd3

now we need to improve Relay official docs

@wbyoung

This comment has been minimized.

wbyoung commented Jul 23, 2018

@sibelius thanks for sharing the awesome write up and the very clean and organized example code. I hope this can make its way into the public docs and we can start seeing use of this become more practical to integrate.

Have you tried this over connections at all (when deferred query variables end up as lists)?

I made it about this far a month or two ago based on the comments here and some digging in the Relay source code. I eventually ditched the solution because I was unable to get it to work when deferring through to “many” items — for instance deferring fields from nodes in a connection which was the main requirement for me. I was able to get partway through figuring out how to extract the proper variables to send with the queries, but eventually ran into a situation where obtaining the proper key for the variable was not possible (I don’t remember the exact reason, but could probably find out if someone from the core team wanted to help us continue to push forward here). My code has more application specific stuff in it, but seems functionally close to what you have ended up looking like this.

Have you tried getting things working through connections?

Somewhat related… for the time being, I tried using a refetchContainer to build something similar, but hit a wall eventually when realizing that the callback for the refetch isn’t always called (my implementation assumed this), so I’ve now created something from QueryRenderer and fragmentContainer that batches all requests from the same tick. Even this approach has required me to use some internal knowledge and hackery to get everything working.

It’d be really nice to use deferred queries instead of hacking it together.

@sibelius

This comment has been minimized.

Collaborator

sibelius commented Jul 23, 2018

I have a @todo to implement it here https://github.com/sibelius/relay-modern-network-deep-dive/blob/master/src/5-deferrable-queries/batchRequestQuery.js#L20

So when you have a list you probably should fire N requests or use nodes instead of node interface

@wbyoung

This comment has been minimized.

wbyoung commented Jul 23, 2018

@sibelius yeah, using a nodes based query was what I was working toward, but I actually couldn't align the variables properly from the given inputs. The value to enumerate (each) over was present, but either the key didn't match up with the value in the operation or the key didn't match with the proper place in the response data — I can't remember which right now. I gave up because I figured that trying to push forward and open a PR here would be time wasted since this is an active area of work for the core team & not considered stable at the moment… I didn't want to duplicate work & didn't quite have the time either.

@sibelius

This comment has been minimized.

Collaborator

sibelius commented Oct 18, 2018

@relay(defer: true) was removed from master Relay

@defer on Apollo is still on pull request apollographql/apollo-server#1287

there is no final solution of doing defer on GraphQL world

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment