graphql-js icon indicating copy to clipboard operation
graphql-js copied to clipboard

execute: integrate subscriptions and refactor pipeline

Open yaacovCR opened this issue 2 years ago • 5 comments

execute no longer runs the query algorithm for subscription operations. Rather, subscription operations are performed, as per the spec. subscribe and createSourceEventStream are removed.

See additional comments below.

yaacovCR avatar Jun 14 '22 10:06 yaacovCR

Deploy Preview for compassionate-pike-271cb3 ready!

Name Link
Latest commit 3f69fb3581931a9e6ac459430f0ba5d01332121f
Latest deploy log https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/62bb53b590545b0008a5aa4b
Deploy Preview https://deploy-preview-3644--compassionate-pike-271cb3.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

netlify[bot] avatar Jun 14 '22 10:06 netlify[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

github-actions[bot] avatar Jun 14 '22 10:06 github-actions[bot]

Now that #3658 suggests that we may be awaiting a serious refactor, this PR has been reworked to represent a least change approach to the existing API. Rather than list the individual steps to the granularity of every renamed function, I will just list broadly what the PR accomplishes, after which I will list what the PR does not do.

  1. updates execute to run the full ExecuteSubscription algorithm rather than just ExecuteSubscriptionEvent, updating the return types of execute and graphql
  2. removes subscribe/createSourceEventStream (these would have to be deprecated in a 16.x release).
  3. introduces DocumentInfo and ExecutionInfo interfaces that advanced clients can use to analyze a document and set up execution parameters. Internal errors variable -- the only necessarily private information required during execution -- is left within ExecutionContext. This is generally similar to the setup within defer/stream with payload context objects, so the separate variable is not much of an issue (in my opinion).
  4. export helpers buildDocumentInfo, coerceVariableValues, as well as buildExecutionInfo so that advanced clients can have some help creating parts or all of the DocumentInfo/ExecutionInfo.
  5. expose helpers executeQuery, executeMutation, executeSubscription, and createSourceEventStream which take ExecutionInfo rather than ExecutionContext objects. These functions return appropriately typed versions, i.e. executeQuery/executeMutation do not ever return streams (until defer/stream) merged.
  6. no need anymore for the newly introduced *Impl functions within execute

What this PR does not do:

  1. compile schemas/documents/field collections/variables/arguments
  2. provide runtime or type checks for the arguments to executeQuery, etc. I.e. executeQuery can only take an ExecutionInfo object with an operation that is of type QUERY (for ExecuteQuery) or SUBSCRIPTION (for ExecuteSubscriptionEvent). executeMutation should only work for info objects containing a MUTATION, and executeSubscription and createSourceEventStream should only work for info objects containing a SUBSCRIPTION. That is because that TS does not (yet?) support discriminated unions via nested properties and so it's not as easy to do without introducing repetitive type guards that drill down into the ExecutionInfo object or an additional somewhat unnecessary property on ExecutionInfo itself. Considering the type safety comes at a runtime expense, I am forgoing this for now.

yaacovCR avatar Jun 29 '22 05:06 yaacovCR

The new exported helpers have not yet been exported from main, and are only available via deep imports. That can/should change once the API is finalized.

yaacovCR avatar Jun 29 '22 05:06 yaacovCR

Ah => I forgot to mention that I have also snuck in here a new argument to allow total customization of the executor used by EXECUTE_SUBSCRIPTION_EVENT.

As well as a default executor:

export const defaultSubscriptionEventExecutor = (
  exeInfo: ExecutionInfo,
  payload: unknown,
): PromiseOrValue<ExecutionResult> =>
  executeQuery({
    ...exeInfo,
    rootValue: payload,
  });

This workflow allows the buildPerEventExecutionContext function to be removed.

yaacovCR avatar Jun 29 '22 05:06 yaacovCR