incrementalDelivery: original execute should not throw on new directives

[yaacovCR]

original execute and executeSync should ignore @defer and @stream rather than throw.

extracted from #3726

[netlify[bot]]

✅ Deploy Preview for compassionate-pike-271cb3 ready!

Name	Link
🔨 Latest commit	ff46be16e13e27b79dfb9c82806c19f27570b98e
🔍 Latest deploy log	https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/6321dfa9eba12b000a21175f
😎 Deploy Preview	https://deploy-preview-3727--compassionate-pike-271cb3.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[github-actions[bot]]

Hi @yaacovCR, I'm @github-actions bot happy to help you with this PR 👋

Supported commands

Please post this commands in separate comments and only one per comment:

• @github-actions run-benchmark - Run benchmark comparing base and merge commits for this PR
• @github-actions publish-pr-on-npm - Build package from this PR and publish it on NPM

[robrichard]

Why is it preferable to ignore the directives rather than throw?

[yaacovCR]

We are talking about operations that pass validation because the directives are in the schema. If for some reason the server has not actually enabled the incremental delivery behavior, it is not the fault of the client. I am just having a hard time figuring out why they should throw.

Basically, what I believe we are trying to do is enable experimental behavior. If the behavior is not implemented, it should behave just as before, no?

[yaacovCR]

Alternative to #3722

[robrichard]

@yaacovCR would it be more likely that someone is trying to get defer/stream to work but doesn't realize they are using the wrong functions/not passing the experimental flag? And in that case, an error message would be helpful to guide them to correct usage?

[yaacovCR]

Suppose a server developer mistakenly thought that the alpha included defer/stream support without the use of an experimental function/flag.

So they upgraded GraphQL to alpha, sent in a query with defer, and disappointingly got just a single response.

I do think an error would help that developer.

But how likely is that? Upgrading a server to support incremental delivery requires preparing for execution to return an async iterable rather a single result... But the return type of the original execute just includes a single response -- so why would a developer think it would work?

Perhaps because the developer is using JavaScript and not TS and also does not have IDE support for types?

But we recently basically pulled support for JS #3642 ?

I also truly do not know what is more likely. I can imagine that a developer might by accident (or even on purpose?) ship to production a schema with the directives included, but not want to yet enable it in prod. That developer would expect that clients would continue to receive results.

I can imagine a different solution that might better help a developer. Rather than only failing queries with incremental results, we could modify the original execute to immediately throw if the defer/stream directives are in the schema, or at least print a warning. We could also modify the new function/flag to throw or print a warning if both directives are not in the schema.

[yaacovCR]

Closing in favor of #3722.