Skip to content

Remove wrong Input Object coercion example #311

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Remove wrong Input Object coercion example #311

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion spec/Section 3 -- Type System.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -837,7 +837,6 @@ input ExampleInputObject {
Original Value | Variables | Coerced Value
----------------------- | --------------- | -----------------------------------
`{ a: "abc", b: 123 }` | {null} | `{ a: "abc", b: 123 }`
`{ a: 123, b: "123" }` | {null} | `{ a: "123", b: 123 }`
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of removing it, can you update it to show that this produces an Error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well .... the error is an error saying that the type is wrong. I don't think this has anything to do with input coercion. That's the reason I decided to remove it.

Copy link
Contributor

@wincent wincent May 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I think that distinction is a little too subtle. Whether coercion fails because you didn't provide a required value, or because you provided one of the wrong type, the user is going to see an error, and probably will care more about the fact that it didn't work than whether it is technically a "coercion" error. Also, note that even though "coercion" is narrowly defined in the spec, it also has a more general meaning of translation from one type to another which many readers will have in mind when they look at this table (and might also explain why this example was here in the first place, even if it was wrong).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the fundamental problem here is that this section Input coercion mixes both aspects: "Input as ast literal" and "Input as variables (maps)".

For example in the first paragraph it states for the "Input as variables" aspect:

This unordered map should not contain any entries with names not defined by a field of this input object type, otherwise an error should be thrown.

But it doesn't say anything about extra fields for literals, because this is handled in the validation section. (I assume that is the reason)

In the table it is unclear if the "Original value" is a literal or a Map or both:
For example { b: $var } can only be a literal, but the third entry { a: "abc" } could be both.
And the second entry {a: 123, b: "123" } is wrong if it is a literal, because the types are wrong.
BUT: in the reference implementation it is actually correct if {a: 123, b: "123" } is interpreted as variable input. Could that be the reason it is there? (But I am not sure the reference implementation is correct in this case)

`{ a: "abc" }` | {null} | Error: Missing required field {b}
`{ a: "abc", b: null }` | {null} | Error: {b} must be non-null.
`{ a: null, b: 1 }` | {null} | `{ a: null, b: 1 }`
Expand Down