Discuss the effect on public schema of @internal directive

[kamilkisiela]

In Federation v2, the @inaccessible directive (in our case it's called @internal) hides a field from the end consumer of GraphQL API.

# schema A

type User @key(fields: "id") {
  id: ID!
  secret: ID: @internal @shareable
}

# schema B

type User @key(fields: "id") {
  id: ID!
  secret: ID @shareable
}

What GraphQL API consumer sees:

type User {
  id: ID!
}

I would say this approach (hiding a field if it's marked as internal at least once) is a good practice as it reduces chances of leaking a field to the public.
The opposite approach would be to mark a field as internal in all subgraphs that define the field (easy to leak something).

It becomes problematic with combination of the @lookup directive.
I can imagine a situation where there are more than one lookup fields of the same name, and only the subset of them is meant to be used for querying data.

Here's what I mean:

# schema A

type Query {
  userById(id: ID!): User @lookup
}

# schema B

type Query {
  userById(id: ID!): User @lookup @internal
}

In this example, a team that develops schema B, may want to limit the usage of the field to make it available only for the query planner, specifically the entity resolution (internal request to resolve a type based on its key).

What are your opinions? Should we introduce two different behaviors for @internal with and without @lookup or stick to what we have today (At least one @internal to hide a field completely).

[martijnwalraven]

Hmmm, this is a crazy thought perhaps, but would it make sense to retain @inaccessible with the current behavior (marking a field @inaccessible in any subgraph will hide it from the public API) and add @internal to mean just hiding the field from the current subgraph?

[benjie]

I personally would push for consistency over ease, so I'd require that @internal occurred on every instance or none, and if it didn't I would explicitly throw a composition error.

[smyrick]

Is this an issue that can happen with @lookup or should we restrict define the EXACT same lookup resolver in multiple locations?

https://github.com/graphql/composite-schemas-spec/pull/30/files#diff-eb6987ee6c07e5df35ced48c4788775c2d9276557955481a9624b1917f521a36R18-R19

If we use the same name but different args that a different issue, but we could prevent this with look up checks.

---

As for having it existing in one location vs all, we needed to support @inaccessible hiding a field when only mentioned once for the upgrading flow.

Consider the following

I have an existing shared type

# Subgraph A
type Position @shareable {
  x: Float
  y: Float
}

# Subgraph B
type Position @shareable {
  x: Float
  y: Float
}

Now I want to add a new field Position.z. I am not going to be able to do that all at once so each subgraph is slowly going to have to update, even if it is back to back.

When I go and first update Subgraph A, should the change result in failed composition? Apollo Federation allows this today

Making a change to Subgraph A, even if I upgrade Subgraph B in the same hour, should the supergraph be broken in that time?

# Subgraph A
type Position @shareable {
  x: Float
  y: Float
  z: Float @inaccessible
}

# Subgraph B
type Position @shareable {
  x: Float
  y: Float
}

[benjie]

When I go and first update Subgraph A, should the change result in failed composition?

Thinking about it, this would require coordination so contrary to what I said before, the directive should be interpreted locally. I think @martijnwalraven's suggestion sounds reasonable:

foo: String @internal would mean "don't expose this field from here" (but it might be exposed by another subgraph)

foo: String @inaccessible (or maybe foo: String @internal(force: true) or similar) would force the field to never be exposed. For this one I would require that no other subgraph exposes it (i.e. each mention has either @internal or @inaccessible) and I would fail composition otherwise. I'd also ensure through tooling that if @inaccessible exists on one of the other subgraphs you're discouraged from deploying a breaking change here.