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

fix: revert unnecessary breaking change for resolveType #3249

Closed
wants to merge 2 commits into from

Conversation

n1ru4l
Copy link
Contributor

@n1ru4l n1ru4l commented Aug 27, 2021

We can introduce the following type signature for avoiding the breakage of many libraries. In general, I think stuff should first be deprecated in a major release and then finally be removed in the next major release.

export type GraphQLTypeResolver<TSource, TContext> = (
  value: TSource,
  context: TContext,
  info: GraphQLResolveInfo,
  abstractType: GraphQLAbstractType,
) => PromiseOrValue<GraphQLObjectType | string | undefined>;`

@n1ru4l n1ru4l mentioned this pull request Aug 27, 2021
@n1ru4l n1ru4l force-pushed the compat-resolve-type branch from 1ef8cb6 to d3ae05d Compare August 27, 2021 12:50
@IvanGoncharov
Copy link
Member

@n1ru4l Returning string was supported for a long time so I don't see an issue in libraries switching to returning names.

I think stuff should first be deprecated in a major release and then finally be removed in the next major release.

It's done for all the times where the library can't support multiple versions of graphql-js at the same time.
For this particular change, a library can switch to returning a string and support multiple versions of graphql-js at the same time.
But happy to merge if you can provide an example of a library that can't support v15 and v16 in the same release.

@n1ru4l
Copy link
Contributor Author

n1ru4l commented Aug 27, 2021

@IvanGoncharov I double-checked v15.5.1 and you are right, I was not aware of this! This should not be necessary then.

@n1ru4l n1ru4l closed this Aug 27, 2021
@n1ru4l n1ru4l deleted the compat-resolve-type branch August 27, 2021 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants