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

Schema Coordinates

Open magicmark opened this issue 4 years ago β€’ 18 comments

πŸ‘‹

I've pulled out the proposed spec edits for Schema Coordinates (issue: https://github.com/graphql/graphql-spec/issues/735) into this PR.

(The RFC now lives in the original PR: https://github.com/graphql/graphql-spec/pull/746/files)

@leebyron you mentioned it might be possible to simplify the grammar - I played around a bit since the original PR, but I think I need some help here. What did you have in mind?

As suggested, tagging @eapache @mjmahone as additional reviewers as we had some good conversation in last WG meeting about this.

Thanks!

magicmark avatar Nov 18 '20 07:11 magicmark

Oh also, here's another option I had for the examples table, was thinking it might be good to explicitly show all the different kinds of schema you can select:

Screen Shot 2020-11-17 at 10 38 01 PM

felt slightly too verbose to me, but curious to see if anyone prefers this

magicmark avatar Nov 18 '20 07:11 magicmark

At Indeed we log usage of schema elements to 1) support deprecation and 2) understand usage from a product management perspective (i.e. is this API we built adding value?).

We defined our own coordinate system that is not quite as complete as this spec. A nice benefit of formalizing a coordinate system is that shared tools can be built to solve these and other use cases.

πŸ‘ πŸ‘ πŸ‘

mcohen75 avatar Feb 04 '21 18:02 mcohen75

I made some fairly significant edits to the prose and examples section in an attempt to simplify. @magicmark please take a look and let me know what you think.

leebyron avatar Apr 14 '21 01:04 leebyron

IIRC the only thing holding this from Approval stage is a landed GraphQL.js PR?

leebyron avatar Apr 14 '21 21:04 leebyron

I think this is good to go again. I've just added the GraphQL.js implementation which greatly helped in completeness checking this spec change

leebyron avatar Apr 16 '21 09:04 leebyron

Looks like the negative lookahead for . isn't rendering exactly right. I'll need to do a fix on spec-md before merging

leebyron avatar Apr 16 '21 09:04 leebyron

It’s meant to only apply to the dot

leebyron avatar Apr 16 '21 15:04 leebyron

It’s meant to only apply to the dot

I had a quick read of the relevant parts of the spec and they didn't clarify that interpretation for me. There's doesn't seem to be any other examples of lookaheads applying to one of inline lists. We should try and remove the ambiguity here; I don't see any ambiguity in any of the other lookahead references.

Aside: it seems that spec-md is adding the lookahead text via ::before which means Chrome can't search for it in the productions; I had to look at the raw markdown.

benjie avatar Apr 16 '21 15:04 benjie

I think I was trying to over-simplify by keeping this all in the one of and that's falling short on clarity. Here's an interesting existing case I'll try to model https://tc39.es/ecma262/#prod-Punctuator

leebyron avatar Apr 16 '21 16:04 leebyron

At this point I think we're good to RFC 3 approve - I'd like to wait on a final OK from @magicmark and at least a first review from @IvanGoncharov on the reference impl changes (ideally a merge)

leebyron avatar Apr 16 '21 16:04 leebyron

Regarding input arguments, the example is shown as:

Query.searchBusiness(name:)

What would it look like for more interesting sub-input-types on an argument? Example:

input BazInput {
  count: Int
  color: String
}
input BarInput {
  baz_settings: BazInput
}
input FooInput {
  bar_params: BarInput
}

type Query {
  foos(foo_params: FooInput) 
}

Would the above translate to the following coordinates?

Query.foos(foo_params:)
Query.foos(foo_params:bar_params)
Query.foos(foo_params:bar_params:baz_settings:)
Query.foos(foo_params:bar_params:baz_settings:count:)
Query.foos(foo_params:bar_params:baz_settings:color:)

schenkman avatar Oct 12 '22 17:10 schenkman

@schenkman

This would be:

Query.foos(foo_params:) => Query.foos(foo_params:) Query.foos(foo_params:bar_params) => FooInput.bar_params Query.foos(foo_params:bar_params:baz_settings:) => BarInput.baz_settings Query.foos(foo_params:bar_params:baz_settings:count:) => BazInput.count

PascalSenn avatar Oct 13 '22 05:10 PascalSenn

Indeed, each schema entity should have exactly one schema coordinate. If you want to identify those kinds of paths in an operation (rather than the schema), then operation expressions might be what you’re after: https://github.com/graphql/graphql-wg/blob/main/rfcs/OperationExpressions.md#arguments

benjie avatar Oct 13 '22 06:10 benjie

@PascalSenn @benjie Trying to understand the last two comments:

This would be: Query.foos(foo_params:bar_params) => FooInput.bar_params

Q1 - "This would be" - intended to mean this is direction the spec will go as it's not covered in this PR?

I assume it would also apply to directives, so with @A(foo_params: FooInput) then the coordinate for bar_params would be:

@A(foo_params:bar_params)

I saw the comment here: https://github.com/graphql/graphql-wg/blob/main/rfcs/SchemaCoordinates.md?plain=1#L370-L375 but it throws me a little as it talks about supporting more than "schema coordinates" but then the first example seems to be a schema coordinate Foo.bar(baz.qux:)

Also note the example Foo.bar(baz.qux:) is different to the comment above, which would have it as Foo.bar(baz:qux) but the latter seems to make more sense.

ddebrunner avatar Dec 04 '23 17:12 ddebrunner

@ddebrunner Schema Coordinates and Operation Expressions are two separate proposals with related syntax.

Every entity in a schema has exactly one schema coordinate, there is no schema coordinate @A(foo_params:bar_params:), at least not as currently proposed.

The property bar_params on the type FooInput has the schema coordinate FooInput.bar_params, independent of whether it's accessed through a directive (@A(foo_params:{bar_params:...})) or a field (myField(foo_params:{bar_params:...})).

You can, however, use Operation Expressions to indicate the accessing of something in this manner, since Operation Expressions are not unique.

The comment you quote relates to (what became) Operation Expressions, which is a system that builds upon the Schema Coordinates syntax to talk about paths within operations (rather than the schema).

benjie avatar Dec 04 '23 23:12 benjie

@benjie I'm not talking about operations, but schema elements. A type system directive is applied to a schema element and thus is also a schema element(?), so what are the coordinates to such arguments?

E.g.

input IO {
  a:IO2
}
input IO2 {
  b:Int
}
directive @x(v:IO)

type T {
   f:Int @x(v:{a:{b:3}})
}

Now if the value of b is invalid how would I report the coordinate of b in an error message. The comments related to this RFC seem to imply something like:

@x(v:a.b) on T.f

But I'm assuming that the use of type system directive@x is a schema element, and therefore so are its arguments. Maybe I'm missing something?

ddebrunner avatar Dec 05 '23 13:12 ddebrunner

I don't think the application of a directive is covered by this proposal, only the directive itself. At runtime your @x application on T.f no longer exists - e.g. if you introspect that schema or use graphql.printSchema(schema) you'll not see it - it's a build-time-only directive currently. (See #300 for more details.)

You could use operation expressions to indicate the path of this directive application, I guess it's more "document expressions" than "operation expressions" thinking about it.

benjie avatar Dec 05 '23 13:12 benjie

Fair enough, but I hadn't assumed that the RFC was related to runtime, thus the fact that introspection doesn't return type system directives would seem to be irrelevant to the RFC.

ddebrunner avatar Dec 05 '23 17:12 ddebrunner