Skip to content

composition result differences for compose directives #298

Draft
n1ru4l wants to merge 1 commit intodirectives-fixfrom
the-order-matters-lol
Draft

composition result differences for compose directives #298
n1ru4l wants to merge 1 commit intodirectives-fixfrom
the-order-matters-lol

Conversation

@n1ru4l
Copy link
Copy Markdown
Contributor

@n1ru4l n1ru4l commented Apr 13, 2026

The current apollo federation and guild federation libraries behave different on the non-happy path.

If the compose directives definition is different in all supgraph definitions:

The Guild composition raises an exception:

[
  GraphQLError {
    "message": "Type of argument \"@a(name:)\" is incompatible across subgraphs: it has type \"String!\" in subgraph \"a\" but type \"ID!\" in subgraph \"b\"",
    "path": undefined,
    "locations": undefined,
    "extensions": {
      "code": "FIELD_ARGUMENT_TYPE_MISMATCH",
    },
  },
]

The apollo federation composition chooses one definition that will win by sorting by service name descending (you can switch out the name of the services and see that the tests start passing on apollo federation) 😓

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds test cases to verify the behavior of directive definition conflicts during service composition, specifically focusing on whether the order of subgraphs affects the outcome. The review feedback points out that the tests currently assert a successful composition despite a type mismatch in directive arguments (String! vs ID!). The reviewer suggests that this behavior should likely be treated as a validation error rather than being codified as a success, as it may represent a bug or a missing validation rule.

Comment on lines +562 to +563
expect(result.errors).toEqual(undefined);
assertCompositionSuccess(result);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

These assertions validate that a conflict in directive argument types (String! vs ID!) results in a successful composition where one type is silently chosen based on subgraph sorting. However, the PR description suggests that this behavior is inconsistent with expectations ('The Guild composition raises an exception') and notes that Apollo's similar 'silent winner' approach is problematic (😓).

By using assertCompositionSuccess, these tests codify what appears to be a bug or a missing validation rule. If the intention is to ensure that such conflicts are caught (similar to the FIELD_ARGUMENT_TYPE_MISMATCH mentioned in the description), these tests should instead expect a validation error. Codifying the current inconsistent behavior as a success may make it harder to fix later.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant