graphql-spec icon indicating copy to clipboard operation
graphql-spec copied to clipboard

Support non-list variables for list arguments

Open Shane32 opened this issue 2 years ago • 8 comments

See this feature request:

  • #1033

Shane32 avatar Aug 29 '23 18:08 Shane32

Deploy Preview for graphql-spec-draft ready!

Name Link
Latest commit 3dfc15c21d78534309165dba0bffe5e5215a6729
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...

QR Code

Use your smartphone camera to open QR code link.

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

netlify[bot] avatar Aug 29 '23 18:08 netlify[bot]

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 avatar Aug 29 '23 18:08 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 avatar Aug 29 '23 18:08 Shane32

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

  • https://github.com/graphql-dotnet/graphql-dotnet/pull/3662

Shane32 avatar Oct 16 '23 00:10 Shane32

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 avatar Oct 16 '23 12:10 benjie

Oops, just saw https://github.com/graphql/graphql-spec/issues/1033#issuecomment-1763566528

benjie avatar Oct 16 '23 12:10 benjie

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

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

benjie avatar Nov 10 '23 09:11 benjie

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

  • https://github.com/graphql/graphql-spec/pull/1056
  • https://github.com/graphql/graphql-spec/pull/1057
  • https://github.com/graphql/graphql-spec/pull/1058

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

benjie avatar Nov 10 '23 10:11 benjie