Editorial: Define _response map_

[robrichard]

Follow up from #1135, Based on top of #1148

This creates a definition for Response Map, defined as the map normally returned by GraphQL queries and mutations containing data and/or errors. The term response map is already being use so I formally defined it and used the defined term throughout the rest of the spec. I also have an alternative proposal, Response Payload, as I think that fits better in the direction Incremental Delivery will be going.

The definition of Response is updated to include both Response Map, and Response Stream (for subscriptions).

If we go with response map, I think the end result with Incremental Delivery will be something like:

type ResponseMap = {
  data?: object
  errors?: Error[]
  extensions?: object
}
type ResponseStream = ResponseMap[]
type IncrementalStream = [InitialResponseMap, ...SubsequentResponseMap];
type Response = ResponseMap | ResponseStream | IncrementalStream;

I went through every mention of response and most of the remaining usages are around response key or response field, referring to the object property name (ie for aliases and error paths). This could also likely be standardized in a follow up. See https://github.com/graphql/graphql-spec/pull/1147/files

[netlify]

✅ Deploy Preview for graphql-spec-draft ready!

Name	Link
🔨 Latest commit	6c6f765
🔍 Latest deploy log	https://app.netlify.com/sites/graphql-spec-draft/deploys/67ee764231d68900086eaf48
😎 Deploy Preview	https://deploy-preview-1143--graphql-spec-draft.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 configuration.

[benjie]

Looks good to me 👍

(I have not done a full review of the spec to ensure we caught everything.)

[benjie]

I prefer #1149 - "response payload" is much clearer IMO.

[martinbonnin]

I like payload better too 👍

Alternatively, did we already discuss reusing the graphql-js terminology?

/**
 * The result of GraphQL execution.
 *
 *   - `errors` is included when any errors occurred as a non-empty array.
 *   - `data` is the result of a successful execution of the query.
 *   - `hasNext` is true if a future payload is expected.
 *   - `extensions` is reserved for adding non-standard properties.
 *   - `incremental` is a list of the results from defer/stream directives.
 */
export interface ExecutionResult {
  errors?: ReadonlyArray<GraphQLError>;
  data?: TData | null;
  extensions?: TExtensions;
}

[benjie]

I like that, but it would involve renaming a few other things too. In particular ResponseStream being a stream of ExecutionResult is less clear. Since we're introducing IncrementalStream, I wonder if just calling it SubscriptionStream would be better? Then we'd end up with:

type Response = ExecutionResult | SubscriptionStream | IncrementalStream;
type ExecutionResult = {
  data?: object
  errors?: Error[]
  extensions?: object
}
type SubscriptionStream = ExecutionResult[]
type IncrementalStream = [InitialExecutionResult, ...SubsequentExecutionResult];

type SubsequentExecutionResult = {
  incremental?: IncrementalResult[]
  pending?: Pending[]
  completed?: CompletedResult[],
  hasNext?: boolean;
}

type InitialExecutionResult =
  ExecutionResult &
  Partial<IncrementalExecutionResult> &
  { hasNext: true };

type IncrementalResult = IncrementalObjectResult | IncrementalListResult

This kind of works?

[martinbonnin]

That'd work for me 👍 . As I was explaining in the other pr, I expect ${Foo}Stream to be a stream of Foo. If we wanted to keep things a bit more lightweight, we could also remove the Execution from ExecutionResult.

So maybe ResultStream/IncrementalResultStream instead of SubscriptionStream/IncrementalStream?

type Response = Result | ResultStream | IncrementalResultStream;
type Result = {
  data?: object
  errors?: Error[]
  extensions?: object
}
type ResultStream = Result[]
type IncrementalResultStream = [InitialResult, ...SubsequentResult];

type SubsequentResult = {
  incremental?: IncrementalResult[]
  pending?: Pending[]
  completed?: CompletedResult[],
  hasNext?: boolean;
}

type InitialResult =
  Result &
  Partial<IncrementalResult> &
  { hasNext: true };

type IncrementalResult = IncrementalObjectResult | IncrementalListResult

[benjie]

If we wanted to keep things a bit more lightweight, we could also remove the Execution from ExecutionResult.

We use the term "result" throughout the GraphQL spec, so I think that calling it "result" would be dangerous. ("response" is not used so generically, so doesn't suffer the same issue.)

I expect ${Foo}Stream to be a stream of Foo

That's how it's used currently, but I think it's also reasonable for it to be a name for the stream - e.g. "SubscriptionStream" is clearly to me "a stream for a subscription" rather than "a stream of Subscription". We often use prefixes to name things to disambiguate them, e.g. in CollectFields() we use fragmentGroupedFieldSet to mean "the groupedFieldSet related to this fragment".

[leebyron]

I think we should keep "Response" as the general concept and qualify it with each variant. Response Map, Incremental Response Map, Response Stream

This will present longer names, but generally we should aim to avoid long term ambiguity if possible

[martinbonnin]

@leebyron "Response Stream" is really ambiguous to me. Is it a response or is it a stream of responses? Or both?

[martinbonnin]

Also interesting question is how we look at this in the context of GraphQL over HTTP.

Do we consider that application/graphql-response+json may contain incremental/subscription responses (feels quite weird IMO).

Incremental responses could potentially use jsonl format (application/graphql-response+jsonl) but not having the media type discriminate what type of response is expected feels a bit weird.