Changes to the incremental responses

[jer-k]

Fixes: #203

Preface: This is my first exposure to working with these Node objects on the server side so I'm not entirely sure if what I've done is correct.

I changed the way the incremental responses are sent back to the client. In the issue I noted

      for await (const chunk of httpGraphQLResponse.body.asyncIterator) {
        body.push(chunk);
      }

this await is stopping the server from incrementally delivering the responses and instead they're all delivered at once.

These changes allow the responses to be streamed back instead of all at once.

The logs in the client now show

what is data? {book: {…}} false 12:22:18
what is data? {book: {…}} false 12:22:19

Where the second book: {} has the deferred data and is delivered 1 second after the first in which the 1 second is hard coded in my project using this package.

[jer-k]

A couple things I've noticed. With these changes the Preview and Response panes in the Chrome DevTools aren't showing any data even though the response is working

I'm guessing something I've changed messed this up.

[jer-k]

If this header isn't set, then Apollo Client throws an error that looks like this

Error: No number after minus sign in JSON at position 3 (line 2 column 2)

Looking at the data being sent, it lines up with the ---

After setting it to 'multipart/mixed; boundary="-"' the Preview and Response panels display nothing, as I mentioned in the other comment. Not sure why this is?

[jer-k]

Looking below, where I actually spread in the existing headers

      headers: {
        ...headers,
        'Content-Type': 'application/json',
        'Transfer-Encoding': 'chunked',
      },

those requests have multipart/mixed; boundary="-"; deferSpec=20220824 so I probably need to do something similar here instead of hard coding

[jer-k]

I fixed this by ensuring the passed in headers are applied in all cases

[martinnabhan]

Thank you for the thorough explanation! I'll try to take a closer look this week.

[martinnabhan]

I've added some suggestions! Could you also remove noIncrementalDelivery: true from src/__tests__/integration.test.ts so we make sure to test incremental delivery as well?

I also noticed Chrome's Preview and Response panes aren't showing any data but I think that's normal for multipart responses.

[martinnabhan]

Since headers isn't used in the if (isNextApiRequest(req)) code block it would be better to move this below the if statement.

[martinnabhan]

I don't think this is necessary if we use res.send below.

[martinnabhan]

I think it's more common to use pull, and then use the iterator without looping like this:

Suggested change

	async start(controller) {
	for await (const chunk of responseBody.asyncIterator) {
	controller.enqueue(chunk);
	}
	controller.close();
	},
	async pull(controller) {
	if (httpGraphQLResponse.body.kind === 'chunked') {
	const { value, done } = await httpGraphQLResponse.body.asyncIterator.next();
	if (done) {
	controller.close();
	} else {
	controller.enqueue(value);
	}
	}
	},

[martinnabhan]

We should support incremental responses for API routes as well. We can just replace the for loop with this:

res.send(Readable.from(httpGraphQLResponse.body.asyncIterator));

[martinnabhan]

I would prefer if we could use a ternary instead of a re-assignable variable, like this:

    return new Response(
      httpGraphQLResponse.body.kind === 'complete'
        ? httpGraphQLResponse.body.string
        : new ReadableStream({ ... }),
      { headers, status: httpGraphQLResponse.status || 200 },
    );

[jer-k]

Thank you for all the feedback! It is a much cleaner implementation.

On the Preview panel, there definitely is some way for the data to show up. This screenshot is from my work's application, which uses Apollo Client + graphql-ruby. I'll spend some time this morning and see if I can't uncover any information as to what is going on

[jer-k]

Okay I figured it out and I believe it is an upstream issue in Apollo Server and improperly setting the header.

what are raw headers? HeaderMap(2) [Map] {
  'cache-control' => 'no-store',
  'content-type' => 'multipart/mixed; boundary="-"; deferSpec=20220824',
  __identity: Symbol(HeaderMap)
}

When the headers are written in and the deferSpec=20220824', portion is retained, we do not get the Preview response.

I changed the code to

    const headers: Record<string, string> = {
      'content-type': 'multipart/mixed; boundary="-"',
    };
    // console.log('what are raw headers?', httpGraphQLResponse.headers);
    // for (const [key, value] of httpGraphQLResponse.headers) {
    //   headers[key] = value;
    // }

And the preview correctly displays

[martinnabhan]

Thank you for looking into this. I don't think we want to change any headers coming from Apollo Server, so I'll go ahead and add a changeset and release this as is.

Thank you for the pull request!