-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[RFC] Tagged type #733
Closed
Closed
[RFC] Tagged type #733
Changes from all commits
Commits
Show all changes
45 commits
Select commit
Hold shift + click to select a range
3d43179
First pass
benjie d411b60
More edits
benjie 16de657
Input coercion
benjie a73d1cc
Change tagged "fields" to "members"
benjie ceb81d5
Merge branch 'master' into tagged-type
benjie e65126d
Sync type system
benjie a4c4578
Factor in review feedback from @spawnia
benjie c31ddc4
Merge branch 'master' into tagged-type
benjie 55feb1c
Move TaggedMemberDefinition
benjie f786f1c
Add TAGGED_MEMBER_DEFINITION directive location
benjie 5906cd0
Reorder so tagged types comes after interfaces/unions
benjie bd88e51
Edit out comma that snuck in
benjie 514bb72
Add word 'concrete'
benjie 4768d8d
Define member field
benjie 95ccf95
Add Lee's note
benjie 88f983b
Lee's rewording
benjie a6486ce
Add mutually exclusive tagged type example with distinct types
benjie cf8d7a6
Make it clear Tagged type fields can be of any type.
benjie a09b30a
s/objects/results
benjie c81ebb1
:
benjie 5dcb81b
Checking for one key is easier than validating the given keys (maybe)
benjie 71ca892
nit
benjie ce213bb
Allow @deprecated on TAGGED_MEMBER_DEFINITION
benjie 7ac7532
Reword note on tagged member deprecation
benjie 0ae9ba8
Add note that added members must not make the tagged type invalid
benjie 361a235
Fix case
benjie a1705b5
Remove duplicate
benjie 8807c1c
Reword to follow Lee's example
benjie 61b7b8f
Reposition TAGGED to always be between Union and Enum.
benjie 285673a
Terran -> Earthling
benjie d9820c4
Make header consistent
benjie f91d8ba
__Type represents all named types in the system
benjie 82151ba
Apply Lee's suggestion
benjie 02a2a93
Combine
benjie 5bdcc30
Use 'field' rather than 'key'
benjie 88e506a
GetTaggedMember[Field]Name
benjie fee5ca3
__[Tagged]Member[Field]
benjie 0f5d7cc
TAGGED_MEMBER_[FIELD_]DEFINITION
benjie 94a67b7
TaggedMember[Field]Definition/TaggedMember[Field]sDefinition
benjie f66e237
taggedMember[Field]Name
benjie e308258
Fix definition ordering
benjie 6fa34c2
Members -> member fields
benjie 4bf67b4
Grammar
benjie 5afea0b
Fix incorrect capital
benjie ced63be
Separate input and output tagged types
benjie File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Should we distinguish this from
FIELD_DEFINITION
andARGUMENT_DEFINITION
? I can't immediately think of a use case for a directive that would apply on normal fields or normal arguments, but would be incoherent on a tagged member field?If we don't define it specially, that weighs in on the input-vs-output question as well; for a tagged member field, which directives does it permit? Field directives or argument directives? If it's to be used in both input and output, it has to permit both; is that reasonable?
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.
FIELD_DEFINITION
,ARGUMENT_DEFINITION
andINPUT_FIELD_DEFINITION
have separate entries because they are separate things, tagged member fields are also separate to all of these, they're neither FIELD nor INPUT_FIELD, so I think it makes sense to have a separate directive location for them. As you rightly point out member fields are different because they could be for input or output, so it's clear that using directives targetted at fields or arguments may not make sense for tagged, so again differentiation is critical.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.