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

[Federation] Support value types #3063

Merged
merged 42 commits into from
Jul 28, 2019
Merged

[Federation] Support value types #3063

merged 42 commits into from
Jul 28, 2019

Conversation

trevor-scheer
Copy link
Member

This PR adds support and improved validation errors around "value types" in federation.

What are value types?

Value types are type definitions that live on multiple services (more specifically - types, inputs, interfaces, and unions). We've seen the desire internally and within the community to support the idea of a common type that multiple services can reference. A value type is identical by name, kind, and fields across all services.

Approach

In order to accomplish this, we've extended two of the existing validators: one for type names, and the other for field names. The changes to the validators permit duplicate type names and fields in the case that we've encountered a value type. The validators also provide some actionable, user-friendly errors in the case that we encounter a kind or field type mismatch.

Copy link
Contributor

@JakeDawkins JakeDawkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 Great work here! A Few comments, but don't be too worried, most are just nits and error message improvements!

Copy link
Member

@abernix abernix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a couple nitty comments that need not be circled back on. (Also, some worthy considerations from @JakeDawkins's review too.)

Fixed and added tests for edge cases
Only report field type diff errors if types have the same exact shape
Return more info from the type node diffing function.
Address issues that arose by implementing a more correct testing strategy.
Namely, we can't use typeFromAST on a fresh schema since it may not have
the types we're trying to reference. Revert back to using print() for now.
* Address issue that arose from the more correct testing strategy:
Duplicate types need to also be found in the typedefs, not just the schema.

* Add error codes to our error messages
Update unique field name validator to disregard entities -
this is handled in the unique type name validator.
* Remove extraneous "kind" validation in the field names validator
Copy link
Contributor

@JakeDawkins JakeDawkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎉

@abernix abernix merged commit cca20d9 into master Jul 28, 2019
@abernix abernix deleted the trevor/value-types branch July 28, 2019 21:05
@abernix abernix added this to the Release 2.8.x milestone Jul 28, 2019
abernix pushed a commit to apollographql/federation that referenced this pull request Sep 4, 2020
…-server#3063)

Value types are type definitions that live on multiple services (more specifically - types, inputs, interfaces, and unions). We've seen the desire internally and within the community to support the idea of a common type that multiple services can reference. A value type is identical by name, kind, and fields across all services.

In order to accomplish this, we've extended two of the existing validators: one for type names, and the other for field names. The changes to the validators permit duplicate type names and fields in the case that we've encountered a value type. The validators also provide some actionable, user-friendly errors in the case that we encounter a kind or field type mismatch.
Apollo-Orig-Commit-AS: apollographql/apollo-server@cca20d9
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 16, 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 this pull request may close these issues.

3 participants