Repository navigation
feat: add onConflict option for same-named types and root fields - #95
Open
Simon-Lind-glitch wants to merge 4 commits into
Open
Simon-Lind-glitch wants to merge 4 commits into
Simon-Lind-glitch wants to merge 4 commits into
Conversation
…nions Same-named types from different subgraphs were merged with no conflict detection: an enum kept the first subgraph's values, an object type got its field lists concatenated, and a Query or Mutation field published by several subgraphs kept the last subgraph's resolver. A call could then validate against one subgraph's enum and execute against another. - an enum is now the union of the values of every subgraph, each value keeping the description of the subgraph that declared it - object fields are merged by name, the first declaration wins - a shared Query/Mutation field is routed by an enum argument: it must be declared with the same enum type in every subgraph, as Enum or Enum!, with every value declared by exactly one subgraph; the value the client passes selects the subgraph - composition fails when a shared root field cannot be routed: no such argument, a value declared by two subgraphs, or more than one argument that could route it Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Adds `onConflict` to the composer options: "error" (default) fails composition on a shared Query or Mutation field, "first" and "last" pick that subgraph's resolver and enum values with a warning, and "route" merges the enum and routes each call by the enum argument. Routing now checks what a routed call relies on and fails composition otherwise: the subgraphs must agree on the return type, on every argument's type and on the defaults of the non-routing arguments; the routing argument must be non-null or have a default; and no subgraph outside the field's publishers may add values to the enum, since such a value would pass validation with nowhere to go. The shared-field check uses the root type names actually seen in the subgraphs rather than the configured ones. Also fixes a pre-existing crash when a client leaves an argument to its schema default: graphql-js fills the value but there is no AST node for it, and the query builder dereferenced it. The argument is now left out of the subgraph query so the subgraph applies the same default. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
An unset option keeps the composer's behaviour as it was: the first subgraph's values for a shared enum, the concatenated field lists for a shared object type, and the last subgraph's resolver for a shared root field. The only addition is a warning that names the field and the subgraphs and points at the option, so upgrading is not a breaking change. The four strategies are opt-in; the default can move in a future major version. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Comments and README describe the mechanism that exists rather than narrating what changed; the test names say what unset does. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
|
looks nice to me, @Simon-Lind-glitch any reason this is draft? |
Simon-Lind-glitch
marked this pull request as ready for review
October 25, 2026 20:07
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The composer merges types that have the same name in more than one subgraph. It does not check for conflicts.
QueryorMutation, the composer keeps the resolver of the last subgraph.Result: a client sends a value that the first subgraph accepts. The gateway sends the call to the last subgraph. The last subgraph does not know the value. The composer does not report the conflict.
Example: Subgraph A publishes
enum AclEntity { tenant }andrecomputeAclDefaults(entity: AclEntity!). Subgraph B publishesenum AclEntity { asset }and the same field. The composed enum contains onlytenant. The gateway sendsrecomputeAclDefaults(entity: tenant)to subgraph B. Subgraph B rejects the value.Change
The
onConflictoption decides how the composer merges a type or root field that more than one subgraph publishes.onConflicterrorfirstlastrouteThe default keeps the current behaviour, so this is not a breaking change. The current behaviour is inconsistent: a shared enum keeps the first subgraph's values, and a shared root field is answered by the last subgraph. A call can validate against one subgraph and run against another. The composer now logs a warning for each shared root field. Select a strategy to get consistent behaviour. We suggest that the default changes to
errorin the next major version.With
route, the subgraphs must agree on the field signature. The routing argument must be non-null or have a default value. Each enum value must come from exactly one subgraph. Composition fails when these conditions do not hold. See the README section "Same-named types across subgraphs".The change also corrects a crash in the query builder. When a client left an argument to its schema default, the builder read an AST node that did not exist. The builder now leaves the argument out, and the subgraph applies the same default.
Question for the maintainers
We use
route. We are not sure that routing by argument value is a pattern this project wants. The option makes it easy to accept onlyerror,firstandlast. Tell us which strategies you want to keep.Tests
test/shared-root-fields.test.jsadds tests for the default behaviour, each strategy, routing by literal, by variable and by a defaulted argument, and each composition failure.standardreports no errors.🤖 Generated with Claude Code