Support non-list variables for list arguments

[Shane32]

See this feature request:

• [Feature request]: Support non-list variables for list arguments #1033

[netlify]

✅ Deploy Preview for graphql-spec-draft ready!

Name	Link
🔨 Latest commit	3dfc15c
🔍 Latest deploy log	https://app.netlify.com/sites/graphql-spec-draft/deploys/64ee5a0232c6af000800db13
😎 Deploy Preview	https://deploy-preview-1043--graphql-spec-draft.netlify.app/draft
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[Shane32]

Example stepping through logic for AreTypesCompatible with new steps when comparing list-type locations with non-list type variables:

Step	Scenario 1	Scenario 2	Scenario 3	Scenario 4	Scenario 5
variableType	String	String!	String	String!	String
locationType	[String!]	[String!]	[String]	[String]	[String]!
1 Is locationType is non-null type	false	false	false	false	true
1a If variableType is NOT a non-null type					true - return false
2 if variableType is a non-null type	false	true	false	true
2a let nullableVariableType...		String		String
2b AreTypesCompatible		String, [String!]		String, [String]
3 if locationType is a list type	true		true
3a let itemLocationType	String!		String
3b If variableType is NOT a list type	true		true
3b1 If itemLocationType is a non-null type	true		false
3b1a Let nullableItemLocationType...	String
3b1b AreTypesCompatible	String, String
3b2 AreTypesCompatible			String, String

[Shane32]

Note that no rules have changed for how default values for variables work. Since all existing nullability checks apply to the outermost layer, all are still valid with these new rules, in line with 'option 5' outlined in #1033.

[Shane32]

Note: this is simpler to write and equally as effective (and potentially more understandable):

Suggested change

	- If {itemLocationType} is a non-null type:
	- Let {nullableItemLocationType} be the unwrapped nullable type of {itemLocationType}.
	- Return {AreTypesCompatible(variableType, nullableItemLocationType)}.
	- Otherwise, return {AreTypesCompatible(variableType, itemLocationType)}.
	- If {itemLocationType} is a non-null type:
	- Let {nonNullVariableType} be the non-null type of {variableType}.
	- Return AreTypesCompatible(nonNullVariableType, itemLocationType).

However, all the rest of the logic favors unwrapping types, rather than wrapping types. So with a single extra line in the logic here, we are only unwrapping types, and never wrapping types.

[Shane32]

GraphQL.NET's implementation of this is here, locked behind an experimental option flag:

• Allow nullable variables for non-null arguments that have defaults; allow scalar variable to list types graphql-dotnet/graphql-dotnet#3662

[benjie]

If you haven't already, please consider adding this to an upcoming GraphQL Working Group. Let me know if you need any guidance.

https://github.com/graphql/graphql-wg/tree/main/agendas/2023

[benjie]

Oops, just saw #1033 (comment)

[benjie]

This change alone is not sufficient; the behavior must also be adopted in CoerceArgumentValues:

https://spec.graphql.org/draft/#sec-Coercing-Field-Arguments

Specifically 5.f.iii.

3. Let value be the value provided in variableValues for the name variableName.

Would need to become something like:

3. Let {variableValue} be the value provided in {variableValues} for the name {variableName}.
4. If {hasValue} is {false}, or {variableValue} is {null}, or {variableValue} is a list, or {argumentType} is neither a list type nor a non-null list type, or {variableType} is either a list type or a non-null list type, then:
   1. Let {value} be {variableValue}.
5. Otherwise:
   1. Let {value} be a list containing the single value {variableValue}.

Further we'd need to change logic in coercion too, for which it would be wise to land #1058 first.

[benjie]

I think we'd wrap all of that above in its own algorithm, something like:

3. Let {value} be {CoerceVariableValue(variableName, argumentType)}.

[benjie]

Before working any more on this, I recommend that we land:

• Fix CoerceArgumentValues() hasValue #1056
• Fix coercion table for list #1057
• [RFC] List coercion algorithm #1058

I then have edits in mind that I think would make this a clearer win.

[Shane32]

@benjie Any update on this?