-
Notifications
You must be signed in to change notification settings - Fork 2k
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
federation: fix warning for mismatched unnamed @external
#4392
Conversation
Some TypeScript weirdness means we were getting the GraphQLNamedType overload of typeFromAST, which meant we were calling `.name` on it, which is undefined for non-null and list types.
a7412d1
to
5424379
Compare
@@ -36,6 +38,10 @@ describe('validateExternalDirectivesOnSchema', () => { | |||
"code": "EXTERNAL_TYPE_MISMATCH", | |||
"message": "[serviceB] Product.sku -> Type \`String\` does not match the type of the original field in serviceA (\`String!\`)", | |||
}, | |||
Object { | |||
"code": "EXTERNAL_TYPE_MISMATCH", | |||
"message": "[serviceB] Product.skew -> Type \`String!\` does not match the type of the original field in serviceA (\`String\`)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without the code change, this says undefined
instead of String!
Hi @glasser!,
We’re in the process of moving federation-related concerns out of this repository (where it has historically lived) into a new home specifically for Apollo Federation. This PR is affected by the transition since it touches code which has been moved. I’ve done some preparations to make this as easy as possible, but we’ll need to close this PR, and I could use your help in re-opening it on the new Apollo Federation repository. In apollographql/federation#134 (which has a lot more details, if you’re interested), we introduced the same code that was in this repository to the new Federation repository (maintaining its history) and removed the code from this repository in #4561. While this PR still needs to get reviewed and approved to land, it should no longer live in this repository in its current state since it cannot merge in anymore. To facilitate the movement of this PR to the new home, I’ve automatically generated branches on the new repository using the same commit authorship and messages that you originally included on this PR. Pull-requests can’t be moved on GitHub in the same way Issues can be. While I could just create the PRs from these new branches too, the contribution belongs to you! Original PR author only: Click this button to open a new PR from the auto-created (The code and your commits should be the same and you will have an opportunity to confirm, but this way you can continue to be the author and track its progress on the new Federation repository!) There’s no easy way to bring over any existing comments on this PR, so I encourage you to copy and paste those onto the new issue if possible. Overall, I hope this process is relatively easy for you while maintaining your commit contribution authorship. I apologize for any inconvenience caused by this shuffle, but please ping me if I can help in any way! 🚀 |
Some TypeScript weirdness means we were getting the GraphQLNamedType overload of
typeFromAST, which meant we were calling
.name
on it, which is undefined fornon-null and list types.