fix list coercion examples

[jjergus]

I believe this example is inconsistent with this part of the spec:

If the value passed as an input to a list type is not a list and not the null value, then the result of input coercion is a list of size one, where the single item value is the result of input coercion for the list’s item type on the provided value (note this may apply recursively for nested lists).

Specifically the part about how "this may apply recursively for nested lists" is applicable here.

[IvanGoncharov]

Both above cases are correct as is since this coercion applied only if:
http://facebook.github.io/graphql/June2018/#sec-Type-System.List

If the value passed as an input to a list type is NOT A LIST

[IvanGoncharov]

Fix already proposed in:
https://github.com/facebook/graphql/pull/493/files

[jjergus]

The reference implementation seems to be consistent with my interpretation: graphql/graphql-js#1528

I don't think it would make sense any other way. If we accept that 1 coerces to [1], 2 coerces to [2], etc. (which seems to be clear), then why would that coercion behave differently if that 1 and 2 is nested inside another list? That would be a very confusing and unexpected implementation.

[IvanGoncharov]

@leebyron Can you please comment on whenever this limitation was intentional?
Or it's just a mistake in the example table?

@jjergus My assumption here: is that spec trying to be conservative and introduce complexity only if it provides significant benefits.
The provided use case is fully covered by current behavior:

This allows inputs which accept one or many arguments (sometimes referred to as “var args”) to declare their input type as a list while for the common case of a single value, a client can just pass that value directly rather than constructing the list.

[IvanGoncharov]

@jjergus I did a small research and this was intentional change, see here: 007a93e
and motivated by #372

I don't think it would make sense any other way. If we accept that 1 coerces to [1], 2 coerces to [2], etc. (which seems to be clear), then why would that coercion behave differently if that 1 and 2 is nested inside another list? That would be a very confusing and unexpected implementation.

After rereading spec text I figure out why 1 => [[1]] is correct but [1, 2, 3] is not. 1 is meet initial requirements:

If the value passed as an input to a list type is not a list and not the null value

And coerce according to next rule:

list of size one, where the single item value is the result of input coercion for the list’s item type on the provided value

So 1 is first coerced as [Int] and only then wrapped in array.

On the other hand [1, 2, 3] is falling is not a list check.

I'm closing issue since examples are fully in sync with specification text.
That said if you want to change current please feel free to open a separate issue with proposed change and we can start RFC process.

[jjergus]

The reference implementation is consistent with my interpretation (see the test cases I added in graphql/graphql-js#1528). Do you think the reference implementation is wrong and should be changed?

[IvanGoncharov]

Do you think the reference implementation is wrong and should be changed?

@jjergus Yes, I think so even though it's actually harder to implement coercion according to the current specification.
As June 2018 was a first stable release graphql-js and other implementations need some time to catch up.
Do you want to open an issue or work on a PR?

---

I don't know @leebyron original intention but bellow is my best guess.

If we start to coerce arrays it will create ambiguity since you can use two strategies:

• Deep first coercion [1, 2] => [[1, 2]]
• Root first coercion [1, 2] => [[1], [2]]

And both strategies are logical and have their pros & cons.

On the other hand, they produce exactly the same result on scalars, e.g. 1 => [[1]].

[OlegIlyenko]

This is quite interesting issue. I got curious and also tested it in sangria. Turns out it behaves in the same way as reference implementation. This implies that it violates the spec example: [[Int]] | [1, 2, 3] | Error: Incorrect item value.

After thinking about it for some time, I would agree with @jjergus original proposal for changing the example and allow coercion: [[Int]] | [1, 2, 3] | [[1], [2], [3]].

---

Here is my interpretation:

Gven following 2 rules for list type input coercion from the spec:

1. When expected as an input, list values are accepted only when each item in the list can be accepted by the list’s item type.
2. If the value passed as an input to a list type is not a list and not the null value, then the result of input coercion is a list of size one, where the single item value is the result of input coercion for the list’s item type on the provided value (note this may apply recursively for nested lists).

I assume:

• "input" = an input value independent of the context. it might be the full value provided for an argument or nested somewhere inside of other input value. (the coercion is recursive)
• "[value] can be accepted by [type]" = according to the type's coercion rules, the value is correct.

When I coerce input value [1, [2]] with expected type [[Int]]:

(step 1): For value [....] and type [...] Rule 1 might apply if I can also verify coercion rules for all list items (steps 2 and 3)
(step 2): For value 1 and type [Int] Rule 1 does not apply, but Rule 2 does apply, thus the value is valid and must be coerced to [1]
(step 3): For value [2] and type [Int] Rule 1 does apply, thus the value is valid and must be coerced to [1]. Rule 2 is irrelevant in this case.
(step 4): Coming back to the original input value [....]. We just proved that all list items are valid according to the item type coercion rules and coerced item values are [1] and [2]. Thus Rule 1 does apply and coerced value is [[1], [2]]. Rule 2 is irrelevant here.

---

So my conclusion: reference implementation behaviour is compliant with the spec, but example ([[Int]] | [1, 2, 3] | Error: Incorrect item value) in the spec itself is incorrect and needs to be updated (with the proposed change).

If we will conclude that [[Int]] | [1, 2, 3] | Error: Incorrect item value example is correct, then I would understand it as a spec change, so we would also need to update the spec text.

Have I missed something? The interpretation is a bit tricky :)

[OlegIlyenko]

If we start to coerce arrays it will create ambiguity since you can use two strategies:

• Deep first coercion [1, 2] => [[1, 2]]
• Root first coercion [1, 2] => [[1], [2]]

@IvanGoncharov I also wanted to point out that according to the current list input coercion rules (and my interpretation of these rules), the only valid coerced value in this example is [[1], [2]], so it is unambiguous. I'm not sure how value [[1, 2]] might be inferred based on these rules. Could you please describe it in more detail?

[IvanGoncharov]

@jjergus @OlegIlyenko Just to clarify both the rule 2. and the example were added by a single person inside a single and pretty small commit. So I would assume this particular example was intentional.

But since we discussing it for so long it means that it wasn't written clearly enough.
@leebyron Can you please clarify this example and intention behind it?

[[Int]] | [1, 2, 3] | Error: Incorrect item value

---

"input" = an input value independent of the context. it might be the full value provided for an argument or nested somewhere inside of other input value. (the coercion is recursive)

This is a terminology problem since we don't have the clear definition.
Based on the provided example I would assume that rule 2. is intended to be used only on a full value and rule 1. should be read as:

When expected as an input, list values are accepted only when each item in the list can be accepted AS AN INPUT by the list’s item type.

@OlegIlyenko Thanks for suggesting better terminology, we should definitely use terms like the full value provided for an argument and nested value instead of input after we finally clarify what input means in this context.

I also wanted to point out that according to the current list input coercion rules (and my interpretation of these rules), the only valid coerced value in this example is [[1], [2]], so it is unambiguous. I'm not sure how value [[1, 2]] might be inferred based on these rules. Could you please describe it in more detail?

You are completely correct about what would be the correct behavior according to the spec assuming such coercion is allowed.
Now I see that ambiguity wasn't the right word and I didn't explain my thought very well.

What I meant is that if GraphQL user sees 1 being coerced to [[1]] he can assume (without reading the spec) that [1, 2] can be coerced both ways and be surprised if the result doesn't match his expectation.

Note: As I said in my previous comment I assume this PR is about clarifying specification to better explain original intention and not an RFC for a proposed change. So I just wrote my best guess on why [1, 2] coercion was explicitly forbidden and only Lee knows the real reason.

So I propose to keep this PR open until we get necessary clarifications from Lee.

[benjie]

This may have been closed in error due to deletion of master branch, however I cannot reopen it on mobile. Cc @IvanGoncharov

[ilslv]

@IvanGoncharov @leebyron I've encountered the same issue, while covering list input coercion in juniper. Currently implementation recursively coerces [[Int]] | [1, 2, 3] | [[1], [2], [3]]. Should this be changed to an error or this is an ambiguity in GraphQL spec? Because I totally agree with arguments made by @jjergus and current juniper implementation makes the most sense to me.

[benjie]

Someone should add this topic to the next GraphQL Working Group, this seems to be an inconsistency between the spec and the reference implementation and should be solved one way or the other: https://github.com/graphql/graphql-wg/blob/main/agendas/2022/2022-01-06.md

@ilslv Would you like to do so?