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

Error when reading typedef from file in V2.6.6 #2921

Closed
ais-one opened this issue Jun 25, 2019 · 4 comments · Fixed by #2924
Closed

Error when reading typedef from file in V2.6.6 #2921

ais-one opened this issue Jun 25, 2019 · 4 comments · Fixed by #2924

Comments

@ais-one
Copy link

ais-one commented Jun 25, 2019

Package Name & Version

The issue is present in apollo-server-express, the latest version (2.6.6).

Latest version where the problem did not occur

My previous version was 2.6.3

Expected Behavior

gql schema can load from file

Actual Behavior

The following fatal error occurs

TypeError: Cannot read property 'some' of undefined

Repo demonstrating the problem

https://github.com/ais-one/apollo-express-test
It's a simple example showing how to replicate the problem

@TheKidCoder

This comment has been minimized.

@mrsunboss
Copy link
Contributor

Maybe is this problem => #2924

abernix added a commit to mrsunboss/apollo-server that referenced this issue Jun 26, 2019
abernix added a commit to mrsunboss/apollo-server that referenced this issue Jun 26, 2019
Specifically, make sure to test all possibilities which might come in here.
@jesstelford
Copy link
Contributor

See also #2753

jesstelford added a commit to keystonejs/keystone that referenced this issue Jun 26, 2019
Apollo released a breaking change in a semver-minor which causes it to
stop understanding the SDL (string) GraphQL typeDefs we were passing it.
This fix ensures we're converting to an AST to avoid the error being
thrown.

See apollographql/apollo-server#2921

Fixes #1340
@abernix
Copy link
Member

abernix commented Jun 26, 2019

Thank you @mrsunboss for tracking down the issue and opening the appropriate PR to fix the problem. This subtle but positive triaging, linking and fixing is well received and great for the project, so thank you!

This was introduced by an unguarded property access in #2762. TypeScript offers us a lot of protections, and seemed to indicate that there was nothing to worry about, but TypeScript is still is no substitute for defensive coding.

Tests help protect all involved here, both on our side, and on the side of our consumers — who should, as a best practice — have integration tests in place to make sure their production expectations and needs are met.

On top of @mrsunboss's #2924, I've added a regression test for this in 32deb9f, a further defense for another explored possibility in 490c93a and updated the ApolloServer constructor types with 77207ff, which was ultimately most at fault for allowing this regression to occur, since JavaScript users do still pass string typeDefs (those not wrapped with the gql tag) as a matter of historical fact.

I'll release 2.6.7 soon with the fix.

jesstelford added a commit to keystonejs/keystone that referenced this issue Jun 26, 2019
Apollo released a breaking change in a semver-minor which causes it to
stop understanding the SDL (string) GraphQL typeDefs we were passing it.
This fix ensures we're converting to an AST to avoid the error being
thrown.

See apollographql/apollo-server#2921

Fixes #1340
jesstelford added a commit to keystonejs/keystone that referenced this issue Jun 26, 2019
Apollo released a breaking change in a semver-minor which causes it to
stop understanding the SDL (string) GraphQL typeDefs we were passing it.
This fix ensures we're converting to an AST to avoid the error being
thrown.

See apollographql/apollo-server#2921

Fixes #1340
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 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.

5 participants