Skip to content

[resolvers][federation] Fix fields or types being wrong generated when marked with @external #10287

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

Draft
wants to merge 1 commit into
base: federation-fixes
Choose a base branch
from

Conversation

eddeee888
Copy link
Collaborator

@eddeee888 eddeee888 commented Feb 10, 2025

Description

When a field or type is marked with @external, the resolver should not be generated (unless part of @provides). This PR takes care of the following scenarios:

  • If a field is marked as @external, do not generate it
  • If any field is marked with @external, the object type's resolvers signature must still use ParentType for its first param
  • If all fields in an object type is marked as @external, do not generate the type
  • If the object type is marked as @external, do not generate the type
  • Question: if a field returns object type and not marked with @external, but the object type itself is marked with @external, what happens?
  • Question: same as above but with @provides in play?

Related #10206

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Unit test

Copy link

changeset-bot bot commented Feb 10, 2025

🦋 Changeset detected

Latest commit: 7949e63

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

This PR includes changesets to release 10 packages
Name Type
@graphql-codegen/visitor-plugin-common Patch
@graphql-codegen/typescript-resolvers Patch
@graphql-codegen/plugin-helpers Patch
@graphql-codegen/typescript-document-nodes Patch
@graphql-codegen/gql-tag-operations Patch
@graphql-codegen/typescript-operations Patch
@graphql-codegen/typed-document-node Patch
@graphql-codegen/typescript Patch
@graphql-codegen/graphql-modules-preset Patch
@graphql-codegen/client-preset Patch

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

Comment on lines +442 to +493
// A ObjectTypeDefinitionNode's directive looks like `{ kind: 'Directive', name: 'external', arguments: [] }`
// However, other directives looks like `{ kind: 'Directive', name: { kind: 'Name', value: 'external' }, arguments: [] }`
// Therefore, we need to check for both `d.name.value` and d.name
return d.name.value === name || (d.name as unknown as string) === name;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is an observation from running unit tests. I'm not sure right now if this is a bug in graphql lib or we did something in our plugins that caused this. Either way, I'm putting this in first to unblock this PR

@eddeee888 eddeee888 marked this pull request as draft February 13, 2025 15:38
@eddeee888 eddeee888 force-pushed the fix-external-signature branch from ddfa8e3 to fbab7d6 Compare February 13, 2025 15:53
Base automatically changed from fix-is-typeof-when-needed to federation-fixes February 19, 2025 09:42
- Handle external directive when part or whole type is marked
- Add changeset
- Add test cases for @provides and @external
@eddeee888 eddeee888 force-pushed the fix-external-signature branch from fbab7d6 to 7949e63 Compare April 23, 2025 13:17
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