Refactor GraphQLResponse type; drop formatResponse #6502
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit aafdb67:
|
|
|
||
| return { | ||
| hints: cacheHints, | ||
| policyIfCacheable: response.extensions!.__policyIfCacheable__, | ||
| policyIfCacheable: response.result.extensions!.__policyIfCacheable__ as any, |
There was a problem hiding this comment.
There's a type argument to FormattedExecutionResult that might be the solution to the as any here
There was a problem hiding this comment.
hmm, not really sure where that would work. we would want to add generics to GraphQLResponse too then? which would have to be different for different operations? or are you just saying add a slightly different cast here?
note that the reason for this change is just that the old typings had extensions have any as its record value type and now it's unknown.
| function parse(query: string, parseOptions?: ParseOptions): DocumentNode { | ||
| return graphqlParse(query, parseOptions); |
There was a problem hiding this comment.
Not related to your changes but why does this exist?
There was a problem hiding this comment.
Before Jesse removed the old extension stack API this function called it. (05f0d2f) Sure, let's inline, why not.
| updateResponseHTTP(http); | ||
|
|
||
| if (!requestContext.response.http.statusCode) { | ||
| // TODO(AS4): This is a bit weird because it can |
There was a problem hiding this comment.
I think I made it less weird... was going to say that a few of the things that set it seemed more 500y, but as noted in the PR desc I made those into 500s.
Previously, the GraphQLResponse type contains the fields that are returned in the body of a GraphQL response, plus an `http` field with statusCode and headers. Now, we put the body fields (the "result") inside a `result` field next to `http`. This lets us just use the graphql-js ExecutionResult and FormattedExecutionResult types in a bunch of places as well. Among other things, this set us up for an incremental delivery world where sometimes there's an async iterator of these results instead of a single result. Changes vs v3 include: - The `GraphQLResponse` type as seen on plugin `GraphQLRequestContext.response` has most of its fields move to a `result` sub-object. Also, the types of values in the objects under `data` and `extensions` become `unknown` instead of `any` (so consumers may need to do some casts). - Remove `formatResponse` hook. The newer plugin API `willSendResponse` can do anything that it can do. - `GraphQLRequestContext.response` is always set, as is `GraphQLResponse.http`. - `GraphQLExecutionResult` is replaced with the nearly identical `ExecutionResult`. This is the type returned by the gateway execute command. The same slight typing difference about any vs unknown applies here but should be pretty irrelevant as the executor is producing these values. - If a resolveOperation hook throws or if the main execute call throws, return 500 status code instead of 400. Internal changes include: - Various functions in requestPipeline no longer return the GraphQLResponse (which can just be read off of requestContext.response). - New HTTPGraphQLHead type and newHTTPGraphQLHead() function (previously we used a `Pick<HTTPGraphQLResponse>` thing for this). - Instead of turning sendErrorResponse calls into 400s in runHttpQuery as a weird special case, just do it directly in sendErrorResponse. These errors will now also contain a `content-length` header.
glasser
force-pushed
the
glasser/v4-response-refactor
branch
from
f6be96a
to
aafdb67
Compare
Previously, the GraphQLResponse type contains the fields that are
returned in the body of a GraphQL response, plus an
httpfield withstatusCode and headers.
Now, we put the body fields (the "result") inside a
resultfield nextto
http. This lets us just use the graphql-js ExecutionResult andFormattedExecutionResult types in a bunch of places as well. Among other
things, this set us up for an incremental delivery world where sometimes
there's an async iterator of these results instead of a single result.
Changes vs v3 include:
GraphQLResponsetype as seen on pluginGraphQLRequestContext.responsehas most of its fields move to aresultsub-object. Also, the types of values in the objects underdataandextensionsbecomeunknowninstead ofany(soconsumers may need to do some casts).
formatResponsehook. The newer plugin APIwillSendResponsecan do anything that it can do.
GraphQLRequestContext.responseis always set, as isGraphQLResponse.http.GraphQLExecutionResultis replaced with the nearly identicalExecutionResult. This is the type returned by the gateway executecommand. The same slight typing difference about any vs unknown
applies here but should be pretty irrelevant as the executor is
producing these values.
return 500 status code instead of 400.
Internal changes include:
GraphQLResponse (which can just be read off of
requestContext.response).
(previously we used a
Pick<HTTPGraphQLResponse>thing for this).as a weird special case, just do it directly in sendErrorResponse.
These errors will now also contain a
content-lengthheader.