Validation of directive argument values in schema is not quite spec compliant for lists

[pcmanus]

Currently, a schema (for say, subgraph) that looks like this:

directive @test(v: [[Int]]) on FIELD_DEFINITION

type Query {
    f: Int @test(v: [1, 2, 3])
}

is currently accepted by federation, but it's technically not completely graphQL compliant because while something like:

directive @test(v: [[Int]]) on FIELD_DEFINITION

type Query {
    f: Int @test(v: 1)
}

is valid, as 1 coerces to [[1]], this kind of coercion does apply if the initial value is a list (see the input coercions for lists).

Creating this ticket so there is a record for this, and this could be fixed relatively easily by modifying a bit the logic around here, but I'll add that:

1. this really on affect the validation of input values in subgraph, and thus only argument of directive applications within schema. Operations are otherwise validated by the gateway/router using different code (the gateway uses graphQL-js usual validations which handle this, and the router used to also use those validation through deno, though I think it switched to apollo-rs validations).
2. said validation of directive argument values within SDL has actually never been implemented by graphQL-js, the reference implementation (see add validation of directives in SDL graphql/graphql-js#1389), which actually does not validate those values at all (so the example above also validates with graphQL-js afaict). Note that federation does in general validate those values, because we believe it's more helpful (and as federation use directives in schema quite a bit, this felt like something worth doing), and so fixing this is more consistent than not, but I think this context helps put the urgency of this issue in perspective somewhat
3. fixing this validation theoretically means that some subgraphs that currently compose would start being rejected. I'm of the mind that it's not a reason to not fix this, but it could be taken into account when choosing which release get such a fix.

[sachindshinde]

I was curious about the input coercion rules for lists, and when looking at the relevant text in 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).

The way that I'd interpret "recursively" here is:

1. We're trying to coerce [1, 2, 3] to the type [[Int]].
2. [1, 2, 3] is already a list, so next (through recursion) we have to coerce 1 to [Int], 2 to [Int], and 3 to [Int].
3. Since those aren't lists, the paragraph above applies, coercing 1, 2, and 3 into [1], [2], and [3].
4. This makes the coercion of [1, 2, 3] be [[1], [2], [3]].

I spun up an Apollo Server CodeSandbox to see how graphql-js coerces lists for operations, and it indeed coerces [1, 2, 3] of type [[Int]] to [[1], [2], [3]].

However, as you've noted, the 3rd entry down in the examples list in the spec says [1, 2, 3] should error when coerced to type [[Int]]. Spec folks are aware of the discrepancy between graphql-js and spec.

I talked to @pcmanus more, and we've decided to follow graphql-js's behavior for now, as it's the one most would expect (and graphql-js devs couldn't change it without breaking operation execution for many users).