Skip to content

fix(composition): Address post-merge feedback for #3391 - #3422

Merged
sachindshinde merged 5 commits into
nextfrom
sachin/address-post-merge-feedback-for-3391
Mar 27, 2026
Merged

fix(composition): Address post-merge feedback for #3391#3422
sachindshinde merged 5 commits into
nextfrom
sachin/address-post-merge-feedback-for-3391

Conversation

@sachindshinde

@sachindshinde sachindshinde commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

This PR:

  • Address post-merge feedback given for #3391 (specifically, this review).
  • Fixes an unrelated bug in sourceFeature(), where it would return the wrong name-in-spec sometimes.
    • Note this bug only occurs when both a @composeDirective spec (or the connect spec) is aliased and a directive with the same name as the spec is used (which should be rare).

@sachindshinde
sachindshinde requested a review from a team as a code owner March 26, 2026 19:34
@apollo-librarian

apollo-librarian Bot commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

✅ Docs preview has no changes

The preview was not built because there were no changes.

Build ID: d72e719bc8ed8965411e5c32
Build Logs: View logs


✅ AI Style Review — No Changes Detected

No MDX files were changed in this pull request.

Review Log: View detailed log

This review is AI-generated. Please use common sense when accepting these suggestions, as they may not always be accurate or appropriate for your specific context.

@changeset-bot

changeset-bot Bot commented Mar 26, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 45a8237

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
@apollo/composition Minor
@apollo/federation-internals Minor
@apollo/gateway Minor
@apollo/query-planner Minor
@apollo/query-graphs Minor
@apollo/subgraph Minor
apollo-federation-integration-testsuite Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@sachindshinde
sachindshinde force-pushed the sachin/address-post-merge-feedback-for-3391 branch from 4246470 to 45a8237 Compare March 26, 2026 20:42
@sachindshinde
sachindshinde requested review from apollo-mateuswgoettems and removed request for matheusazevedoapollo March 26, 2026 21:59

@dariuszkuc dariuszkuc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks good

question: do we want to allow scenarios where older specs define more directives than new one? this would make it easier to evolve* but at the same time limiting to latest spec version is safer as it ensures that all those custom directives are still valid

*if they do need to bump the spec folks will have to import all of them in a single subgraph until migration is over (so following current behavior)

@sachindshinde

Copy link
Copy Markdown
Contributor Author

@dariuszkuc
When folks have to upgrade, they'll indeed need to have the first subgraph that upgrades define all the spec directives in-use by the supergraph if they don't want to break composition. That's not quite the current behavior though, since current composition arbitrarily picks a subgraph (so e.g. the second subgraph to upgrade could cause an error today, whereas after these PR changes it would not). After the upgrade is done, subgraphs can shift back to just defining what they use. I'm not sure if there's a nicer way to do it tbh; we can't really place older directive definitions into the supergraph schema since it may break downstream user logic.

@sachindshinde
sachindshinde merged commit e1fd4ac into next Mar 27, 2026
17 checks passed
@sachindshinde
sachindshinde deleted the sachin/address-post-merge-feedback-for-3391 branch March 27, 2026 16:41
briannafugate408 pushed a commit that referenced this pull request Apr 30, 2026
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to next, this PR will
be updated.


# Releases
## @apollo/composition@2.14.0

### Minor Changes

- Relax `@composeDirective` validation when definitions are absent in
some subgraphs.
([#3422](#3422))

Previously, if some set of spec directives were being composed into the
supergraph schema via `@composeDirective`, then subgraphs with the
latest version of that spec would each have to declare all of those spec
directive definitions. Not following this rule would often result in
composition emitting a `DIRECTIVE_COMPOSITION_ERROR` error. This
restriction has now been relaxed, and a definition needs to only be in
at least one of those subgraphs with the latest version of that spec.

As an example, the following `@composeDirective` usage could previously
fail, but is now valid.

    ```graphql
    # subgraph A — composes and defines @foo and @bar
    extend schema
      # ...
@link(url: "https://myorg.dev/myspec/v1.0", import: ["@foo", "@bar"])
      @composeDirective(name: "@foo")
      @composeDirective(name: "@bar")
    # ...
    directive @foo on FIELD
    directive @bar on FIELD

    # subgraph B — composes and defines only @foo
    extend schema
      # ...
      @link(url: "https://myorg.dev/myspec/v1.0", import: ["@foo"])
      @composeDirective(name: "@foo")
    # ...
    directive @foo on FIELD
    ```

### Patch Changes

- Fixed print logic when calculating the max number of elements to
include in the message. Previously we were not passing
([#3424](#3424))
the current calculated length correctly leading to inclusion of
additional elements in the error/hints message.
- Updated dependencies
\[[`21cf465d4c687daeed71635422718c3c7b7d2d0e`](21cf465),
[`5b36fc6b5a494aa6983e0339713dc45a0bd031e3`](5b36fc6),
[`a20279a0184d9dfbc01a806d849dc8ae22497298`](a20279a)]:
    -   @apollo/federation-internals@2.14.0
    -   @apollo/query-graphs@2.14.0

## @apollo/gateway@2.14.0

### Minor Changes

- Add opt-out anonymous deployment environment telemetry. To opt-out,
set APOLLO_TELEMETRY_DISABLED=1 in your environment.
([#3379](#3379))

### Patch Changes

- Fixed print logic when calculating the max number of elements to
include in the message. Previously we were not passing
([#3424](#3424))
the current calculated length correctly leading to inclusion of
additional elements in the error/hints message.
- Updated dependencies
\[[`21cf465d4c687daeed71635422718c3c7b7d2d0e`](21cf465),
[`e1fd4ac10f72bb09027995f0811ec6e0021bcd49`](e1fd4ac),
[`5b36fc6b5a494aa6983e0339713dc45a0bd031e3`](5b36fc6),
[`a20279a0184d9dfbc01a806d849dc8ae22497298`](a20279a)]:
    -   @apollo/composition@2.14.0
    -   @apollo/federation-internals@2.14.0
    -   @apollo/query-planner@2.14.0

## @apollo/federation-internals@2.14.0

### Minor Changes

- Relax `@interfaceObject` validation for Fed 1 subgraphs.
([#3392](#3392))

Previously, any use of `@interfaceObject` in a Fed 2 subgraph caused an
`INTERFACE_OBJECT_USAGE_ERROR` if any Fed 1 subgraph was present in the
composition, regardless of whether the types conflicted.

The check is now per-type: an error is only raised when a Fed 2 subgraph
uses `@interfaceObject` on type `T` **and** a Fed 1 subgraph has `@key`
on an interface also named `T`. `@key` on an interface in a Fed 1
subgraph does not mean it can fulfill the `__typename`-resolution
requirement that `@interfaceObject` depends on — but they are otherwise
compatible with `@interfaceObject` usage on unrelated types.

- Add validations for `@link` usages to prevent name conflicts.
([#3430](#3430))

This change helps to avoid ambiguity for downstream `@link`-consuming
code, which previously may have found different results for a spec
schema element depending on search order. If your composition fails
after this change, please rename conflicting elements via
`@link(import:)` and conflicting specs/features via `@link(as:)`.

Note that if you were declaring `@link`s for the
`https://specs.apollo.dev/tag` or
`https://specs.apollo.dev/inaccessible` specs in your subgraph schema,
you will need to instead import `@tag` and `@inaccessible` from the
`https://specs.apollo.dev/federation` spec. This previous pattern only
succeeded due to a now-fixed bug and is fragile/may lead to undesirable
behavior.

### Patch Changes

- Fixed print logic when calculating the max number of elements to
include in the message. Previously we were not passing
([#3424](#3424))
the current calculated length correctly leading to inclusion of
additional elements in the error/hints message.

## @apollo/query-graphs@2.14.0

### Patch Changes

- Updated dependencies
\[[`21cf465d4c687daeed71635422718c3c7b7d2d0e`](21cf465),
[`5b36fc6b5a494aa6983e0339713dc45a0bd031e3`](5b36fc6),
[`a20279a0184d9dfbc01a806d849dc8ae22497298`](a20279a)]:
    -   @apollo/federation-internals@2.14.0

## @apollo/query-planner@2.14.0

### Patch Changes

- Updated dependencies
\[[`21cf465d4c687daeed71635422718c3c7b7d2d0e`](21cf465),
[`5b36fc6b5a494aa6983e0339713dc45a0bd031e3`](5b36fc6),
[`a20279a0184d9dfbc01a806d849dc8ae22497298`](a20279a)]:
    -   @apollo/federation-internals@2.14.0
    -   @apollo/query-graphs@2.14.0

## @apollo/subgraph@2.14.0

### Patch Changes

- Updated dependencies
\[[`21cf465d4c687daeed71635422718c3c7b7d2d0e`](21cf465),
[`5b36fc6b5a494aa6983e0339713dc45a0bd031e3`](5b36fc6),
[`a20279a0184d9dfbc01a806d849dc8ae22497298`](a20279a)]:
    -   @apollo/federation-internals@2.14.0

## apollo-federation-integration-testsuite@2.14.0

### Minor Changes

- Add opt-out anonymous deployment environment telemetry. To opt-out,
set APOLLO_TELEMETRY_DISABLED=1 in your environment.
([#3379](#3379))

### Patch Changes

- Fixed print logic when calculating the max number of elements to
include in the message. Previously we were not passing
([#3424](#3424))
the current calculated length correctly leading to inclusion of
additional elements in the error/hints message.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

3 participants