apollographql / federation

🌐  Build and scale a single data graph across multiple services with Apollo's federation gateway.
https://apollographql.com/docs/federation/
Other
659 stars 248 forks source link

`@composeDirective` silently erases executable directives #2334

Open dbanty opened 1 year ago

dbanty commented 1 year ago

If you use @composeDirective with an executable directive, like this:

extend schema
    @link(
        url: "https://specs.apollo.dev/federation/v2.1",
        import: ["@composeDirective"]
        )
    @link(
        url: "https://specs.apollo.dev/customDirectives/v1.0",
        import: ["@lowercase"]
    )
    @composeDirective(name: "@lowercase")

directive @lowercase on FIELD

The directive will be silently removed from composition. Composition should either still include executable directives (so @composeDirective has no effect) or error to alert the user that they did the wrong thing.

In the meantime, I have opened a PR to warn about not using @composeDirective with executable directives: https://github.com/apollographql/federation/pull/2333

smyrick commented 6 months ago

Interesting caveat, while executable directives do not work with @composeDirective they can still end up in the supergraph as long as the directive definition is exactly the same across all subgraphs.

Subgraph 1

extend schema
  @link(url: "https://specs.apollo.dev/federation/v2.1")

directive @timeout(value: Int) on FIELD

type Query {
  foo: String
}

Subgraph 2

extend schema
  @link(url: "https://specs.apollo.dev/federation/v2.1")

directive @timeout(value: Int) on FIELD

type Query {
  bar: String
}

This does actually move the schema definition into the supergraph and it can be added to operations

query {
  foo @timeout(value: 5)
  bar @timeout(value: 5)
}

If however you do not have the EXACT same definition across all subgraphs you will get the following hint, but not error, and the directive will no longer exist in the supergraph:

HINT: [INCONSISTENT_EXECUTABLE_DIRECTIVE_PRESENCE]: Executable directive "@timeout" will not be part of the supergraph as it does not appear in all subgraphs: it is defined in subgraph "locations" but not in subgraph "reviews".

This is intentional so that you can incrementally add a new directive to subgraphs and then once all subgraphs agree it becomes available in the subgraph. However this could cause an issue if you already have a executable directive existing in the supergraph and a new subgraph comes along and forgets to add it, it will get removed for everyone without any composition errors, just hints.

Today you would have to catch this with the GraphOS linter checks and fail on INCONSISTENT_EXECUTABLE_DIRECTIVE_PRESENCE but that then prevents the incremental adoption of new directives too.


Some possible fixes