Skip to content

fix: invalid public sdl for inaccessible interface type#198

Merged
n1ru4l merged 6 commits intomainfrom
fix-interface-implements-inaccessible
Oct 22, 2025
Merged

fix: invalid public sdl for inaccessible interface type#198
n1ru4l merged 6 commits intomainfrom
fix-interface-implements-inaccessible

Conversation

@n1ru4l
Copy link
Copy Markdown
Contributor

@n1ru4l n1ru4l commented Oct 22, 2025

The public schema SDL is wrong when it implements an inaccessible interface type, it retains the interface implementation even though it is not part of the public schema.

See changeset for an example diff of what was fixed

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Oct 22, 2025

🚀 Snapshot Release (alpha)

The latest changes of this PR are available as alpha on npm (based on the declared changesets):

Package Version Info
@theguild/federation-composition 0.20.2-alpha-20251022114045-bc5e6f4cacc50658c12b6a84dfee5c5407839b87 npm ↗︎ unpkg ↗︎

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Could not find a clever way of not having a second visit without changing the call signature of transformSupergraphToPublicSchema. 😄 I guess this is okay enough.

Copy link
Copy Markdown
Member

@dotansimha dotansimha left a comment

Choose a reason for hiding this comment

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

nice catch. @kamilkisiela @jdolle thoughts?

@n1ru4l n1ru4l merged commit 1b98c17 into main Oct 22, 2025
4 checks passed
@n1ru4l n1ru4l deleted the fix-interface-implements-inaccessible branch October 22, 2025 15:46
n1ru4l pushed a commit that referenced this pull request Oct 22, 2025
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 main, this PR will
be updated.


# Releases
## @theguild/federation-composition@0.20.2

### Patch Changes

-
[#198](#198)
[`1b98c17`](1b98c17)
Thanks [@user!](https://github.com/User!),
[@user!](https://github.com/User!)! - Fix public schema SDL in case a
object type implements an inaccessible interface.

    Composing the following subgraph:

    ```graphql
    schema
      @link(
        url: "https://specs.apollo.dev/federation/v2.3"
        import: ["@inaccessible"]
      ) {
      query: Query
    }

    type Query {

    }

    interface Node @inaccessible {
      id: ID!
    }

    type User implements Node {
      id: ID!
    }
    ```

    now result in the following valid public SDL:

    ```diff
      type Query {

      }

    - type User implements Node {
    + type User {
        id: ID!
      }
    ```

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@jdolle jdolle left a comment

Choose a reason for hiding this comment

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

To avoid a full second pass youd need to keep track of where interfaces are used, then do a lookup based on the interface name to the nodes.

I understand why we have to do this, but I think this behavior is all a bit strange. We don't automatically clean up fields if they have a returnType that is made inaccessible.... Should we do that also?

@n1ru4l
Copy link
Copy Markdown
Contributor Author

n1ru4l commented Oct 23, 2025

@jdolle I think that is already done in a pre step as part of the supergraph validation and building. @inaccessible is already added to these fields. But since it is not possible to add directives on an interface implementation and it needs to be present within the supergraph, the only option for cleaning the SDL is to do it this way I think. Obviously, we could keep track of the interfaces before and pass it as an input to the printSupergraph function to avoid the second traversal 🤔

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