diff --git a/composition-js/src/__tests__/compose.directiveArgumentMergeStrategies.test.ts b/composition-js/src/__tests__/compose.directiveArgumentMergeStrategies.test.ts index b1be5c54c..ee7de6daa 100644 --- a/composition-js/src/__tests__/compose.directiveArgumentMergeStrategies.test.ts +++ b/composition-js/src/__tests__/compose.directiveArgumentMergeStrategies.test.ts @@ -89,17 +89,19 @@ describe('composition of directive with non-trivial argument strategies', () => t: 2, k: 1, b: 4, }, }, { - name: 'sum', - type: (schema: Schema) => new NonNullType(schema.intType()), - compositionStrategy: ARGUMENT_COMPOSITION_STRATEGIES.SUM, - argValues: { - s1: { t: 3, k: 1 }, - s2: { t: 2, k: 5, b: 4 }, - }, - resultValues: { - t: 5, k: 6, b: 4, - }, - }, { + // NOTE: See the note for the SUM strategy in argumentCompositionStrategies.ts + // for more information on why this is commented out. + // name: 'sum', + // type: (schema: Schema) => new NonNullType(schema.intType()), + // compositionStrategy: ARGUMENT_COMPOSITION_STRATEGIES.SUM, + // argValues: { + // s1: { t: 3, k: 1 }, + // s2: { t: 2, k: 5, b: 4 }, + // }, + // resultValues: { + // t: 5, k: 6, b: 4, + // }, + // }, { name: 'intersection', type: (schema: Schema) => new NonNullType(new ListType(new NonNullType(schema.stringType()))), compositionStrategy: ARGUMENT_COMPOSITION_STRATEGIES.INTERSECTION, diff --git a/internals-js/src/__tests__/values.test.ts b/internals-js/src/__tests__/values.test.ts index eaab39aa7..6e1d167fb 100644 --- a/internals-js/src/__tests__/values.test.ts +++ b/internals-js/src/__tests__/values.test.ts @@ -414,5 +414,6 @@ describe('objectEquals tests', () => { expect(valueEquals({ foo: 'foo', bar: undefined }, { foo: 'foo' })).toBe( false, ); + expect(valueEquals({}, null)).toBe(false); }); }); diff --git a/internals-js/src/argumentCompositionStrategies.ts b/internals-js/src/argumentCompositionStrategies.ts index 85a870773..f9d60ef3f 100644 --- a/internals-js/src/argumentCompositionStrategies.ts +++ b/internals-js/src/argumentCompositionStrategies.ts @@ -13,87 +13,105 @@ export type ArgumentCompositionStrategy = { function supportFixedTypes(types: (schema: Schema) => InputType[]): TypeSupportValidator { return (schema, type) => { const supported = types(schema); - if (!supported.some((t) => sameType(t, type))) { - return { valid: false, supportedMsg: `type(s) ${supported.join(', ')}` }; - } - return { valid: true }; + return supported.some((t) => sameType(t, type)) + ? { valid: true } + : { valid: false, supportedMsg: `type(s) ${supported.join(', ')}` }; }; } function supportAnyNonNullArray(): TypeSupportValidator { - return (_, type) => { - if (!isNonNullType(type) || !isListType(type.ofType)) { - return { valid: false, supportedMsg: 'non nullable list types of any type'}; - } - return { valid: true }; + return (_, type) => isNonNullType(type) && isListType(type.ofType) + ? { valid: true } + : { valid: false, supportedMsg: 'non nullable list types of any type' } +} + +function supportAnyArray(): TypeSupportValidator { + return (_, type) => isListType(type) || (isNonNullType(type) && isListType(type.ofType)) + ? { valid: true } + : { valid: false, supportedMsg: 'list types of any type' }; +} + +// NOTE: This function makes the assumption that for the directive argument +// being merged, it is not "nullable with non-null default" in the supergraph +// schema (this kind of type/default combo is confusing and should be avoided, +// if possible). This assumption allows this function to replace null with +// undefined, which makes for a cleaner supergraph schema. +function mergeNullableValues( + mergeValues: (values: T[]) => T +): (values: (T | null | undefined)[]) => T | undefined { + return (values: (T | null | undefined)[]) => { + const nonNullValues = values.filter((v) => v !== null && v !== undefined) as T[]; + return nonNullValues.length > 0 + ? mergeValues(nonNullValues) + : undefined; }; } +function unionValues(values: any[]): any { + return values.reduce((acc, next) => { + const newValues = next.filter((v1: any) => !acc.some((v2: any) => valueEquals(v1, v2))); + return acc.concat(newValues); + }, []); +} + export const ARGUMENT_COMPOSITION_STRATEGIES = { MAX: { name: 'MAX', isTypeSupported: supportFixedTypes((schema: Schema) => [new NonNullType(schema.intType())]), - mergeValues: (values: any[]) => Math.max(...values), + mergeValues: (values: number[]) => Math.max(...values), }, MIN: { name: 'MIN', isTypeSupported: supportFixedTypes((schema: Schema) => [new NonNullType(schema.intType())]), - mergeValues: (values: any[]) => Math.min(...values), - }, - SUM: { - name: 'SUM', - isTypeSupported: supportFixedTypes((schema: Schema) => [new NonNullType(schema.intType())]), - mergeValues: (values: any[]) => values.reduce((acc, val) => acc + val, 0), + mergeValues: (values: number[]) => Math.min(...values), }, + // NOTE: This doesn't work today because directive applications are de-duped + // before being merged, we'd need to modify merge logic if we need this kind + // of behavior. + // SUM: { + // name: 'SUM', + // isTypeSupported: supportFixedTypes((schema: Schema) => [new NonNullType(schema.intType())]), + // mergeValues: (values: any[]) => values.reduce((acc, val) => acc + val, 0), + // }, INTERSECTION: { name: 'INTERSECTION', isTypeSupported: supportAnyNonNullArray(), - mergeValues: (values: any[]) => values.reduce((acc, val) => acc.filter((v1: any) => val.some((v2: any) => valueEquals(v1, v2))), values[0]), + mergeValues: (values: any[]) => values.reduce((acc, next) => { + if (acc === undefined) { + return next; + } else { + return acc.filter((v1: any) => next.some((v2: any) => valueEquals(v1, v2))); + } + }, undefined) ?? [], }, UNION: { name: 'UNION', isTypeSupported: supportAnyNonNullArray(), - mergeValues: (values: any[]) => - values.reduce((acc, val) => { - const newValues = val.filter((v1: any) => !acc.some((v2: any) => valueEquals(v1, v2))); - return acc.concat(newValues); - }, []), + mergeValues: unionValues, }, NULLABLE_AND: { name: 'NULLABLE_AND', - isTypeSupported: supportFixedTypes((schema: Schema) => [schema.booleanType()]), - mergeValues: (values: (boolean | null | undefined)[]) => values.reduce((acc, next) => { - if (acc === null || acc === undefined) { - return next; - } else if (next === null || next === undefined) { - return acc; - } else { - return acc && next; - } - }, undefined), + isTypeSupported: supportFixedTypes((schema: Schema) => [ + schema.booleanType(), + new NonNullType(schema.booleanType()) + ]), + mergeValues: mergeNullableValues( + (values: boolean[]) => values.every((v) => v) + ), }, NULLABLE_MAX: { name: 'NULLABLE_MAX', - isTypeSupported: supportFixedTypes((schema: Schema) => [schema.intType(), new NonNullType(schema.intType())]), - mergeValues: (values: any[]) => values.reduce((a: any, b: any) => a !== undefined && b !== undefined ? Math.max(a, b) : a ?? b, undefined), + isTypeSupported: supportFixedTypes((schema: Schema) => [ + schema.intType(), + new NonNullType(schema.intType()) + ]), + mergeValues: mergeNullableValues( + (values: number[]) => Math.max(...values) + ) }, NULLABLE_UNION: { name: 'NULLABLE_UNION', - isTypeSupported: (_: Schema, type: InputType) => ({ valid: isListType(type) }), - mergeValues: (values: any[]) => { - if (values.every((v) => v === undefined)) { - return undefined; - } - - const combined = new Set(); - for (const subgraphValues of values) { - if (Array.isArray(subgraphValues)) { - for (const value of subgraphValues) { - combined.add(value); - } - } - } - return Array.from(combined); - } + isTypeSupported: supportAnyArray(), + mergeValues: mergeNullableValues(unionValues), } } diff --git a/internals-js/src/definitions.ts b/internals-js/src/definitions.ts index e52013047..fa48c460e 100644 --- a/internals-js/src/definitions.ts +++ b/internals-js/src/definitions.ts @@ -1064,7 +1064,7 @@ export class CoreFeatures { const feature = this.byAlias.get(splitted[0]); return feature ? { feature, - nameInFeature: splitted[1], + nameInFeature: splitted.slice(1).join('__'), isImported: false, } : undefined; } else { @@ -1076,7 +1076,7 @@ export class CoreFeatures { if ((as ?? name) === importName) { return { feature, - nameInFeature: name.slice(1), + nameInFeature: isDirective ? name.slice(1) : name, isImported: true, }; } @@ -1088,8 +1088,8 @@ export class CoreFeatures { if (directFeature && isDirective) { return { feature: directFeature, - nameInFeature: directFeature.imports.find(imp => imp.as === `@${element.name}`)?.name.slice(1) ?? element.name, - isImported: true, + nameInFeature: element.name, + isImported: false, }; } diff --git a/internals-js/src/extractSubgraphsFromSupergraph.ts b/internals-js/src/extractSubgraphsFromSupergraph.ts index 3d412d039..421c66bee 100644 --- a/internals-js/src/extractSubgraphsFromSupergraph.ts +++ b/internals-js/src/extractSubgraphsFromSupergraph.ts @@ -40,7 +40,7 @@ import { parseSelectionSet } from "./operations"; import fs from 'fs'; import path from 'path'; import { validateStringContainsBoolean } from "./utils"; -import { CONTEXT_VERSIONS, ContextSpecDefinition, DirectiveDefinition, FeatureUrl, FederationDirectiveName, SchemaElement, errorCauses, isFederationDirectiveDefinedInSchema, printErrors } from "."; +import { ContextSpecDefinition, CostSpecDefinition, SchemaElement, errorCauses, isFederationDirectiveDefinedInSchema, printErrors } from "."; function filteredTypes( supergraph: Schema, @@ -194,7 +194,7 @@ function typesUsedInFederationDirective(fieldSet: string | undefined, parentType } export function extractSubgraphsFromSupergraph(supergraph: Schema, validateExtractedSubgraphs: boolean = true): [Subgraphs, Map] { - const [coreFeatures, joinSpec] = validateSupergraph(supergraph); + const [coreFeatures, joinSpec, contextSpec, costSpec] = validateSupergraph(supergraph); const isFed1 = joinSpec.version.equals(new FeatureVersion(0, 1)); try { // We first collect the subgraphs (creating an empty schema that we'll populate next for each). @@ -224,13 +224,13 @@ export function extractSubgraphsFromSupergraph(supergraph: Schema, validateExtra } const types = filteredTypes(supergraph, joinSpec, coreFeatures.coreDefinition); - const originalDirectiveNames = getApolloDirectiveNames(supergraph); const args: ExtractArguments = { supergraph, subgraphs, joinSpec, + contextSpec, + costSpec, filteredTypes: types, - originalDirectiveNames, getSubgraph, getSubgraphEnumValue, }; @@ -293,8 +293,9 @@ type ExtractArguments = { supergraph: Schema, subgraphs: Subgraphs, joinSpec: JoinSpecDefinition, + contextSpec: ContextSpecDefinition | undefined, + costSpec: CostSpecDefinition | undefined, filteredTypes: NamedType[], - originalDirectiveNames: Record, getSubgraph: (application: Directive) => Subgraph | undefined, getSubgraphEnumValue: (subgraphName: string) => string } @@ -352,7 +353,9 @@ function addAllEmptySubgraphTypes(args: ExtractArguments): TypesInfo { const subgraph = getSubgraph(application); assert(subgraph, () => `Should have found the subgraph for ${application}`); const subgraphType = subgraph.schema.addType(newNamedType(type.kind, type.name)); - propagateDemandControlDirectives(type, subgraphType, subgraph, args.originalDirectiveNames); + if (args.costSpec) { + propagateDemandControlDirectives(type, subgraphType, subgraph, args.costSpec); + } } break; } @@ -401,17 +404,8 @@ function addEmptyType( } } } - - const coreFeatures = supergraph.coreFeatures; - assert(coreFeatures, 'Should have core features'); - const contextFeature = coreFeatures.getByIdentity(ContextSpecDefinition.identity); - let supergraphContextDirective: DirectiveDefinition<{ name: string}> | undefined; - if (contextFeature) { - const contextSpec = CONTEXT_VERSIONS.find(contextFeature.url.version); - assert(contextSpec, 'Should have context spec'); - supergraphContextDirective = contextSpec.contextDirective(supergraph); - } - + + const supergraphContextDirective = args.contextSpec?.contextDirective(supergraph); if (supergraphContextDirective) { const contextApplications = type.appliedDirectivesOf(supergraphContextDirective); // for every application, apply the context directive to the correct subgraph @@ -438,8 +432,6 @@ function extractObjOrItfContent(args: ExtractArguments, info: TypeInfo 1; for (const { type: subgraphType, subgraph } of subgraphsInfo.values()) { - addSubgraphField({ field, type: subgraphType, subgraph, isShareable, originalDirectiveNames }); + addSubgraphField({ + field, + type: subgraphType, + subgraph, + isShareable, + costSpec: args.costSpec + }); } } else { const isShareable = isObjectType(type) @@ -478,58 +478,21 @@ function extractObjOrItfContent(args: ExtractArguments, info: TypeInfo renamedCost` with the `@` prefix removed. - * - * If the directive is imported under its default name, that also results in an entry. So, - * ```graphql - * @link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@cost"]) - * ``` - * results in a map entry of `cost -> cost`. This duals as a way to check if a directive - * is included in the supergraph schema. - * - * **Important:** This map does _not_ include directives imported from identities other - * than `specs.apollo.dev`. This helps us avoid extracting directives to subgraphs - * when a custom directive's name conflicts with that of a default one. - */ -function getApolloDirectiveNames(supergraph: Schema): Record { - const originalDirectiveNames: Record = {}; - for (const linkDirective of supergraph.schemaDefinition.appliedDirectivesOf("link")) { - if (linkDirective.arguments().url && linkDirective.arguments().import) { - const url = FeatureUrl.maybeParse(linkDirective.arguments().url); - if (!url?.identity.includes("specs.apollo.dev")) { - continue; - } - - for (const importedDirective of linkDirective.arguments().import) { - if (importedDirective.name && importedDirective.as) { - originalDirectiveNames[importedDirective.name.replace('@', '')] = importedDirective.as.replace('@', ''); - } else if (typeof importedDirective === 'string') { - originalDirectiveNames[importedDirective.replace('@', '')] = importedDirective.replace('@', ''); + addSubgraphField({ + field, + type: subgraphType, + subgraph, isShareable, + joinFieldArgs, + costSpec: args.costSpec + }); } } } } - - return originalDirectiveNames; } function extractInputObjContent(args: ExtractArguments, info: TypeInfo[]) { const fieldDirective = args.joinSpec.fieldDirective(args.supergraph); - const originalDirectiveNames = args.originalDirectiveNames; for (const { type, subgraphsInfo } of info) { for (const field of type.fields()) { @@ -537,19 +500,30 @@ function extractInputObjContent(args: ExtractArguments, info: TypeInfo[]) { // This was added in join 0.3, so it can genuinely be undefined. const enumValueDirective = args.joinSpec.enumValueDirective(args.supergraph); - const originalDirectiveNames = args.originalDirectiveNames; for (const { type, subgraphsInfo } of info) { - for (const { type: subgraphType, subgraph } of subgraphsInfo.values()) { - propagateDemandControlDirectives(type, subgraphType, subgraph, originalDirectiveNames); + if (args.costSpec) { + for (const { type: subgraphType, subgraph } of subgraphsInfo.values()) { + propagateDemandControlDirectives(type, subgraphType, subgraph, args.costSpec); + } } for (const value of type.values) { @@ -678,20 +653,20 @@ function maybeDumpSubgraphSchema(subgraph: Subgraph): string { } } -function propagateDemandControlDirectives(source: SchemaElement, dest: SchemaElement, subgraph: Subgraph, originalDirectiveNames?: Record) { - const costDirectiveName = originalDirectiveNames?.[FederationDirectiveName.COST]; - if (costDirectiveName) { - const costDirective = source.appliedDirectivesOf(costDirectiveName).pop(); - if (costDirective) { - dest.applyDirective(subgraph.metadata().costDirective().name, costDirective.arguments()); +function propagateDemandControlDirectives(source: SchemaElement, dest: SchemaElement, subgraph: Subgraph, costSpec: CostSpecDefinition) { + const costDirective = costSpec.costDirective(source.schema()); + if (costDirective) { + const application = source.appliedDirectivesOf(costDirective)[0]; + if (application) { + dest.applyDirective(subgraph.metadata().costDirective().name, application.arguments()); } } - const listSizeDirectiveName = originalDirectiveNames?.[FederationDirectiveName.LIST_SIZE]; - if (listSizeDirectiveName) { - const listSizeDirective = source.appliedDirectivesOf(listSizeDirectiveName).pop(); - if (listSizeDirective) { - dest.applyDirective(subgraph.metadata().listSizeDirective().name, listSizeDirective.arguments()); + const listSizeDirective = costSpec.listSizeDirective(source.schema()); + if (listSizeDirective) { + const application = source.appliedDirectivesOf(listSizeDirective)[0]; + if (application) { + dest.applyDirective(subgraph.metadata().listSizeDirective().name, application.arguments()); } } } @@ -707,14 +682,14 @@ function addSubgraphField({ subgraph, isShareable, joinFieldArgs, - originalDirectiveNames, + costSpec, }: { field: FieldDefinition, type: ObjectType | InterfaceType, subgraph: Subgraph, isShareable: boolean, joinFieldArgs?: JoinFieldDirectiveArguments, - originalDirectiveNames?: Record, + costSpec?: CostSpecDefinition, }): FieldDefinition { const copiedFieldType = joinFieldArgs?.type ? decodeType(joinFieldArgs.type, subgraph.schema, subgraph.name) @@ -723,7 +698,9 @@ function addSubgraphField({ const subgraphField = type.addField(field.name, copiedFieldType); for (const arg of field.arguments()) { const argDef = subgraphField.addArgument(arg.name, copyType(arg.type!, subgraph.schema, subgraph.name), arg.defaultValue); - propagateDemandControlDirectives(arg, argDef, subgraph, originalDirectiveNames) + if (costSpec) { + propagateDemandControlDirectives(arg, argDef, subgraph, costSpec); + } } if (joinFieldArgs?.requires) { subgraphField.applyDirective(subgraph.metadata().requiresDirective(), {'fields': joinFieldArgs.requires}); @@ -769,7 +746,9 @@ function addSubgraphField({ subgraphField.applyDirective(subgraph.metadata().shareableDirective()); } - propagateDemandControlDirectives(field, subgraphField, subgraph, originalDirectiveNames); + if (costSpec) { + propagateDemandControlDirectives(field, subgraphField, subgraph, costSpec); + } return subgraphField; } @@ -779,13 +758,13 @@ function addSubgraphInputField({ type, subgraph, joinFieldArgs, - originalDirectiveNames, + costSpec, }: { field: InputFieldDefinition, type: InputObjectType, subgraph: Subgraph, joinFieldArgs?: JoinFieldDirectiveArguments, - originalDirectiveNames?: Record + costSpec?: CostSpecDefinition, }): InputFieldDefinition { const copiedType = joinFieldArgs?.type ? decodeType(joinFieldArgs?.type, subgraph.schema, subgraph.name) @@ -794,7 +773,9 @@ function addSubgraphInputField({ const inputField = type.addField(field.name, copiedType); inputField.defaultValue = field.defaultValue - propagateDemandControlDirectives(field, inputField, subgraph, originalDirectiveNames); + if (costSpec) { + propagateDemandControlDirectives(field, inputField, subgraph, costSpec); + } return inputField; } diff --git a/internals-js/src/specs/costSpec.ts b/internals-js/src/specs/costSpec.ts index f6f1bda54..ff15c9aa5 100644 --- a/internals-js/src/specs/costSpec.ts +++ b/internals-js/src/specs/costSpec.ts @@ -1,7 +1,7 @@ import { DirectiveLocation } from 'graphql'; import { createDirectiveSpecification } from '../directiveAndTypeSpecification'; import { FeatureDefinition, FeatureDefinitions, FeatureUrl, FeatureVersion } from './coreSpec'; -import { ListType, NonNullType } from '../definitions'; +import { DirectiveDefinition, ListType, NonNullType, Schema } from '../definitions'; import { registerKnownFeature } from '../knownCoreFeatures'; import { ARGUMENT_COMPOSITION_STRATEGIES } from '../argumentCompositionStrategies'; @@ -41,6 +41,14 @@ export class CostSpecDefinition extends FeatureDefinition { supergraphSpecification: (fedVersion) => COST_VERSIONS.getMinimumRequiredVersion(fedVersion) })); } + + costDirective(schema: Schema): DirectiveDefinition | undefined { + return this.directive(schema, 'cost'); + } + + listSizeDirective(schema: Schema): DirectiveDefinition | undefined { + return this.directive(schema, 'listSize'); + } } export const COST_VERSIONS = new FeatureDefinitions(costIdentity) diff --git a/internals-js/src/supergraphs.ts b/internals-js/src/supergraphs.ts index 3f37a103a..da4d52751 100644 --- a/internals-js/src/supergraphs.ts +++ b/internals-js/src/supergraphs.ts @@ -1,7 +1,9 @@ import { DocumentNode, GraphQLError } from "graphql"; -import { ErrCoreCheckFailed, FeatureUrl, FeatureVersion } from "./specs/coreSpec"; import { CoreFeatures, Schema, sourceASTs } from "./definitions"; +import { ErrCoreCheckFailed, FeatureUrl, FeatureVersion } from "./specs/coreSpec"; import { joinIdentity, JoinSpecDefinition, JOIN_VERSIONS } from "./specs/joinSpec"; +import { CONTEXT_VERSIONS, ContextSpecDefinition } from "./specs/contextSpec"; +import { COST_VERSIONS, costIdentity, CostSpecDefinition } from "./specs/costSpec"; import { buildSchema, buildSchemaFromAST } from "./buildSchema"; import { extractSubgraphsNamesAndUrlsFromSupergraph, extractSubgraphsFromSupergraph } from "./extractSubgraphsFromSupergraph"; import { ERRORS } from "./error"; @@ -81,11 +83,17 @@ function checkFeatureSupport(coreFeatures: CoreFeatures, supportedFeatures: Set< } } -export function validateSupergraph(supergraph: Schema): [CoreFeatures, JoinSpecDefinition] { +export function validateSupergraph(supergraph: Schema): [ + CoreFeatures, + JoinSpecDefinition, + ContextSpecDefinition | undefined, + CostSpecDefinition | undefined, +] { const coreFeatures = supergraph.coreFeatures; if (!coreFeatures) { throw ERRORS.INVALID_FEDERATION_SUPERGRAPH.err("Invalid supergraph: must be a core schema"); } + const joinFeature = coreFeatures.getByIdentity(joinIdentity); if (!joinFeature) { throw ERRORS.INVALID_FEDERATION_SUPERGRAPH.err("Invalid supergraph: must use the join spec"); @@ -95,7 +103,27 @@ export function validateSupergraph(supergraph: Schema): [CoreFeatures, JoinSpecD throw ERRORS.INVALID_FEDERATION_SUPERGRAPH.err( `Invalid supergraph: uses unsupported join spec version ${joinFeature.url.version} (supported versions: ${JOIN_VERSIONS.versions().join(', ')})`); } - return [coreFeatures, joinSpec]; + + const contextFeature = coreFeatures.getByIdentity(ContextSpecDefinition.identity); + let contextSpec = undefined; + if (contextFeature) { + contextSpec = CONTEXT_VERSIONS.find(contextFeature.url.version); + if (!contextSpec) { + throw ERRORS.INVALID_FEDERATION_SUPERGRAPH.err( + `Invalid supergraph: uses unsupported context spec version ${contextFeature.url.version} (supported versions: ${CONTEXT_VERSIONS.versions().join(', ')})`); + } + } + + const costFeature = coreFeatures.getByIdentity(costIdentity); + let costSpec = undefined; + if (costFeature) { + costSpec = COST_VERSIONS.find(costFeature.url.version); + if (!costSpec) { + throw ERRORS.INVALID_FEDERATION_SUPERGRAPH.err( + `Invalid supergraph: uses unsupported cost spec version ${costFeature.url.version} (supported versions: ${COST_VERSIONS.versions().join(', ')})`); + } + } + return [coreFeatures, joinSpec, contextSpec, costSpec]; } export function isFed1Supergraph(supergraph: Schema): boolean { diff --git a/internals-js/src/values.ts b/internals-js/src/values.ts index 11b2968e5..4796d5076 100644 --- a/internals-js/src/values.ts +++ b/internals-js/src/values.ts @@ -135,8 +135,10 @@ export function valueEquals(a: any, b: any): boolean { if (Array.isArray(a)) { return Array.isArray(b) && arrayValueEquals(a, b) ; } - if (typeof a === 'object') { - return typeof b === 'object' && objectEquals(a, b); + // Note that typeof null === 'object', so we have to manually rule that out + // here. + if (a !== null && typeof a === 'object') { + return b !== null && typeof b === 'object' && objectEquals(a, b); } return a === b; } @@ -249,7 +251,7 @@ export function withDefaultValues(value: any, argument: ArgumentDefinition) throw buildError(`Cannot compute default value for argument ${argument} as the type is undefined`); } if (value === undefined) { - if (argument.defaultValue) { + if (argument.defaultValue !== undefined) { return applyDefaultValues(argument.defaultValue, argument.type); } }