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

Update @graphql-tools/executor peer dependencies to exclude graphql v16 #6279

Closed
alessbell opened this issue Jun 18, 2024 · 2 comments · Fixed by #6280
Closed

Update @graphql-tools/executor peer dependencies to exclude graphql v16 #6279

alessbell opened this issue Jun 18, 2024 · 2 comments · Fixed by #6280

Comments

@alessbell
Copy link

alessbell commented Jun 18, 2024

Hi GraphQL Tools team 👋 I'm using @graphql-tools/executor over in https://github.com/apollographql/graphql-testing-library and it's been super nice to use, so thanks :)

One thing I noticed is that when using it with a peer dependency of [email protected] I'm getting the following error:

Support for returning GraphQLObjectType from resolveType was removed in [email protected] please return type name instead.

This was surprising since graphql is listed as a peer dependency within the following range:

"peerDependencies": {
"graphql": "^14.0.0 || ^15.0.0 || ^16.0.0 || ^17.0.0"
},

Given the error that's being thrown I'd expect the peer dependency range to not include v15 or below.

Edit: I see that execute.ts is copy pasted from graphql-js@16, so the peer dependency range is even more misleading here :)

@ardatan
Copy link
Owner

ardatan commented Jun 19, 2024

Thank you for creating the issue!

Actually the idea behind this executor is to support defer/stream, and the new features from the next graphql-js releases with the schemas created by any graphql version.

The executor also accepts signal: AbortSignal which allows us to cancel the execution (used in a Yoga plugin)

I think instead of dropping a peer dependency which would introduce a breaking change, we can basically support the old way of resolving the types as I did in this PR;
#6280

Let me know what you think. And you can also check the alpha versions published by CI if you want to try it out before the patch release.

Here you can find it; #6279 (comment)

@alessbell
Copy link
Author

Thanks @ardatan! Appreciate the quick fix.

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 a pull request may close this issue.

2 participants