Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

formatResponse typings: null response? #5009

Closed
nwalters512 opened this issue Mar 8, 2021 · 3 comments · Fixed by #5089
Closed

formatResponse typings: null response? #5009

nwalters512 opened this issue Mar 8, 2021 · 3 comments · Fixed by #5089

Comments

@nwalters512
Copy link
Contributor

In #3431, the typings for formatResponse were set such that the response argument could be null. However, this causes problems when trying to write a basic implementation that, for instance, adds a property to the object:

new ApolloServer({
  formatResponse: (response) => {
    if (response) {
      if (!response.extensions) {
        response.extensions = {};
      }
      response.extensions.code_version = VERSION;
    }
    return response;
  }
})

This produces a TypeScript error, as response is typed as GraphQLResponse | null but the expected return type is just GraphQLResponse (non-nullable):

Type '(response: GraphQLResponse | null) => GraphQLResponse | null' is not assignable to type '(response: GraphQLResponse | null, requestContext: GraphQLRequestContext<object>) => GraphQLResponse'.
  Type 'GraphQLResponse | null' is not assignable to type 'GraphQLResponse'.
    Type 'null' is not assignable to type 'GraphQLResponse'.

It's not clear to me if response ever actually can be null here, but I would expect this function to be typed such that the response argument and the return value have the same type. If response will not ever be null, I'd like the types to reflect that.

This is mentioned in a comment on #4186, though I don't think that's actually the issue that OP encountered there, so I opted to lift this into a new issue.

glasser added a commit that referenced this issue Apr 6, 2021
This typing was added in #3431; it looks like we put the `| null` in the wrong
place. Fortunately, a function that's OK with accepting a null argument and will
never return null can still typecheck with the new type.

Fixes #5009.
glasser added a commit that referenced this issue Apr 6, 2021
This typing was added in #3431; it looks like we put the `| null` in the wrong
place. Fortunately, a function that's OK with accepting a null argument and will
never return null can still typecheck with the new type.

Fixes #5009.
@glasser
Copy link
Member

glasser commented Apr 6, 2021

Thanks for the report. Looks like we put the | null in the wrong place in #3431. Looks like #5089 should fix it.

glasser added a commit that referenced this issue Apr 6, 2021
This typing was added in #3431; it looks like we put the `| null` in the wrong
place. Fortunately, a function that's OK with accepting a null argument and will
never return null can still typecheck with the new type.

Fixes #5009.
glasser added a commit that referenced this issue Apr 6, 2021
This typing was added in #3431; it looks like we put the `| null` in the wrong
place. Fortunately, a function that's OK with accepting a null argument and will
never return null can still typecheck with the new type.

Fixes #5009.
@glasser
Copy link
Member

glasser commented Apr 9, 2021

I've released a prerelease with this fix, version 2.23.0-alpha.0. Try out the alpha and see if it works for you! Please provide any feedback in #5094.

@glasser
Copy link
Member

glasser commented Apr 14, 2021

This is released in Apollo Server 2.23.0.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants