Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deliver: Compose user directives #1893

Closed
clenfest opened this issue May 26, 2022 · 3 comments
Closed

Deliver: Compose user directives #1893

clenfest opened this issue May 26, 2022 · 3 comments

Comments

@clenfest
Copy link
Contributor

We'd like to compose user directives in a standard way that mirrors what we've done in toAPISchema.

In general there are two cases we need to handle, executable directives which apply to queries and type system directives which apply to type system locations.

The rule for each is:

  1. all executable directives are (potentially) merged, but they are merged by "intersection" across subgraphs and that means that a given definition may not "make it" in the supergraph if definitions are too inconsistent between subgraphs. Because we don't know when the directive will be used in queries, we need to restrict the definition so that no query will be forced to deal with a directive it doesn't understand.
  2. For type system directives, it is safe to merge the locations by union since we know that any references to the directive will originate from the subgraphs themselves, and and a broader definition is not problematic.

If a directive definition has both type system and executable locations, composition will ignore the type system locations and merge the directive as an executable one, with only its executable locations

The default behavior should be to not compose user directives unless requested to during composition. Let's add an exposeDirectives argument to compose which takes a list of custom directives (we need to be able to support both core schema and regular directives). If a directive is present in the list, it will appear in the supergraph, otherwise not. Also, ensure that all directives specified as an argument are present in the schema of at least one subgraph to mitigate directive name typos.

There is probably work to be done in field merging as well to ensure that directives that are repeatable will get values from all subgraphs, and values that are not repeatable deterministically select a sane choice from one of the subgraphs.

@romanschejbal
Copy link

Hey @clenfest, I'm more than happy to hack and help with this one since we also need this functionality.

Please, let me know if there's anything I can do.

@clenfest
Copy link
Contributor Author

clenfest commented Jun 1, 2022

Hi @romanschejbal. Thanks for the offer, I've got a draft PR up that we'll kick around, but we're moving on this one.

@jeffjakub
Copy link
Contributor

Closing out this issue as it was released earlier this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants