Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[federation][resolvers] Add generateInternalResolversIfNeeded. __resolveReference to generate __resolveReference only when resolvable #9989

Merged
merged 11 commits into from
Oct 8, 2024

Conversation

eddeee888
Copy link
Collaborator

@eddeee888 eddeee888 commented Jun 6, 2024

Description

From Apollo Federation doc: https://www.apollographql.com/docs/federation/federated-types/federated-directives/#resolvable: when there's no resolvable @keys on an object type in a subgraph, we cannot move to the object type in said subgraph.

This PR:

  • adds generateInternalResolversIfNeeded.__resolveType
  • By default, generateInternalResolversIfNeeded.__resolveType = false generates optional __resolveType without checking whether there's at least one resolvable @key
  • When generateInternalResolversIfNeeded.__resolveType = true:
    • avoids generating __resolveReference if there's no resolvable @key
    • generates non-required __resolveReference for resolvable @key and skip non-resolvable ones
    • report whether a type has reference resolver in meta so something like Server Preset can handle it correctly

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Screenshots/Sandbox (if appropriate/relevant):

Adding links to sandbox or providing screenshots can help us understand more about this PR and take action on it as appropriate

How Has This Been Tested?

  • Unit tests for federation and resolvers plugin

Copy link

changeset-bot bot commented Jun 6, 2024

🦋 Changeset detected

Latest commit: 5103ffd

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 Minor
@graphql-codegen/typescript-resolvers Minor
@graphql-codegen/plugin-helpers Minor
@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

Copy link
Contributor

github-actions bot commented Jun 6, 2024

🚀 Snapshot Release (alpha)

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

Package Version Info
@graphql-codegen/visitor-plugin-common 5.5.0-alpha-20241008120609-5103ffdfd3b1dde3243e745e73922d58be0c104f npm ↗︎ unpkg ↗︎
@graphql-codegen/typescript-document-nodes 4.0.11-alpha-20241008120609-5103ffdfd3b1dde3243e745e73922d58be0c104f npm ↗︎ unpkg ↗︎
@graphql-codegen/gql-tag-operations 4.0.11-alpha-20241008120609-5103ffdfd3b1dde3243e745e73922d58be0c104f npm ↗︎ unpkg ↗︎
@graphql-codegen/typescript-operations 4.3.1-alpha-20241008120609-5103ffdfd3b1dde3243e745e73922d58be0c104f npm ↗︎ unpkg ↗︎
@graphql-codegen/typescript-resolvers 4.4.0-alpha-20241008120609-5103ffdfd3b1dde3243e745e73922d58be0c104f npm ↗︎ unpkg ↗︎
@graphql-codegen/typed-document-node 5.0.11-alpha-20241008120609-5103ffdfd3b1dde3243e745e73922d58be0c104f npm ↗︎ unpkg ↗︎
@graphql-codegen/typescript 4.1.1-alpha-20241008120609-5103ffdfd3b1dde3243e745e73922d58be0c104f npm ↗︎ unpkg ↗︎
@graphql-codegen/client-preset 4.5.0-alpha-20241008120609-5103ffdfd3b1dde3243e745e73922d58be0c104f npm ↗︎ unpkg ↗︎
@graphql-codegen/graphql-modules-preset 4.0.11-alpha-20241008120609-5103ffdfd3b1dde3243e745e73922d58be0c104f npm ↗︎ unpkg ↗︎
@graphql-codegen/plugin-helpers 5.1.0-alpha-20241008120609-5103ffdfd3b1dde3243e745e73922d58be0c104f npm ↗︎ unpkg ↗︎

Copy link
Contributor

github-actions bot commented Jun 6, 2024

💻 Website Preview

The latest changes are available as preview in: https://9d3f5c3e.graphql-code-generator.pages.dev

@eddeee888 eddeee888 force-pushed the federation-no-resolve-reference-if-not-resolvable branch from 9e14ec7 to 2063b04 Compare August 20, 2024 11:23
@eddeee888 eddeee888 force-pushed the federation-no-resolve-reference-if-not-resolvable branch from e443756 to 06035ca Compare August 22, 2024 10:32
@eddeee888 eddeee888 force-pushed the federation-no-resolve-reference-if-not-resolvable branch from 05dac21 to 3034c9a Compare August 22, 2024 10:56
@eddeee888 eddeee888 marked this pull request as ready for review August 22, 2024 12:17
@eddeee888 eddeee888 force-pushed the federation-no-resolve-reference-if-not-resolvable branch from 18a679b to 50b53f8 Compare August 25, 2024 13:23
@eddeee888 eddeee888 force-pushed the federation-no-resolve-reference-if-not-resolvable branch from 50b53f8 to 587899f Compare August 25, 2024 13:37
@eddeee888 eddeee888 changed the title [federation][resolvers] Do not generate __resolveReference if not resolvable [federation][resolvers] Add generateInternalResolversIfNeeded. __resolveReference to generate __resolveReference only when resolvable Aug 25, 2024
export function checkObjectTypeFederationDetails(
node: ObjectTypeDefinitionNode | GraphQLObjectType,
schema: GraphQLSchema
): { resolvableKeyDirectives: readonly DirectiveNode[] } | false {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wanted to make the return type { resolvableKeyDirectives: readonly DirectiveNode[] } | undefined, and make undefined represent the case when the node is not a object type with federation.

However, the returned value is never undefined (image attached) 🤔 So I've made it false so at least typecheck works

Screenshot 2024-08-25 at 11 41 12 PM

@@ -275,7 +291,10 @@ export class ApolloFederation {
* Checks if Object Type is involved in Federation. Based on `@key` directive
* @param node Type
*/
function isFederationObjectType(node: ObjectTypeDefinitionNode | GraphQLObjectType, schema: GraphQLSchema): boolean {
export function checkObjectTypeFederationDetails(
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 function checks whether a node has is federation, and returns federation metadata for the node (for now, it returns an array of resolvable @keys)

I'm not very happy with the name checkObjectTypeFederationDetails as it reads a bit clumsy 🤔 Open for suggestion.

@@ -656,7 +681,6 @@ export class BaseResolversVisitor<
protected _globalDeclarations = new Set<string>();
protected _federation: ApolloFederation;
protected _hasScalars = false;
protected _hasFederation = false;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Previously, whenever there's a type with __resolveReference, this will be set to true.

Instead of a boolean, we now want to track all the types with generated __resolveReference types as output meta, so downstream consumers (e.g. Server Preset) can easily use it.
The ApolloFederation class now tracks this metadata.

I could just put it here as a property of the plugin e.g. _federationMeta but it seems like ApolloFederation is supposed to handle federation concerns. Happy to discuss if needed.

@eddeee888 eddeee888 changed the title [federation][resolvers] Add generateInternalResolversIfNeeded. __resolveReference to generate __resolveReference only when resolvable [federation][resolvers] Add generateInternalResolversIfNeeded. __resolveReference to generate __resolveReference only when resolvable Sep 5, 2024
@eddeee888 eddeee888 merged commit 55a1e9e into master Oct 8, 2024
19 checks passed
@eddeee888 eddeee888 deleted the federation-no-resolve-reference-if-not-resolvable branch October 8, 2024 12:15
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants