diff --git a/.changeset/smooth-terms-decide.md b/.changeset/smooth-terms-decide.md new file mode 100644 index 000000000..98dbe2943 --- /dev/null +++ b/.changeset/smooth-terms-decide.md @@ -0,0 +1,5 @@ +--- +"@apollo/subgraph": patch +--- + +When a `GraphQLScalarType` resolver is provided to `buildSubgraphSchema()`, omitted configuration options in the `GraphQLScalarType` no longer cause the corresponding properties in the GraphQL document/AST to be cleared. To explicitly clear these properties, use `null` for the configuration option instead. diff --git a/subgraph-js/src/__tests__/buildSubgraphSchema.test.ts b/subgraph-js/src/__tests__/buildSubgraphSchema.test.ts index 58243240c..d159783f0 100644 --- a/subgraph-js/src/__tests__/buildSubgraphSchema.test.ts +++ b/subgraph-js/src/__tests__/buildSubgraphSchema.test.ts @@ -6,10 +6,12 @@ import { execute, type DefinitionNode, OperationTypeNode, + GraphQLScalarType, GraphQLUnionType, printType, } from 'graphql'; import { buildSubgraphSchema } from '../buildSubgraphSchema'; +import { printSubgraphSchema } from '../printSubgraphSchema'; import { typeSerializer } from 'apollo-federation-integration-testsuite'; import { errorCauses } from '@apollo/federation-internals'; @@ -964,10 +966,12 @@ describe('buildSubgraphSchema', () => { const validateTag = async ( header: string, + printedHeader: string, directiveDefinitions: string, typeDefinitions: string, + withResolvers: boolean, ) => { - const schema = buildSubgraphSchema(gql` + const typeDefs = gql` ${header} type User @key(fields: "email") @tag(name: "tagOnType") { email: String @tag(name: "tagOnField") @@ -978,19 +982,15 @@ describe('buildSubgraphSchema', () => { } union UserButAUnion @tag(name: "tagOnUnion") = User - `); - const { data, errors } = await graphql({ schema, source: query }); - expect(errors).toBeUndefined(); - expect((data?._service as any).sdl).toMatchString( - (header.length === 0 - ? '' - : ` - ${header.trim()} - `) + - ` - ${directiveDefinitions.trim()} + scalar CustomScalar @tag(name: "tagOnCustomScalar") + enum Enum @tag(name: "tagOnEnum") { + ENUM_VALUE @tag(name: "tagOnEnumValue") + } + `; + + const printedTypeDefsWithoutHeader = ` type User @key(fields: "email") @tag(name: "tagOnType") @@ -1008,15 +1008,72 @@ describe('buildSubgraphSchema', () => { @tag(name: "tagOnUnion") = User + scalar CustomScalar + @tag(name: "tagOnCustomScalar") + + enum Enum + @tag(name: "tagOnEnum") + { + ENUM_VALUE @tag(name: "tagOnEnumValue") + } + `; + + const schema = withResolvers + ? buildSubgraphSchema({ + typeDefs, + resolvers: { + User: { + email: () => 1, + }, + CustomScalar: new GraphQLScalarType({ + name: 'CustomScalar', + serialize: () => 1, + parseValue: () => 1, + parseLiteral: () => 1, + }), + Enum: { + ENUM_VALUE: 1, + }, + }, + }) + : buildSubgraphSchema(typeDefs); + + // Check the sdl field's schema. + const { data, errors } = await graphql({ schema, source: query }); + expect(errors).toBeUndefined(); + expect((data?._service as any).sdl).toMatchString( + (header.length === 0 + ? '' + : ` + ${header.trim()} + `) + + ` + ${directiveDefinitions.trim()} + + ${printedTypeDefsWithoutHeader.trim()} + ${typeDefinitions.trim()} `, ); + + // Check the printSubgraphSchema's schema. + expect(printSubgraphSchema(schema)).toMatchString( + (printedHeader.length === 0 + ? '' + : ` + ${printedHeader.trim()} + `) + + ` + ${printedTypeDefsWithoutHeader.trim()} + `, + ); }; - it.each([ + const testCases = [ { name: 'fed1', header: '', + printedHeader: '', directiveDefinitions: ` directive @key(fields: _FieldSet!, resolvable: Boolean = true) repeatable on OBJECT | INTERFACE @@ -1053,6 +1110,10 @@ describe('buildSubgraphSchema', () => { extend schema @link(url: "https://specs.apollo.dev/link/v1.0") @link(url: "https://specs.apollo.dev/federation/v2.0", import: ["@key", "@tag"]) + `, + printedHeader: ` + extend schema + @link(url: "https://specs.apollo.dev/federation/v2.0", import: ["@key", "@tag"]) `, directiveDefinitions: ` directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA @@ -1106,10 +1167,41 @@ describe('buildSubgraphSchema', () => { } `, }, - ])( + ]; + + it.each(testCases)( 'adds it for $name schema', - async ({ header, directiveDefinitions, typesDefinitions }) => { - await validateTag(header, directiveDefinitions, typesDefinitions); + async ({ + header, + printedHeader, + directiveDefinitions, + typesDefinitions, + }) => { + await validateTag( + header, + printedHeader, + directiveDefinitions, + typesDefinitions, + false, + ); + }, + ); + + it.each(testCases)( + 'adds it for $name schema with resolvers', + async ({ + header, + printedHeader, + directiveDefinitions, + typesDefinitions, + }) => { + await validateTag( + header, + printedHeader, + directiveDefinitions, + typesDefinitions, + true, + ); }, ); }); diff --git a/subgraph-js/src/schema-helper/buildSchemaFromSDL.ts b/subgraph-js/src/schema-helper/buildSchemaFromSDL.ts index 9325325a4..46d64c36b 100644 --- a/subgraph-js/src/schema-helper/buildSchemaFromSDL.ts +++ b/subgraph-js/src/schema-helper/buildSchemaFromSDL.ts @@ -133,7 +133,16 @@ export function addResolversToSchema( if (isScalarType(type)) { for (const fn in fieldConfigs) { - (type as any)[fn] = (fieldConfigs as any)[fn]; + const fnValue = (fieldConfigs as any)[fn]; + // When users provide a `GraphQLScalarType` for resolvers, they often + // omit several config options (effectively providing `undefined`), but + // they probably don't mean for this to unset values in the existing + // `GraphQLScalarType` (e.g., clearing the AST nodes or description). + // So we explicitly ignore `undefined` values here; if users do want to + // unset existing values, they can use `null` instead. + if (fnValue !== undefined) { + (type as any)[fn] = fnValue; + } } }