Skip to content
This repository has been archived by the owner on Mar 20, 2023. It is now read-only.

Add support for defer and stream directives (feedback is welcome) #726

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

robrichard
Copy link
Collaborator

@robrichard robrichard commented Nov 23, 2020

This branch is being published to npm as express-graphql@experimental-stream-defer

Continued from #583

References:

@IvanGoncharov IvanGoncharov added the PR: feature 🚀 requires increase of "minor" version number label Nov 23, 2020
@acao acao requested review from danielrearden and benjie November 29, 2020 17:05
@robrichard robrichard force-pushed the defer-stream branch 4 times, most recently from da0c17c to 213d365 Compare December 10, 2020 04:00
@acao
Copy link
Member

acao commented Jan 26, 2021

This PR is looking fantastic! locally with a simple express server its working great. with the demo schema resolvers I made, and a query such as the one below, I can mimic mixed rapid or high latency streams by setting a delay argument

we have a PR that implements this branch in graphiql:
graphql/graphiql#1770

I figured out a way to make it actually update state with every multipart increment. in 2.0 this will be handled with a much more performant state management strategy, but for now it seems fine.

we now have a (mostly) working netlify deploy preview, using a query like this:

{
  isTest
  stream: streamable(delay: 500) @stream(initialCount: 2) {
    text
  }
  anotherStream: streamable(delay: 200) @stream(initialCount: 0) {
    text
  }
}

notice how it waits and then batches all the increments at the end so it looks like a normal, very delayed request? and if you increase the delay, the more it delays the whole request.

what I'm guessing is that netlify's CDN is batching the multipart requests somehow? i can't seem to increase the delay to a point that changes this, so I wonder what else I'm missing.

I used aws-serverless-express which is deprecated, but seems to work fine for normal POST and GET requests? we use it for swapi-graphql. maybe will try out serverless-http instead.

We added multipart/mixed to the list of known mime types as well

I apologize if this question is not relevant to the PR discussion, and if so can move this comment over to our PR!

@maraisr
Copy link

maraisr commented Jan 31, 2021

Hi @IvanGoncharov im working with @acao around getting defer/stream into GraphiQL and was wondering if you could do us a solid and cut a release under the current experimental tag (or whatever you feel) that includes 633ca8b.

meros (used by graphiql now) depends on this behavior being present, as the timing of flushing chunks with boundaries is critical, largely based on latest "spec proposal" updates.

Base automatically changed from master to main February 10, 2021 15:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
PR: feature 🚀 requires increase of "minor" version number
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants