[resolvers][federation] Add `__resolveReference` to applicable `Interface` entities, fix Interface types having non-meta resolver fields #10221

eddeee888 · 2024-12-22T13:44:23Z

TODO: video walkthrough

Description

This PR ensures correct types for Interfaces:

1. `__resolveReference` are generated for Interface entities

Federation Checklist: #10206

interface User @key(fields: "id") { # must generate `__resolveReference` for User
  # User fields
}

https://www.apollographql.com/docs/graphos/schema-design/federated-schemas/entities/interfaces#example-schemas

2. Interfaces do not have non-meta resolvers

Related PR: #5666

interface User { 
  # fields below should not be generated for User interface because they are never called.
  # Implementing types fields are called instead
  id: ID!
  name: String!
}

Breaking Changes

As part of this work, we remove unnecessary config that could generate wrong types for both Federation and normal use cases:

BREAKING CHANGES: Deprecate onlyResolveTypeForInterfaces because majority of use cases cannot implement resolvers in Interfaces.
BREAKING CHANGES: Deprecate generateInternalResolversIfNeeded.__resolveReference because types do not have __resolveReference if they are not Federation entities or are not resolvable. Users should not have to manually set this option. This option was put in to wait for this major version.

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

Unit test

changeset-bot · 2024-12-22T13:44:27Z

🦋 Changeset detected

Latest commit: 65fa44f

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

This PR includes changesets to release 18 packages

Name	Type
@graphql-codegen/visitor-plugin-common	Major
@graphql-codegen/typescript-resolvers	Major
@graphql-codegen/plugin-helpers	Major
@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/introspection	Patch
@graphql-codegen/client-preset	Patch
@graphql-codegen/graphql-modules-preset	Patch
@graphql-codegen/cli	Patch
@graphql-codegen/core	Patch
@graphql-codegen/testing	Patch
@graphql-codegen/add	Patch
@graphql-codegen/fragment-matcher	Patch
@graphql-codegen/schema-ast	Patch
@graphql-codegen/time	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

eddeee888 · 2024-12-22T13:47:41Z

packages/plugins/other/visitor-plugin-common/src/base-resolvers-visitor.ts

-  /**
-   * @type boolean
-   * @default false
-   * @description Turning this flag to `true` will generate resolver signature that has only `resolveType` for interfaces, forcing developers to write inherited type resolvers in the type itself.
-   */
-  onlyResolveTypeForInterfaces?: boolean;


Related #5648

onlyResolveTypeForInterfaces is only applicable for some rare use cases mentioned here. This option's implementation blocks __resolveReference being created for Interfaces when the option is turned on.

I think it's simpler just to deprecate this option as we are moving to a new major version. If anyone needs it, we can put it back as a minor version

eddeee888 · 2024-12-22T13:49:16Z

packages/plugins/other/visitor-plugin-common/src/base-resolvers-visitor.ts

   * @description If relevant internal resolvers are set to `true`, the resolver type will only be generated if the right conditions are met.
   * Enabling this allows a more correct type generation for the resolvers.
   * For example:
   * - `__isTypeOf` is generated for implementing types and union members
-   * - `__resolveReference` is generated for federation types that have at least one resolvable `@key` directive
   */
  generateInternalResolversIfNeeded?: GenerateInternalResolversIfNeededConfig;


generateInternalResolversIfNeeded might be deprecated in this major version, it'll come in one of the next few PRs.
Since we are bumping a major version, it's worth generating these meta types only when required

eddeee888 · 2024-12-22T13:52:55Z

packages/utils/plugins-helpers/src/federation.ts

+ * Adds `__resolveReference` in each ObjectType and InterfaceType involved in Federation.
 * @param schema
 */
-export function addFederationReferencesToSchema(schema: GraphQLSchema): GraphQLSchema {
-  return mapSchema(schema, {
+export function addFederationReferencesToSchema(schema: GraphQLSchema): {


We are transforming the schema to have __resolveReference fields to prepare for the resolvers plugin. Because of this pattern, it's worth capturing meta here so, and use the meta instead of running checkTypeFederationDetails in the plugin.

Because of this, checkTypeFederationDetails is now only used in this file i.e. it's no longer exported and called in resolvers plugin -> better logic encapsulation

eddeee888 · 2025-01-12T06:43:16Z

packages/utils/plugins-helpers/src/federation.ts

 * @param schema
 */
-export function addFederationReferencesToSchema(schema: GraphQLSchema): GraphQLSchema {
-  return mapSchema(schema, {
+export function addFederationReferencesToSchema(schema: GraphQLSchema): {


We now collect Federation meta in this function because this is called before calling visitor.
This means we no longer have to collect meta as we find entities during visitor -> simpler, more predictable, and more performant

eddeee888 · 2025-01-12T06:46:29Z

packages/plugins/other/visitor-plugin-common/src/base-resolvers-visitor.ts

+type FieldDefinitionPrintFn = (
+  parentName: string,
+  avoidResolverOptionals: boolean
+) => { value: string | null; meta: { federation?: { isResolveReference: boolean } } };


Unfortunately, we don't know if a field is __resolveReference or not until we have generated it. This is because we are injecting __resolveReference as real fields in addFederationReferencesToSchema. Therefore, we need to add meta to have more info about the generated field

eddeee888 · 2025-01-12T06:56:13Z

packages/plugins/other/visitor-plugin-common/src/base-resolvers-visitor.ts

-        if (this.config.generateInternalResolversIfNeeded.__resolveReference) {
-          const federationDetails = checkObjectTypeFederationDetails(
-            parentType.astNode as ObjectTypeDefinitionNode,
-            this._schema
-          );
-
-          if (!federationDetails || federationDetails.resolvableKeyDirectives.length === 0) {
-            return '';
-          }
-          signature.modifier = ''; // if a federation type has resolvable @key, then it should be required
+        if (!this._federation.getMeta()[parentType.name].hasResolveReference) {
+          return { value: '', meta };
        }
-
-        this._federation.setMeta(parentType.name, { hasResolveReference: true });


We no longer check and collect meta as we hit federation fields any more because we are doing it before running visitor. Relevant comment.
Therefore, we can check federation meta directly here

eddeee888 · 2025-01-12T06:56:44Z

packages/plugins/other/visitor-plugin-common/src/base-resolvers-visitor.ts

-        if (signature.genericTypes.length >= 3) {
-          signature.genericTypes = signature.genericTypes.slice(0, 3);
-        }


It's a bit odd to run .slice to limit generics to 3, may as well be explicit here!

eddeee888 · 2025-01-18T08:37:52Z

packages/plugins/typescript/resolvers/tests/ts-resolvers.federation.spec.ts

-  it.skip('should handle interface types', async () => {
-    const federatedSchema = /* GraphQL */ `
-      type Query {
-        people: [Person]
-      }
-
-      extend interface Person @key(fields: "name { first last }") {
-        name: Name! @external
-        age: Int @requires(fields: "name")
-      }
-
-      extend type User implements Person @key(fields: "name { first last }") {
-        name: Name! @external
-        age: Int @requires(fields: "name { first last }")
-        username: String
-      }
-
-      type Admin implements Person @key(fields: "name { first last }") {
-        name: Name! @external
-        age: Int @requires(fields: "name { first last }")
-        permissions: [String!]!
-      }
-
-      extend type Name {
-        first: String! @external
-        last: String! @external
-      }
-    `;
-
-    const content = await generate({
-      schema: federatedSchema,
-      config: {
-        federation: true,
-      },
-    });
-
-    expect(content).toBeSimilarStringTo(`
-    export type PersonResolvers<ContextType = any, ParentType extends ResolversParentTypes['Person'] = ResolversParentTypes['Person']> = {
-      __resolveType: TypeResolveFn<'User' | 'Admin', ParentType, ContextType>;
-      age?: Resolver<Maybe<ResolversTypes['Int']>, { __typename: 'User' | 'Admin' } & GraphQLRecursivePick<ParentType, {"name":{"first":true,"last":true}}>, ContextType>;
-    };
-    `);
-  });


This Interface test is now covered in ts-resolvers.federation.interface.spec.ts

- Deprecate generateInternalResolversIfNeeded.__resolveReference - Fix tests - Deprecate onlyResolveTypeForInterfaces - Add changeset - Cleanup - Handle __resolveReference generation in Interface - Let FieldDefinition decide whether to generate __resolveReference by checking whether parent has resolvable key

eddeee888 commented Dec 22, 2024

View reviewed changes

eddeee888 changed the title ~~[resolvers][federation] Add __resolveReference to applicable Interfaces~~ [resolvers][federation] Add __resolveReference to applicable Interface entities Dec 22, 2024

eddeee888 mentioned this pull request Dec 22, 2024

Federation support checklist #10206

Open

35 tasks

Base automatically changed from fix-federation-mapper to federation-fixes January 9, 2025 12:26

eddeee888 force-pushed the fix-interface-with-key-ref branch from 05b4751 to f4ba147 Compare January 9, 2025 12:32

eddeee888 commented Jan 12, 2025

View reviewed changes

eddeee888 force-pushed the federation-fixes branch from aa191e7 to dd2b0bd Compare January 18, 2025 08:12

eddeee888 force-pushed the fix-interface-with-key-ref branch 2 times, most recently from 14fbb52 to 21db36f Compare January 18, 2025 08:36

eddeee888 commented Jan 18, 2025

View reviewed changes

eddeee888 force-pushed the fix-interface-with-key-ref branch from 21db36f to bbf0cf7 Compare January 18, 2025 08:41

Fix test

65fa44f

eddeee888 changed the title ~~[resolvers][federation] Add __resolveReference to applicable Interface entities~~ [resolvers][federation] Add __resolveReference to applicable Interface entities, fix Interface types having non-meta resolver fields Jan 18, 2025

eddeee888 requested review from dotansimha and kamilkisiela January 18, 2025 11:40

eddeee888 marked this pull request as ready for review January 18, 2025 11:42

dotansimha approved these changes Jan 22, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[resolvers][federation] Add `__resolveReference` to applicable `Interface` entities, fix Interface types having non-meta resolver fields #10221

[resolvers][federation] Add `__resolveReference` to applicable `Interface` entities, fix Interface types having non-meta resolver fields #10221

eddeee888 commented Dec 22, 2024 •

edited

Loading

changeset-bot bot commented Dec 22, 2024 •

edited

Loading

eddeee888 Dec 22, 2024 •

edited

Loading

eddeee888 Dec 22, 2024

eddeee888 Dec 22, 2024

eddeee888 Jan 12, 2025

eddeee888 Jan 12, 2025

eddeee888 Jan 12, 2025

eddeee888 Jan 12, 2025 •

edited

Loading

eddeee888 Jan 18, 2025

[resolvers][federation] Add __resolveReference to applicable Interface entities, fix Interface types having non-meta resolver fields #10221

Are you sure you want to change the base?

[resolvers][federation] Add __resolveReference to applicable Interface entities, fix Interface types having non-meta resolver fields #10221

Conversation

eddeee888 commented Dec 22, 2024 • edited Loading

Description

1. __resolveReference are generated for Interface entities

2. Interfaces do not have non-meta resolvers

Breaking Changes

Type of change

How Has This Been Tested?

changeset-bot bot commented Dec 22, 2024 • edited Loading

🦋 Changeset detected

eddeee888 Dec 22, 2024 • edited Loading

Choose a reason for hiding this comment

eddeee888 Dec 22, 2024

Choose a reason for hiding this comment

eddeee888 Dec 22, 2024

Choose a reason for hiding this comment

eddeee888 Jan 12, 2025

Choose a reason for hiding this comment

eddeee888 Jan 12, 2025

Choose a reason for hiding this comment

eddeee888 Jan 12, 2025

Choose a reason for hiding this comment

eddeee888 Jan 12, 2025 • edited Loading

Choose a reason for hiding this comment

eddeee888 Jan 18, 2025

Choose a reason for hiding this comment

[resolvers][federation] Add `__resolveReference` to applicable `Interface` entities, fix Interface types having non-meta resolver fields #10221

[resolvers][federation] Add `__resolveReference` to applicable `Interface` entities, fix Interface types having non-meta resolver fields #10221

eddeee888 commented Dec 22, 2024 •

edited

Loading

1. `__resolveReference` are generated for Interface entities

changeset-bot bot commented Dec 22, 2024 •

edited

Loading

eddeee888 Dec 22, 2024 •

edited

Loading

eddeee888 Jan 12, 2025 •

edited

Loading