CoerceInputValue()

[leebyron]

As an alternative to #1058 and #1178 - this is a more substantial overhaul of how input value coercion is defined in favor of a system of composed algorithms.

A quick summary of what this introduces:

• Introduces the concept of a coercion failure which has a similar "raised error" behavior, but is caught and translated into specific errors based on each callsite. That removes the awkwardness of "a request error is actually an execution error here".

• A new "Input Coercion" section which defines CoerceInputValue(). This is the point of reusability which handles nullability and then forks out to each specific input type.

• Algorithms for each kind of type's input coercion. Notably input objects and lists which have been under-defined (as an alt to [RFC] List coercion algorithm #1058).

• A validation rule for default variable values, which I believe exists in graphql.js but was missing in spec.

Open Qs:

• Is this the right place for the Input Coercion subsection?

• Have I missed anything?

[netlify]

✅ Deploy Preview for graphql-spec-draft ready!

Name	Link
🔨 Latest commit	e9a4ec2
🔍 Latest deploy log	https://app.netlify.com/projects/graphql-spec-draft/deploys/68689ba6d3b6c8000823d807
😎 Deploy Preview	https://deploy-preview-1179--graphql-spec-draft.netlify.app
📱 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 project configuration.

[benjie]

I really like the introduction of coercion failure. This is a solid change, and we should consider extracting it and fast-track merging it as an editorial change.

I like the more algorithmic approach to validation, I think it should add significant clarity.

However, I'm struggling to actually comprehend this as it currently stands - see my second comment for details.

(As an aside, I think that it might behoove us to add "pseudotypes" to our algorithms to make it much clearer exactly what the type of each variable in an algorithm is.)

[benjie]

I'm uncomfortable with how the CoerceInputValue algorithm seems to be fed values which are a mixture of literals (AST nodes) from the document (required for things inside of input objects/lists within value to be a "variable", right?) versus raw values from the client (required for its usage from CoerceVariableValues()). This feels like it is essentially mixing together GraphQL.js' parseLiteral and parseValue and not being very explicit about it?

If this is not the case, please could you explain further what it means for something within value to "be a variable" if it's also accepting raw values from CoerceVariableValues() which could be essentially anything (due to custom scalars) - or maybe point out where literals and values are handled separately and I've misread the algorithms.

[leebyron]

You are correct. The spec groups literal values and externally provided values under the same umbrella when describing input coercion.

I share the unease around this, and maybe part of what we need is a better name for value though to be fair we already have this "variable is a kind of value" here https://spec.graphql.org/draft/#Value

It's true that we are quite hand-wavy about how externally provided values are handled, and some of that is necessary because we need to support different runtimes and serialization. Perhaps we need some description in this "input coercion" section (or the variables coercion section?) where we explain how we expect a GraphQL service to provide a way to represent their internal values as those GraphQL expects.

An example were we do spell some detail here is enums https://spec.graphql.org/draft/#sec-Enums.Input-Coercion where we need to answer the question of whether an enum should input coerce a string? The answer is only if there isn't a better option. GraphQL literals has a better option, EDN has a better option, JSON does not.

[benjie]

Noting this does not (yet) handle oneOf, so we'll need to decide which order we are merging these. Rewriting for oneOf may require a bit of refactoring due to the way ResolveVariable works.

[leebyron]

oneof is, barring last few q's on the thread, ready to go - so I assume I'd be rebasing over it.

they are essentially different types, so I was thinking something like forking based on a directive conditional check to a CoerceInputOneOfValue() algo

[benjie]

The JSON scalar, discouraged though it may be, requires access to variableValues.

I'm okay with omitting this from the spec, but it's worth noting that GraphQL.js does pass variableValues to parseLiteral/parseValue.

[benjie]

See also:

• Using variables in scalars #1011
• Clarify validation of custom scalar literals #1156

[leebyron]

yeah this is a reasonable compliance question - is it valid to consider variable values during scalar input coercion? I think it's okay to say yes, though I agree with you that this is a bit weird, it doesn't seem harmful.

The one consideration here is that we'd want to make sure we actually reference variableValues in the prose, I think we'd do that sort of like `Return the result of calling the internal method provided by the type system for determining the “input coercion” of {scalarType} given the value {value} (which may consider {variableValues})."

[martinbonnin]

+1 to taking variableValues into account: CoerceScalarValue(scalarType, value, variableValues)

It's late here and I don't have all the context any more but we might even want coercedVariableValues?

[leebyron]

wordy but more accurate :)

[benjie]

Suggested change

	- If {coercedVariableValues} has an entry named {variableName}, let
	- If {coercedVariableValues} exists and has an entry named {variableName}, let

[leebyron]

I think we could make this an assertion, right? The only time it doesn't exist is when we're coercing constant literals which do not have variables in them

[benjie]

This may be different to your intention, but I figure we call this from validation too (e.g. in “values of correct type”) where variable values are unknown (null) so we need to assume they’re valid (by “all variables usages are allowed”) and thus we simply don’t access the variable when variableValues is explicitly null (which implies it is being called during validation?)