Move the key arg of `@join__type` into its own `@join__key` directive.

[pcmanus]

https://github.com/apollographql/specs-join/pull/14 adds @join__implements and @martijnwalraven mentioned how having it is slightly inconsistent with the fact key is an argument of @join__type (in the sense that key and implements share that only object/interface have them and a given type can have more then one of them).

One way to fix the inconsistency would be to move 'implements' as an argument of @join__type, but I'd like to suggest the other way around instead: to remove the key arg of @join__type and have a @join__key instead. I think that, in the relaxed model, this tidy up the spec a bit more in that:

• @join_type becomes a directive whose only job is to indicate that a give subgraph declare that type, and this for all types. Having key and/or implements muddies the directive purpose a bit in that case (including the fact they only apply to some types).
• this might evolve a tad more gracefully in that if we later support more options for @key and we need those to be added to the join spec, adding them as argument of a @join__key directive is going to be a tad cleaner than needing a new key_something argument to @join__type.

[pcmanus]

It occurred to me that, if we do so, one might ask why not also move provides and requires into specific join directives? I think it make sense to do so as the arguments above apply fairly directly to @join__field.

[queerviolet]

i actually just commented that perhaps we should move interfaces and unions into @join__type.

you can always normalize directives, but i'm not sure it's great to do so. detaching @provides from @join__resolve, for example, would let you express this:

type X {
  y: Int @join__resolve(graph: alpha) @join__provides(graph: beta, fields: "{ x y }") @join__provides(graph: alpha, fields: "{ a }") @join__provides(graph: alpha, fields: "{ a b c }")
}

do we… merge those selection sets? express a validation rule which this breaks?

similarly for @join__key… why? it does make the directives smaller, which is nice, but it also scatters the information the query planner needs over multiple directives, which is unfortunate. part of the goal here is collecting all that, for human and machine consumption.

that said, we might break out @key for different reasons—if we implement generic flags which let us mark the subgraph for a @key directive (would look something like @key(fields: "...", join__source: someSubgraph), then there would be no reason not to simply pass through the subgraph key declarations (appropriately flagged as coming from that subgraph). the join spec could reference the same key spec as the rest of federation, which would be a considerable simplification, probably worth the scattering. and at that point we could consider doing something similar with @requires and @provides

[pcmanus]

do we… merge those selection sets? express a validation rule which this breaks?

Well, insofar as we need to make @join__field repeatable (since we have no field ownership anymore), even if we keep provides as an argument, we still have the exact same problem with:

type X {
  y: Int @join__field(graph: alpha, provides: "{ a }") @join__field(graph: beta, provides: "{ x y }") @join__field(graph: alpha, provides: "{ a b c}")
}

So I don't think we can get away without specifying that some directives are repeatable but must only appear once for each value of the graph argument. Which doesn't feel like a big deal to me as it's a fairly simple and understandable rule.

Somewhat relatedly, we would kind of want to use that same rule for @join__type, but we actually cannot because you can have multiple keys in a subgraphs. Add to that the fact that key is optional, and now we have to say if:

type X
  @join__type(graph: alpha)
  @join__type(graph: alpha, key: "k1")
  @join__type(graph: alpha, key: "k2") {
...
}

and

type X
  @join__type(graph: alpha, key: "k1")
  @join__type(graph: alpha, key: "k2") {
...
}

are both valid or not. So the idea of separating things here was to make it clear the "only" valid version is:

type X
  @join__type(graph: alpha)
  @join__key(graph: alpha, fields: "k1")
  @join__key(graph: alpha, fields: "k2") {
...
}

with each directive having a simple single meaning:

• @join__type is just about indicating where the type is defined. This exists for both entity and non-entity types.
• @join__key declares a key.

But to be fair, an alternative could be to make the key argument of @join__type a list, so that the "only" valid version would be:

type X @join__type(graph: alpha, keys: ["k1", "k2"]) {
  ...
}

and I'm not strongly opposed to that.

But to sum up, I feel separating 'key', 'provides', etc.. into directives instead of arguments:

• make each directive being more targeted; each do only one simple thing. Feels like a good design principle.
• is more likely to evolve cleanly (in a backward compatible way). Again, because if we need to keep some new info for a 'key' or a 'provide' tomorrow, adding a new argument to the dedicated @join__key or @join__provides is convenient.
• does make things more verbose. It's certainly a downside. I suppose I'm just not convinced it's a super important one.
• does slightly scatter the info, but insofar as everything is still attached to a given element, I've a hard time imagining this making a meaningful difference in practice (for either machine or human).

With that, I think I still have a small preference to separating. But not enough to insist if others feel strongly about going the other route (of cramming more into arguments).

@martijnwalraven: do you happen to have an opinion here?

[queerviolet]

i've thought about it, and i think i broadly agree. and actually, i think we can go a step further and make these their own independent features. @key has perfectly reasonable semantics outside of join—it says these fields are semantically a key for this type.

the join spec was the first spec written, and we didn't really know what features or core docs would look like. i think we have a lot more comfort with core directives now and a better sense of how we can link multiple specs together. i'll work up a pr with these thoughts!

[queerviolet]

that said, i think the process i'd like is to make some minimal changes with only what's needed to support the relaxed query planner for 0.2 (for pre/alpha), and work towards a bigger revamp for 0.3? there's a little bit of tooling i'd like to have, particularly around having different specs link together

[pcmanus]

make some minimal changes with only what's needed to support the relaxed query planner for 0.2

I definitively don't want to spend too much time on this in the short term given current time constraints. But experience also taught me that it's always hard to predict what your priorities will be in the future, and so I'm ok with spending just a little more time making 0.2 consistent-ish, just in case its shelf life is longer than expected. So either move @key, @provides and @requires as their own directives, or move join__implements as an argument, but avoid the current mix of both, neither of which feels much effort (both on changing the spec and updating the related code; splitting the spec does feel quite a bit more work on that front).

[queerviolet]

yeah, my preference would be to move @join__implements to be an argument of @join__type. for now.

longer-term, we'd like to explore supporting a join model where you do not have to be joining types from distinct subgraphs, but can e.g. join types from the same subgraph or even the supergraph itself. we might model such a thing like so (sketchy concept):

enum join__Graph @core__values {
  admin @id(url: "...")
  auth @id(url: "...")
}

type User
  @join(to: auth__User, requires: "{ id }")
  @join(to: admin__User, requires: "{ id }")
{
  id: ID! @join(to: "auth__User.id")
  name: String!
  token: Token!
  roles: [String!] @join(to: "admin__User.roles", requires: "{ token }")
}

type admin__User @external(join__graph: admin) {
  id: ID!
  roles: [String!]
}

type auth__User @external(join__graph: auth) {
  id: ID!
  name: String!
  token: Token!
}

(the @external markings are not strictly necessary, as the definitions are namespaced but we may want to require them for explicitness)

a model like this would simplify our story for including the signatures of subgraph fields and simplify the join spec overall. but it would make it difficult to key on graph, since graph is no longer an explicit argument nor expected to necessarily be unique.