diff --git a/.changeset/tall-cats-flash.md b/.changeset/tall-cats-flash.md new file mode 100644 index 000000000..62c3b8e5c --- /dev/null +++ b/.changeset/tall-cats-flash.md @@ -0,0 +1,7 @@ +--- +"@apollo/composition": patch +"@apollo/federation-internals": patch +--- + +Fixing issue where redeclaration of custom scalars in a fed1 schema may cause upgrade errors + \ No newline at end of file diff --git a/.cspell/cspell-dict.txt b/.cspell/cspell-dict.txt index 396cfd18d..3de941c55 100644 --- a/.cspell/cspell-dict.txt +++ b/.cspell/cspell-dict.txt @@ -175,6 +175,7 @@ Queryf reacheable reasonse recusive +redeclaration refered Referencer referencer diff --git a/composition-js/src/__tests__/composeFed1Subgraphs.test.ts b/composition-js/src/__tests__/composeFed1Subgraphs.test.ts index e22bede09..8e158fb41 100644 --- a/composition-js/src/__tests__/composeFed1Subgraphs.test.ts +++ b/composition-js/src/__tests__/composeFed1Subgraphs.test.ts @@ -727,4 +727,22 @@ describe('override', () => { '[subgraphB] Invalid definition for directive \"@override\": missing required argument "from"', ]]); }); + + it('repro redefined built-in scalar breaks @key directive', () => { + const subgraphA = { + typeDefs: gql` + scalar Boolean + type Query { + q: String + } + type A @key(fields: "k") { + k: ID! + } + `, + name: 'subgraphA', + }; + + const result = composeServices([subgraphA,]); + assertCompositionSuccess(result); + }); }); diff --git a/internals-js/src/validate.ts b/internals-js/src/validate.ts index e4a02bf36..755f79a2d 100644 --- a/internals-js/src/validate.ts +++ b/internals-js/src/validate.ts @@ -8,6 +8,7 @@ import { InterfaceType, isInputObjectType, isNonNullType, + isScalarType, NamedSchemaElement, ObjectType, Schema, @@ -17,7 +18,7 @@ import { VariableDefinitions } from "./definitions"; import { assertName, ASTNode, GraphQLError, GraphQLErrorOptions } from "graphql"; -import { isValidValue, valueToString } from "./values"; +import { isValidValue, valueToString, isValidValueApplication } from "./values"; import { introspectionTypeNames, isIntrospectionName } from "./introspection"; import { isSubtype, sameType } from "./types"; import { ERRORS } from "./error"; @@ -290,10 +291,14 @@ class Validator { ); } if (arg.defaultValue !== undefined && !isValidValue(arg.defaultValue, arg, new VariableDefinitions())) { - this.addError( - `Invalid default value (got: ${valueToString(arg.defaultValue)}) provided for argument ${arg.coordinate} of type ${arg.type}.`, - { nodes: sourceASTs(arg) }, - ); + // don't error if custom scalar is shadowing a builtin scalar + const builtInScalar = this.schema.builtInScalarTypes().find((t) => arg.type && isScalarType(arg.type) && t.name === arg.type.name); + if (!builtInScalar || !isValidValueApplication(arg.defaultValue, builtInScalar, arg.defaultValue, new VariableDefinitions())) { + this.addError( + `Invalid default value (got: ${valueToString(arg.defaultValue)}) provided for argument ${arg.coordinate} of type ${arg.type}.`, + { nodes: sourceASTs(arg) }, + ); + } } } diff --git a/internals-js/src/values.ts b/internals-js/src/values.ts index 7dd7ddd2d..11b2968e5 100644 --- a/internals-js/src/values.ts +++ b/internals-js/src/values.ts @@ -492,7 +492,7 @@ export function isValidValue(value: any, argument: ArgumentDefinition | Inp return isValidValueApplication(value, argument.type!, argument.defaultValue, variableDefinitions); } -function isValidValueApplication(value: any, locationType: InputType, locationDefault: any, variableDefinitions: VariableDefinitions): boolean { +export function isValidValueApplication(value: any, locationType: InputType, locationDefault: any, variableDefinitions: VariableDefinitions): boolean { // Note that this needs to be first, or the recursive call within 'isNonNullType' would break for variables if (isVariable(value)) { const definition = variableDefinitions.definition(value);