Schema Coordinates

[leebyron]

Depends on #3115

Implements https://github.com/graphql/graphql-spec/pull/794/

Adds:

• DOT punctuator in lexer
• Improvements to lexer errors around misuse of .
• Minor improvement to parser core which simplified this addition
• SchemaCoordinate node and isSchemaCoodinate() predicate
• Support in print() and visit()
• Added function parseSchemaCoordinate() since it is a parser entry point.
• Added function resolveSchemaCoordinate() and resolveASTSchemeCoordinate() which implement the semantics (name mirrored from buildASTSchema) as well as the return type GraphQLSchemaElement

[leebyron]

Great feedback - I think I've resolved both:

• Replaced fieldName with memberName in the AST to ensure it's generalized between fields, input fields, and enum values. Those are all specific instances of the general pattern of "members of a set", which is pretty common terminology for this generalized dot access pattern.
• Took both of your great suggestions for wrapping the return type of resolveSchemaCoordinate - the ResolvedSchemaElement type is now a union of wrappers. This had the added benefits of disambiguating between "field argument" and "directive argument" and including the coordinate context in the return value

[benjie]

I've merged the latest main into this branch and got it passing tests/etc again; am adding it to the next GraphQL WG.

[magicmark]

@benjie I don't have write access to this PR since I wasn't the original author -- do you think we could merge in https://github.com/graphql/graphql-js/pull/4422 and then https://github.com/magicmark/graphql-js/pull/1 to collapse the stack for easier review?

(Alternatively, i'm happy to make a yet-another new PR to collapse everything, was trying to avoid making things even more confusing though.)

[martinbonnin]

do you think we could merge in https://github.com/graphql/graphql-js/pull/4422 and then https://github.com/magicmark/graphql-js/pull/1 to collapse the stack for easier review?

+1 on having a single PR for all changes.

[benjie]

@magicmark let's merge https://github.com/magicmark/graphql-js/pull/1 into https://github.com/graphql/graphql-js/pull/4422 (you have to do that since you own that repository), then incorporate changes to support https://github.com/magicmark/graphql-spec/pull/1, then I can merge the whole lot into this PR assuming there's no pushback.

[yaacovCR]

Update

The 2nd member of this stack by @leebyron was #3145, with that PR adding a coordinate field to all schema elements, allowing library users to move from schema element => to coordinate (as opposed to this PR, which allows the reverse, from coordinate => to schema element. I have now updated #3808 to be rebased on top of this PR with this PR as the base. We can still merge it separately.

Point for discussion

Now that we have allowed meta-fields in this PR, the other PR #3145/#3808 demonstrates that the schema element <==> coordinate relationship will not allow cycles with meta-fields.

For example, coordinate SomeType.__typename will resolve to TypeNameMetaFieldDef by this PR, an instance of GraphQLField. In #3808, TypeNameMetaFieldDef has a "coordinate" property of <meta>.__typename, which is not a valid coordinate and cannot be resolved back to TypeNameMetaFieldDef. This also affects SchemaMetaFieldDef and TypeMetaFieldDef. The meta-fields are each defined once by the library and then accessed "magically" by schema.getField(...) -- rather than being artificially defined on every type (for __typename) or every root query type of every schema (for the __schema and __type).

To preserve the symmetry we have with non-meta-fields, we would have to make <meta>.__typename or something like it valid and resolve to TypeNameMetaFieldDef and make SomeType.__typename not resolve to TypeNameMetaFieldDef.

Other options would be to simply acknowledge this asymmetry:

• perhaps by renaming the coordinate property introduced by #3808 to something else
• perhaps by not including a coordinate property at all => schemaElement.toString() essentially holds that value anyway on the main branch after the work done in #4288)
• perhaps simply by documentation

[magicmark]

a bare <meta>.__typename isn't something that would/could exist in a GraphQL operation right? I can't think what use cases there would be to need to support that as something we'd want to parse as a schema coordinate. Therefore, your suggestion of "perhaps simply by documentation" (and possibly a runtime assertion if required) sg2m?

(I don't know if this blocks the spec, or if this is just an edge casey implementation detail)

[benjie]

@yaacovCR thanks for finding out why Lee explicitly forbade meta fields, even he couldn't remember why he did it!

So (pseudocode):

// Get the Type.field GraphQLField instance:
const Type_field = schema.getType('Type').getFields()['field'];

// Schema coord should point to this field instance:
assert.equal(resolveSchemaCoordinate(schema, `Type.field`), Type_field);
// Field instance should have this field coordinate.
assert.equal(Type_field.getSchemaCoordinate(), 'Type.field');
// Works both ways ✅

// Resolving the schema coordinate for `RootQueryType.__schema` could point to our meta definition:
assert.equal(resolveSchemaCoordinate(schema, `RootQueryType.__schema`), SchemaMetaFieldDef);
// But from this global we don't have access to the schema so we can't determine the root operation type name!
SchemaMetaFieldDef.getSchemaCoordinate() // 💢 ERROR! 
// The `RootQueryType.__schema` field is actually "virtual", so we can only resolve it one way! No cycle!

I'm not too worried about the two-way lookup not working; however Foo.__typename is the exact same "virtual" field as Bar.__typename (resolveSchemaCoordinate('Foo.__typename') === resolveSchemaCoordinate('Bar.__typename')), which breaks the "every schema element should have exactly one coordinate" rule.

So we have a few options:

1. Forbid all meta fields (as Lee proposed)
2. Forbid __typename only, and create new instances of SchemaMetaFieldDef and TypeMetaFieldDef for each schema
3. Create new instances of SchemaMetaFieldDef and TypeMetaFieldDef for every schema, and a new instance of TypeNameMetaFieldDef for every object type in every schema
4. Break the cycle only for meta fields, and state in the spec that every non-metafield schema element should have exactly one coordinate

I'm leaning towards option 4.

[magicmark]

state in the spec that every non-metafield schema element should have exactly one coordinate

Done in https://github.com/graphql/graphql-spec/pull/794/commits/62ce19aafb0e0be105cd248e3e044d3f5103a897