From fd250fc3c8248b61a36354d7b7d82fc2f1f80191 Mon Sep 17 00:00:00 2001 From: James Bellenger Date: Tue, 17 Feb 2026 04:21:07 -0800 Subject: [PATCH 01/17] check oneof inhabitability --- src/type/__tests__/validation-test.ts | 193 ++++++++++++++++++++++++++ src/type/validate.ts | 65 +++++++++ 2 files changed, 258 insertions(+) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 8973013fa1..3d35379466 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -2410,6 +2410,199 @@ describe('Type System: OneOf Input Object fields must be nullable', () => { }); }); +describe('Type System: OneOf Input Objects must be inhabitable', () => { + it('accepts a OneOf Input Object with a scalar field', () => { + const schema = buildSchema(` + type Query { + test(arg: A): String + } + + input A @oneOf { + a: String + b: Int + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf Input Object with an enum field', () => { + const schema = buildSchema(` + type Query { + test(arg: A): String + } + + enum Color { RED GREEN BLUE } + + input A @oneOf { + a: Color + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf Input Object with a list field', () => { + const schema = buildSchema(` + type Query { + test(arg: A): String + } + + input A @oneOf { + a: [A] + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf Input Object referencing a non-OneOf input object', () => { + const schema = buildSchema(` + type Query { + test(arg: A): String + } + + input A @oneOf { + a: RegularInput + } + + input RegularInput { + x: String + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf Input Object with at least one escape field', () => { + const schema = buildSchema(` + type Query { + test(arg: A): String + } + + input A @oneOf { + b: B + escape: String + } + + input B @oneOf { + a: A + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts mutually referencing OneOf types where one has a scalar escape', () => { + const schema = buildSchema(` + type Query { + test(arg: A): String + } + + input A @oneOf { + b: B + } + + input B @oneOf { + a: A + escape: Int + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf referencing a non-OneOf which references back', () => { + const schema = buildSchema(` + type Query { + test(arg: A): String + } + + input A @oneOf { + b: RegularInput + } + + input RegularInput { + back: A + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf with multiple fields where one escapes through chained OneOf types', () => { + const schema = buildSchema(` + type Query { + test(arg: A): String + } + + input A @oneOf { + b: B + c: C + } + + input B @oneOf { + a: A + } + + input C @oneOf { + a: A + escape: String + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('rejects a closed subgraph of one OneOf type', () => { + const schema = buildSchema(` + type Query { + test(arg: A): String + } + + input A @oneOf { + self: A + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'OneOf Input Object A must be inhabitable but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle.', + locations: [{ line: 6, column: 7 }], + }, + ]); + }); + + it('rejects a closed subgraph of multiple OneOf types', () => { + const schema = buildSchema(` + type Query { + test(arg: A): String + } + + input A @oneOf { + b: B + } + + input B @oneOf { + c: C + } + + input C @oneOf { + a: A + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'OneOf Input Object A must be inhabitable but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle.', + locations: [{ line: 6, column: 7 }], + }, + { + message: + 'OneOf Input Object B must be inhabitable but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle.', + locations: [{ line: 10, column: 7 }], + }, + { + message: + 'OneOf Input Object C must be inhabitable but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle.', + locations: [{ line: 14, column: 7 }], + }, + ]); + }); +}); + describe('Objects must adhere to Interface they implement', () => { it('accepts an Object which implements an Interface', () => { const schema = buildSchema(` diff --git a/src/type/validate.ts b/src/type/validate.ts index 4579901e2e..851619d95e 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -410,6 +410,8 @@ function validateTypes(context: SchemaValidationContext): void { createInputObjectNonNullCircularRefsValidator(context); const validateInputObjectDefaultValueCircularRefs = createInputObjectDefaultValueCircularRefsValidator(context); + const validateOneOfInputObjectInhabitability = + createOneOfInputObjectInhabitabilityValidator(context); const typeMap = context.schema.getTypeMap(); for (const type of Object.values(typeMap)) { // Ensure all provided types are in fact GraphQL type. @@ -454,6 +456,11 @@ function validateTypes(context: SchemaValidationContext): void { // Ensure Input Objects do not contain invalid default value circular references. validateInputObjectDefaultValueCircularRefs(type); + + // Ensure OneOf Input Objects are inhabitable. + if (type.isOneOf) { + validateOneOfInputObjectInhabitability(type); + } } } } @@ -988,6 +995,64 @@ function createInputObjectDefaultValueCircularRefsValidator( } } +function createOneOfInputObjectInhabitabilityValidator( + context: SchemaValidationContext, +): (inputObj: GraphQLInputObjectType) => void { + // Tracks already validated types to maintain O(N) across top-level calls. + const visitedTypes = new Set(); + + return function validateOneOfInputObjectInhabitability( + inputObj: GraphQLInputObjectType, + ): void { + if (visitedTypes.has(inputObj)) { + return; + } + + if (!isInhabitable(inputObj, new Set())) { + context.reportError( + `OneOf Input Object ${inputObj} must be inhabitable but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle.`, + inputObj.astNode, + ); + } + + visitedTypes.add(inputObj); + }; + + function isInhabitable( + inputObj: GraphQLInputObjectType, + visited: ReadonlySet, + ): boolean { + if (visited.has(inputObj)) { + return false; + } + + const nextVisited = new Set(visited); + nextVisited.add(inputObj); + + for (const field of Object.values(inputObj.getFields())) { + if (isListType(field.type)) { + return true; + } + + const namedType = getNamedType(field.type); + + if (!isInputObjectType(namedType)) { + return true; + } + + if (!namedType.isOneOf) { + return true; + } + + if (isInhabitable(namedType, nextVisited)) { + return true; + } + } + + return false; + } +} + function getAllImplementsInterfaceNodes( type: GraphQLObjectType | GraphQLInterfaceType, iface: GraphQLInterfaceType, From 7f5887efdbb16627ef3d27dbd68958f0e4466636 Mon Sep 17 00:00:00 2001 From: James Bellenger Date: Sat, 28 Feb 2026 04:51:24 -0800 Subject: [PATCH 02/17] Detect uninhabitable input types formed by OneOf and non-OneOf cycles Rework the circular references validation to detect input object types that cannot be provided a finite value. This covers: - Self-recursive OneOf types (e.g. input A @oneOf { a: A }) - Mixed OneOf/non-OneOf cycles with no escape path - Standard non-null circular references (existing behavior preserved) Rename algorithms to match spec terminology: - InputObjectHasUnbreakableCycle (was InputObjectCanBeProvidedAFiniteValue) - InputFieldTypeHasUnbreakableCycle (was FieldTypeCanBeProvidedAFiniteValue) --- src/type/__tests__/validation-test.ts | 207 +++++++++++++------- src/type/validate.ts | 259 ++++++++++++++------------ 2 files changed, 277 insertions(+), 189 deletions(-) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 3d35379466..34365d8a54 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -925,7 +925,7 @@ describe('Type System: Input Objects must have fields', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Invalid circular reference. The Input Object SomeInputObject references itself in the non-null field SomeInputObject.nonNullSelf.', + 'Input Object SomeInputObject references itself via the required fields: SomeInputObject.nonNullSelf.', locations: [{ line: 7, column: 9 }], }, ]); @@ -953,13 +953,31 @@ describe('Type System: Input Objects must have fields', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Invalid circular reference. The Input Object SomeInputObject references itself via the non-null fields: SomeInputObject.startLoop, AnotherInputObject.nextInLoop, YetAnotherInputObject.closeLoop.', + 'Input Object SomeInputObject references itself via the required fields: SomeInputObject.startLoop, AnotherInputObject.nextInLoop, YetAnotherInputObject.closeLoop.', locations: [ { line: 7, column: 9 }, { line: 11, column: 9 }, { line: 15, column: 9 }, ], }, + { + message: + 'Input Object AnotherInputObject references itself via the required fields: AnotherInputObject.nextInLoop, YetAnotherInputObject.closeLoop, SomeInputObject.startLoop.', + locations: [ + { line: 11, column: 9 }, + { line: 15, column: 9 }, + { line: 7, column: 9 }, + ], + }, + { + message: + 'Input Object YetAnotherInputObject references itself via the required fields: YetAnotherInputObject.closeLoop, SomeInputObject.startLoop, AnotherInputObject.nextInLoop.', + locations: [ + { line: 15, column: 9 }, + { line: 7, column: 9 }, + { line: 11, column: 9 }, + ], + }, ]); }); @@ -987,7 +1005,7 @@ describe('Type System: Input Objects must have fields', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Invalid circular reference. The Input Object SomeInputObject references itself via the non-null fields: SomeInputObject.startLoop, AnotherInputObject.closeLoop.', + 'Input Object SomeInputObject references itself via the required fields: SomeInputObject.startLoop, AnotherInputObject.closeLoop.', locations: [ { line: 7, column: 9 }, { line: 11, column: 9 }, @@ -995,16 +1013,20 @@ describe('Type System: Input Objects must have fields', () => { }, { message: - 'Invalid circular reference. The Input Object AnotherInputObject references itself via the non-null fields: AnotherInputObject.startSecondLoop, YetAnotherInputObject.closeSecondLoop.', + 'Input Object AnotherInputObject references itself via the required fields: AnotherInputObject.closeLoop, SomeInputObject.startLoop.', locations: [ - { line: 12, column: 9 }, - { line: 16, column: 9 }, + { line: 11, column: 9 }, + { line: 7, column: 9 }, ], }, { message: - 'Invalid circular reference. The Input Object YetAnotherInputObject references itself in the non-null field YetAnotherInputObject.nonNullSelf.', - locations: [{ line: 17, column: 9 }], + 'Input Object YetAnotherInputObject references itself via the required fields: YetAnotherInputObject.closeSecondLoop, AnotherInputObject.closeLoop, SomeInputObject.startLoop.', + locations: [ + { line: 16, column: 9 }, + { line: 11, column: 9 }, + { line: 7, column: 9 }, + ], }, ]); }); @@ -2410,173 +2432,202 @@ describe('Type System: OneOf Input Object fields must be nullable', () => { }); }); -describe('Type System: OneOf Input Objects must be inhabitable', () => { +describe('Type System: Input Objects must not have unbreakable cycles', () => { it('accepts a OneOf Input Object with a scalar field', () => { const schema = buildSchema(` type Query { - test(arg: A): String + test(arg: A): Int } input A @oneOf { - a: String - b: Int + a: Int } `); expectJSON(validateSchema(schema)).toDeepEqual([]); }); - it('accepts a OneOf Input Object with an enum field', () => { + it('accepts a OneOf Input Object with a recursive list field', () => { const schema = buildSchema(` type Query { - test(arg: A): String + test(arg: A): Int } - enum Color { RED GREEN BLUE } - input A @oneOf { - a: Color + a: [A!] } `); expectJSON(validateSchema(schema)).toDeepEqual([]); }); - it('accepts a OneOf Input Object with a list field', () => { + it('accepts a OneOf Input Object referencing a non-OneOf input object', () => { const schema = buildSchema(` type Query { - test(arg: A): String + test(arg: A): Int } input A @oneOf { - a: [A] + b: B + } + + input B { + x: Int } `); expectJSON(validateSchema(schema)).toDeepEqual([]); }); - it('accepts a OneOf Input Object referencing a non-OneOf input object', () => { + it('accepts a OneOf/OneOf cycle with a scalar escape', () => { const schema = buildSchema(` type Query { - test(arg: A): String + test(arg: A): Int } input A @oneOf { - a: RegularInput + b: B + escape: Int } - input RegularInput { - x: String + input B @oneOf { + a: A } `); expectJSON(validateSchema(schema)).toDeepEqual([]); }); - it('accepts a OneOf Input Object with at least one escape field', () => { + it('accepts a OneOf/non-OneOf cycle with a nullable escape', () => { const schema = buildSchema(` type Query { - test(arg: A): String + test(arg: A): Int } input A @oneOf { b: B - escape: String } - input B @oneOf { + input B { a: A } `); expectJSON(validateSchema(schema)).toDeepEqual([]); }); - it('accepts mutually referencing OneOf types where one has a scalar escape', () => { + it('rejects a self-referencing OneOf type with no escapes', () => { const schema = buildSchema(` type Query { - test(arg: A): String + test(arg: A): Int } input A @oneOf { - b: B - } - - input B @oneOf { - a: A - escape: Int + self: A } `); - expectJSON(validateSchema(schema)).toDeepEqual([]); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Input Object A references itself via the required fields: A.self.', + locations: [{ line: 7, column: 9 }], + }, + ]); }); - it('accepts a OneOf referencing a non-OneOf which references back', () => { + it('rejects a mixed OneOf/non-OneOf cycle with no escapes', () => { const schema = buildSchema(` type Query { - test(arg: A): String + test(arg: A): Int } input A @oneOf { - b: RegularInput + b: B } - input RegularInput { - back: A + input B { + a: A! } `); - expectJSON(validateSchema(schema)).toDeepEqual([]); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Input Object A references itself via the required fields: A.b, B.a.', + locations: [ + { line: 7, column: 9 }, + { line: 11, column: 9 }, + ], + }, + { + message: + 'Input Object B references itself via the required fields: B.a, A.b.', + locations: [ + { line: 11, column: 9 }, + { line: 7, column: 9 }, + ], + }, + ]); }); - it('accepts a OneOf with multiple fields where one escapes through chained OneOf types', () => { + it('accepts a OneOf/non-OneOf with scalar escape', () => { const schema = buildSchema(` type Query { - test(arg: A): String + test(arg: A): Int } input A @oneOf { b: B - c: C + escape: Int } - input B @oneOf { - a: A + input B { + a: A! } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); - input C @oneOf { + it('accepts a non-OneOf/non-OneOf cycle with a nullable escape', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A { + b: B! + } + + input B { a: A - escape: String } `); expectJSON(validateSchema(schema)).toDeepEqual([]); }); - it('rejects a closed subgraph of one OneOf type', () => { + it('accepts a non-OneOf/non-OneOf cycle with a list escape', () => { const schema = buildSchema(` type Query { - test(arg: A): String + test(arg: A): Int } - input A @oneOf { - self: A + input A { + b: [B!]! + } + + input B { + a: A! } `); - expectJSON(validateSchema(schema)).toDeepEqual([ - { - message: - 'OneOf Input Object A must be inhabitable but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle.', - locations: [{ line: 6, column: 7 }], - }, - ]); + expectJSON(validateSchema(schema)).toDeepEqual([]); }); - it('rejects a closed subgraph of multiple OneOf types', () => { + it('rejects a larger mixed OneOf/non-OneOf cycle with no escapes', () => { const schema = buildSchema(` type Query { - test(arg: A): String + test(arg: A): Int } input A @oneOf { b: B } - input B @oneOf { - c: C + input B { + c: C! } input C @oneOf { @@ -2586,18 +2637,30 @@ describe('Type System: OneOf Input Objects must be inhabitable', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'OneOf Input Object A must be inhabitable but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle.', - locations: [{ line: 6, column: 7 }], + 'Input Object A references itself via the required fields: A.b, B.c, C.a.', + locations: [ + { line: 7, column: 9 }, + { line: 11, column: 9 }, + { line: 15, column: 9 }, + ], }, { message: - 'OneOf Input Object B must be inhabitable but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle.', - locations: [{ line: 10, column: 7 }], + 'Input Object B references itself via the required fields: B.c, C.a, A.b.', + locations: [ + { line: 11, column: 9 }, + { line: 15, column: 9 }, + { line: 7, column: 9 }, + ], }, { message: - 'OneOf Input Object C must be inhabitable but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle.', - locations: [{ line: 14, column: 7 }], + 'Input Object C references itself via the required fields: C.a, A.b, B.c.', + locations: [ + { line: 15, column: 9 }, + { line: 7, column: 9 }, + { line: 11, column: 9 }, + ], }, ]); }); diff --git a/src/type/validate.ts b/src/type/validate.ts index 851619d95e..688d5ae73d 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -405,14 +405,15 @@ function validateName( } function validateTypes(context: SchemaValidationContext): void { - // Ensure Input Objects do not contain non-nullable circular references. - const validateInputObjectNonNullCircularRefs = - createInputObjectNonNullCircularRefsValidator(context); + const inputObjectUnbreakableCycleCheck = + createInputObjectUnbreakableCycleCheck(); const validateInputObjectDefaultValueCircularRefs = createInputObjectDefaultValueCircularRefsValidator(context); - const validateOneOfInputObjectInhabitability = - createOneOfInputObjectInhabitabilityValidator(context); const typeMap = context.schema.getTypeMap(); + + // Collect Input Object types that have unbreakable cycles. + const typesWithUnbreakableCycles = new Set(); + for (const type of Object.values(typeMap)) { // Ensure all provided types are in fact GraphQL type. if (!isNamedType(type)) { @@ -450,19 +451,25 @@ function validateTypes(context: SchemaValidationContext): void { // Ensure Input Object fields are valid. validateInputFields(context, type); - // Ensure Input Objects do not contain invalid field circular references. - // Ensure Input Objects do not contain non-nullable circular references. - validateInputObjectNonNullCircularRefs(type); + // Ensure Input Objects do not have unbreakable cycles. + if (inputObjectUnbreakableCycleCheck(type)) { + typesWithUnbreakableCycles.add(type); + } // Ensure Input Objects do not contain invalid default value circular references. validateInputObjectDefaultValueCircularRefs(type); - - // Ensure OneOf Input Objects are inhabitable. - if (type.isOneOf) { - validateOneOfInputObjectInhabitability(type); - } } } + + // Report errors for Input Object types that have unbreakable cycles. + for (const type of typesWithUnbreakableCycles) { + const cyclePath = traceUnbreakableCycle(type, typesWithUnbreakableCycles); + const pathStr = cyclePath.map((p) => p.fieldStr).join(', '); + context.reportError( + `Input Object ${type} references itself via the required fields: ${pathStr}.`, + cyclePath.map((p) => p.astNode), + ); + } } function validateFields( @@ -778,66 +785,142 @@ function validateOneOfInputObjectField( } } -function createInputObjectNonNullCircularRefsValidator( - context: SchemaValidationContext, -): (inputObj: GraphQLInputObjectType) => void { - // Modified copy of algorithm from 'src/validation/rules/NoFragmentCycles.js'. - // Tracks already visited types to maintain O(N) and to ensure that cycles - // are not redundantly reported. - const visitedTypes = new Set(); +// Implements the spec's InputObjectHasUnbreakableCycle algorithm. +// Tracks already checked types to maintain O(N) and to ensure that types +// are not redundantly checked. +function createInputObjectUnbreakableCycleCheck(): ( + inputObj: GraphQLInputObjectType, +) => boolean { + const knownNoCycle = new Set(); + const visited = new Set(); - // Array of types nodes used to produce meaningful errors - const fieldPath: Array<{ fieldStr: string; astNode: Maybe }> = []; + return inputObjectHasUnbreakableCycle; - // Position in the type path - const fieldPathIndexByTypeName: ObjMap = - Object.create(null); + function inputObjectHasUnbreakableCycle( + inputObj: GraphQLInputObjectType, + ): boolean { + if (knownNoCycle.has(inputObj)) { + return false; + } + if (visited.has(inputObj)) { + return true; + } - return detectCycleRecursive; + visited.add(inputObj); - // This does a straight-forward DFS to find cycles. - // It does not terminate when a cycle was found but continues to explore - // the graph to find all possible cycles. - function detectCycleRecursive(inputObj: GraphQLInputObjectType): void { - if (visitedTypes.has(inputObj)) { - return; + let result: boolean; + + if (inputObj.isOneOf) { + // OneOf Input Objects have an unbreakable cycle if every field has one. + result = true; + for (const field of Object.values(inputObj.getFields())) { + if (!inputFieldTypeHasUnbreakableCycle(field.type)) { + result = false; + break; + } + } + } else { + // Normal Input Objects have an unbreakable cycle if any non-null field has one. + result = false; + for (const field of Object.values(inputObj.getFields())) { + if ( + isNonNullType(field.type) && + inputFieldTypeHasUnbreakableCycle(field.type.ofType) + ) { + result = true; + break; + } + } } - visitedTypes.add(inputObj); - fieldPathIndexByTypeName[inputObj.name] = fieldPath.length; + visited.delete(inputObj); - const fields = Object.values(inputObj.getFields()); - for (const field of fields) { - if (isNonNullType(field.type) && isInputObjectType(field.type.ofType)) { - const fieldType = field.type.ofType; - const cycleIndex = fieldPathIndexByTypeName[fieldType.name]; + if (!result) { + knownNoCycle.add(inputObj); + } + return result; + } - fieldPath.push({ - fieldStr: `${inputObj}.${field.name}`, + function inputFieldTypeHasUnbreakableCycle( + fieldType: GraphQLInputType, + ): boolean { + if (isListType(fieldType)) { + return false; + } + if (isNonNullType(fieldType)) { + return inputFieldTypeHasUnbreakableCycle(fieldType.ofType); + } + if (!isInputObjectType(fieldType)) { + return false; + } + return inputObjectHasUnbreakableCycle(fieldType); + } +} + +// For an Input Object type with an unbreakable cycle, traces a witness cycle +// path by following required edges to other types with unbreakable cycles. +function traceUnbreakableCycle( + startType: GraphQLInputObjectType, + typesWithUnbreakableCycles: ReadonlySet, +): Array<{ fieldStr: string; astNode: Maybe }> { + const path: Array<{ fieldStr: string; astNode: Maybe }> = []; + const seen = new Set(); + + let current: Maybe = startType; + while (current != null && !seen.has(current)) { + seen.add(current); + let next: Maybe; + + for (const field of Object.values(current.getFields())) { + let target: Maybe; + + if (current.isOneOf) { + if ( + isInputObjectType(field.type) && + typesWithUnbreakableCycles.has(field.type) + ) { + target = field.type; + } + } else if (isNonNullType(field.type)) { + target = unwrapToUnbreakableCycleType( + field.type.ofType, + typesWithUnbreakableCycles, + ); + } + + if (target != null) { + path.push({ + fieldStr: `${current}.${field.name}`, astNode: field.astNode, }); - if (cycleIndex === undefined) { - detectCycleRecursive(fieldType); - } else { - const cyclePath = fieldPath.slice(cycleIndex); - const pathStr = cyclePath - .map((fieldObj) => fieldObj.fieldStr) - .join(', '); - context.reportError( - `Invalid circular reference. The Input Object ${fieldType} references itself ${ - cyclePath.length > 1 - ? 'via the non-null fields:' - : 'in the non-null field' - } ${pathStr}.`, - cyclePath.map((fieldObj) => fieldObj.astNode), - ); - } - fieldPath.pop(); + next = target; + break; } } - fieldPathIndexByTypeName[inputObj.name] = undefined; + current = next; + } + + return path; +} + +function unwrapToUnbreakableCycleType( + type: GraphQLInputType, + typesWithUnbreakableCycles: ReadonlySet, +): Maybe { + if (isListType(type)) { + return undefined; + } + if (isNonNullType(type)) { + return unwrapToUnbreakableCycleType( + type.ofType, + typesWithUnbreakableCycles, + ); + } + if (isInputObjectType(type) && typesWithUnbreakableCycles.has(type)) { + return type; } + return undefined; } function createInputObjectDefaultValueCircularRefsValidator( @@ -995,64 +1078,6 @@ function createInputObjectDefaultValueCircularRefsValidator( } } -function createOneOfInputObjectInhabitabilityValidator( - context: SchemaValidationContext, -): (inputObj: GraphQLInputObjectType) => void { - // Tracks already validated types to maintain O(N) across top-level calls. - const visitedTypes = new Set(); - - return function validateOneOfInputObjectInhabitability( - inputObj: GraphQLInputObjectType, - ): void { - if (visitedTypes.has(inputObj)) { - return; - } - - if (!isInhabitable(inputObj, new Set())) { - context.reportError( - `OneOf Input Object ${inputObj} must be inhabitable but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle.`, - inputObj.astNode, - ); - } - - visitedTypes.add(inputObj); - }; - - function isInhabitable( - inputObj: GraphQLInputObjectType, - visited: ReadonlySet, - ): boolean { - if (visited.has(inputObj)) { - return false; - } - - const nextVisited = new Set(visited); - nextVisited.add(inputObj); - - for (const field of Object.values(inputObj.getFields())) { - if (isListType(field.type)) { - return true; - } - - const namedType = getNamedType(field.type); - - if (!isInputObjectType(namedType)) { - return true; - } - - if (!namedType.isOneOf) { - return true; - } - - if (isInhabitable(namedType, nextVisited)) { - return true; - } - } - - return false; - } -} - function getAllImplementsInterfaceNodes( type: GraphQLObjectType | GraphQLInterfaceType, iface: GraphQLInterfaceType, From 9c54e796970dd26417a7409f81e521eaba61462b Mon Sep 17 00:00:00 2001 From: James Bellenger Date: Thu, 7 May 2026 05:03:28 -0700 Subject: [PATCH 03/17] simplify type unwrapping and add test to satisfy coverage checker --- src/type/__tests__/validation-test.ts | 36 +++++++++++++++++++++++++++ src/type/validate.ts | 33 ++++++------------------ 2 files changed, 43 insertions(+), 26 deletions(-) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 34365d8a54..49cb634944 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -2616,6 +2616,42 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { expectJSON(validateSchema(schema)).toDeepEqual([]); }); + it('rejects a non-OneOf/non-OneOf cycle with required scalar and list fields', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A { + list: [B]! + b: B! + } + + input B { + value: Int! + a: A! + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Input Object A references itself via the required fields: A.b, B.a.', + locations: [ + { line: 8, column: 9 }, + { line: 13, column: 9 }, + ], + }, + { + message: + 'Input Object B references itself via the required fields: B.a, A.b.', + locations: [ + { line: 13, column: 9 }, + { line: 8, column: 9 }, + ], + }, + ]); + }); + it('rejects a larger mixed OneOf/non-OneOf cycle with no escapes', () => { const schema = buildSchema(` type Query { diff --git a/src/type/validate.ts b/src/type/validate.ts index 688d5ae73d..a56ec8a4e6 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -847,9 +847,6 @@ function createInputObjectUnbreakableCycleCheck(): ( if (isListType(fieldType)) { return false; } - if (isNonNullType(fieldType)) { - return inputFieldTypeHasUnbreakableCycle(fieldType.ofType); - } if (!isInputObjectType(fieldType)) { return false; } @@ -882,10 +879,13 @@ function traceUnbreakableCycle( target = field.type; } } else if (isNonNullType(field.type)) { - target = unwrapToUnbreakableCycleType( - field.type.ofType, - typesWithUnbreakableCycles, - ); + const nullableType = field.type.ofType; + if ( + isInputObjectType(nullableType) && + typesWithUnbreakableCycles.has(nullableType) + ) { + target = nullableType; + } } if (target != null) { @@ -904,25 +904,6 @@ function traceUnbreakableCycle( return path; } -function unwrapToUnbreakableCycleType( - type: GraphQLInputType, - typesWithUnbreakableCycles: ReadonlySet, -): Maybe { - if (isListType(type)) { - return undefined; - } - if (isNonNullType(type)) { - return unwrapToUnbreakableCycleType( - type.ofType, - typesWithUnbreakableCycles, - ); - } - if (isInputObjectType(type) && typesWithUnbreakableCycles.has(type)) { - return type; - } - return undefined; -} - function createInputObjectDefaultValueCircularRefsValidator( context: SchemaValidationContext, ): (inputObj: GraphQLInputObjectType) => void { From 1cea080fc1d4f8e9667afb84c9fa9d13caff610b Mon Sep 17 00:00:00 2001 From: James Bellenger Date: Thu, 7 May 2026 10:11:05 -0700 Subject: [PATCH 04/17] perf and style - add memoization to improve validation performance - graceful handling of empty oneof defintions - add missing test coverage --- src/type/__tests__/validation-test.ts | 151 ++++++++++++++++++++ src/type/validate.ts | 190 ++++++++++++++++++++------ 2 files changed, 303 insertions(+), 38 deletions(-) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 49cb634944..6cfd6eb22f 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -4,6 +4,7 @@ import { assert, expect } from 'chai'; import { dedent } from '../../__testUtils__/dedent.ts'; import { expectJSON } from '../../__testUtils__/expectJSON.ts'; +import { spyOnMethod } from '../../__testUtils__/spyOn.ts'; import { inspect } from '../../jsutils/inspect.ts'; @@ -889,6 +890,24 @@ describe('Type System: Input Objects must have fields', () => { ]); }); + it('rejects a OneOf Input Object type with missing fields', () => { + const schema = buildSchema(` + type Query { + field(arg: SomeInputObject): String + } + + input SomeInputObject @oneOf + `); + + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Input Object type SomeInputObject must define one or more fields.', + locations: [{ line: 6, column: 7 }], + }, + ]); + }); + it('accepts an Input Object with breakable circular reference', () => { const schema = buildSchema(` type Query { @@ -2476,6 +2495,64 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { expectJSON(validateSchema(schema)).toDeepEqual([]); }); + it('accepts a OneOf Input Object referencing an already checked input object', () => { + const schema = buildSchema(` + type Query { + b(arg: B): Int + a(arg: A): Int + } + + input B { + value: Int + } + + input A @oneOf { + b: B + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf Input Object with multiple acyclic input object fields', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A @oneOf { + b: B + c: C + } + + input B { + value: Int + } + + input C { + value: Int + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf Input Object with an input object field and scalar escape', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A @oneOf { + b: B + escape: Int + } + + input B { + value: Int + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + it('accepts a OneOf/OneOf cycle with a scalar escape', () => { const schema = buildSchema(` type Query { @@ -2530,6 +2607,80 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { ]); }); + it('rejects a normal Input Object requiring an unbreakable OneOf cycle', () => { + const schema = buildSchema(` + type Query { + t(arg: T): Int + a(arg: A): Int + } + + input T @oneOf { + self: T + } + + input A { + t: T! + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Input Object T references itself via the required fields: T.self.', + locations: [{ line: 8, column: 9 }], + }, + { + message: + 'Input Object A references itself via the required fields: A.t, T.self.', + locations: [ + { line: 12, column: 9 }, + { line: 8, column: 9 }, + ], + }, + ]); + }); + + it('caches shared unbreakable OneOf subgraphs', () => { + const chainLength = 16; + const types: Array = []; + types[0] = new GraphQLInputObjectType({ + name: 'T0', + isOneOf: true, + fields: () => ({ self: { type: types[0] } }), + }); + + for (let i = 1; i <= chainLength; ++i) { + const previousType = types[i - 1]; + types[i] = new GraphQLInputObjectType({ + name: `T${i}`, + isOneOf: true, + fields: { + a: { type: previousType }, + b: { type: previousType }, + }, + }); + } + + const getFieldsSpies = types.map((type) => spyOnMethod(type, 'getFields')); + + const schema = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'Query', + fields: { + test: { + type: GraphQLInt, + args: { input: { type: types[chainLength] } }, + }, + }, + }), + types, + }); + + expect(validateSchema(schema)).to.have.lengthOf(types.length); + expect( + getFieldsSpies.reduce((sum, spy) => sum + spy.callCount, 0), + ).to.be.lessThan(500); + }); + it('rejects a mixed OneOf/non-OneOf cycle with no escapes', () => { const schema = buildSchema(` type Query { diff --git a/src/type/validate.ts b/src/type/validate.ts index a56ec8a4e6..5f89d03176 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -786,71 +786,185 @@ function validateOneOfInputObjectField( } // Implements the spec's InputObjectHasUnbreakableCycle algorithm. -// Tracks already checked types to maintain O(N) and to ensure that types -// are not redundantly checked. +// Tracks already checked types to ensure that types are not redundantly checked. function createInputObjectUnbreakableCycleCheck(): ( inputObj: GraphQLInputObjectType, ) => boolean { - const knownNoCycle = new Set(); - const visited = new Set(); + const cycleMemo = new Map(); return inputObjectHasUnbreakableCycle; function inputObjectHasUnbreakableCycle( inputObj: GraphQLInputObjectType, ): boolean { - if (knownNoCycle.has(inputObj)) { - return false; + const cachedResult = cycleMemo.get(inputObj); + if (cachedResult !== undefined) { + return cachedResult; } - if (visited.has(inputObj)) { - return true; + + // Unknown reachable types mapped to types that depend on them. + const candidates = new AccumulatorMap< + GraphQLInputObjectType, + GraphQLInputObjectType + >(); + for (const candidate of collectReachableInputObjects(inputObj)) { + candidates.set(candidate, []); } - visited.add(inputObj); + // Tracks unresolved fields for normal Input Objects. + const normalTypeStates = new Map< + GraphQLInputObjectType, + { hasKnownCycleField: boolean; unresolvedFieldCount: number } + >(); - let result: boolean; + // Types proven not to have cycles and ready to remove. + const typesToRemove: Array = []; - if (inputObj.isOneOf) { - // OneOf Input Objects have an unbreakable cycle if every field has one. - result = true; - for (const field of Object.values(inputObj.getFields())) { - if (!inputFieldTypeHasUnbreakableCycle(field.type)) { - result = false; - break; + for (const candidate of candidates.keys()) { + const fields = Object.values(candidate.getFields()); + + if (candidate.isOneOf) { + // OneOf Input Objects have an unbreakable cycle if every field leads to an unbreakable cycle. + if (fields.length === 0) { + typesToRemove.push(candidate); + continue; } - } - } else { - // Normal Input Objects have an unbreakable cycle if any non-null field has one. - result = false; - for (const field of Object.values(inputObj.getFields())) { - if ( - isNonNullType(field.type) && - inputFieldTypeHasUnbreakableCycle(field.type.ofType) - ) { - result = true; - break; + + for (const field of fields) { + const target = getUnbreakableCycleTarget(candidate, field.type); + + if (target == null) { + typesToRemove.push(candidate); + break; + } + + const targetResult = cycleMemo.get(target); + if (targetResult === false) { + typesToRemove.push(candidate); + break; + } + + if (targetResult === undefined) { + candidates.add(target, candidate); + } + } + } else { + // Normal Input Objects have an unbreakable cycle if any non-null field has one. + let hasKnownCycleField = false; + let unresolvedFieldCount = 0; + + for (const field of fields) { + const target = getUnbreakableCycleTarget(candidate, field.type); + + if (target == null) { + continue; + } + + const targetResult = cycleMemo.get(target); + if (targetResult === false) { + continue; + } + + if (targetResult === true) { + hasKnownCycleField = true; + } else { + ++unresolvedFieldCount; + candidates.add(target, candidate); + } + } + + normalTypeStates.set(candidate, { + hasKnownCycleField, + unresolvedFieldCount, + }); + + if (!hasKnownCycleField && unresolvedFieldCount === 0) { + typesToRemove.push(candidate); } } } - visited.delete(inputObj); + while (typesToRemove.length > 0) { + const type = typesToRemove.pop(); + invariant(type != null); + + const dependents = candidates.get(type); + if (dependents === undefined) { + continue; + } + + candidates.delete(type); + cycleMemo.set(type, false); + + for (const dependent of dependents) { + if (!candidates.has(dependent)) { + continue; + } + + if (dependent.isOneOf) { + typesToRemove.push(dependent); + } else { + const state = normalTypeStates.get(dependent); + invariant(state !== undefined); - if (!result) { - knownNoCycle.add(inputObj); + --state.unresolvedFieldCount; + if (!state.hasKnownCycleField && state.unresolvedFieldCount === 0) { + typesToRemove.push(dependent); + } + } + } + } + + for (const candidate of candidates.keys()) { + cycleMemo.set(candidate, true); } + + const result = cycleMemo.get(inputObj); + invariant(result !== undefined); return result; } - function inputFieldTypeHasUnbreakableCycle( + function collectReachableInputObjects( + inputObj: GraphQLInputObjectType, + ): Set { + const reachable = new Set(); + const visited = new Set(); + + collect(inputObj); + return reachable; + + function collect(type: GraphQLInputObjectType): void { + if (visited.has(type) || cycleMemo.has(type)) { + return; + } + + visited.add(type); + reachable.add(type); + + for (const field of Object.values(type.getFields())) { + const target = getUnbreakableCycleTarget(type, field.type); + + if (target != null) { + collect(target); + } + } + } + } + + function getUnbreakableCycleTarget( + inputObj: GraphQLInputObjectType, fieldType: GraphQLInputType, - ): boolean { - if (isListType(fieldType)) { - return false; + ): Maybe { + if (inputObj.isOneOf) { + if (isInputObjectType(fieldType)) { + return fieldType; + } + return undefined; } - if (!isInputObjectType(fieldType)) { - return false; + + if (isNonNullType(fieldType) && isInputObjectType(fieldType.ofType)) { + return fieldType.ofType; } - return inputObjectHasUnbreakableCycle(fieldType); } } From 123d07fe15169b89e48a86790f9dde7224b8c54b Mon Sep 17 00:00:00 2001 From: James Bellenger Date: Thu, 7 May 2026 16:27:18 -0700 Subject: [PATCH 05/17] error reporting updates Update error reporting for uninhabited cycles to match the previous implementation, which would report 1 error per distinct cycle. Also, update the errors to use the spec language "cannot be provided a finite value" --- src/type/__tests__/validation-test.ts | 130 +++++++++++--------------- src/type/validate.ts | 122 ++++++++++++------------ 2 files changed, 115 insertions(+), 137 deletions(-) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 6cfd6eb22f..90c370bfba 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -944,7 +944,7 @@ describe('Type System: Input Objects must have fields', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Input Object SomeInputObject references itself via the required fields: SomeInputObject.nonNullSelf.', + 'Input Object SomeInputObject cannot be provided a finite value because it references itself through fields: SomeInputObject.nonNullSelf.', locations: [{ line: 7, column: 9 }], }, ]); @@ -972,31 +972,13 @@ describe('Type System: Input Objects must have fields', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Input Object SomeInputObject references itself via the required fields: SomeInputObject.startLoop, AnotherInputObject.nextInLoop, YetAnotherInputObject.closeLoop.', + 'Input Object SomeInputObject cannot be provided a finite value because it references itself through fields: SomeInputObject.startLoop, AnotherInputObject.nextInLoop, YetAnotherInputObject.closeLoop.', locations: [ { line: 7, column: 9 }, { line: 11, column: 9 }, { line: 15, column: 9 }, ], }, - { - message: - 'Input Object AnotherInputObject references itself via the required fields: AnotherInputObject.nextInLoop, YetAnotherInputObject.closeLoop, SomeInputObject.startLoop.', - locations: [ - { line: 11, column: 9 }, - { line: 15, column: 9 }, - { line: 7, column: 9 }, - ], - }, - { - message: - 'Input Object YetAnotherInputObject references itself via the required fields: YetAnotherInputObject.closeLoop, SomeInputObject.startLoop, AnotherInputObject.nextInLoop.', - locations: [ - { line: 15, column: 9 }, - { line: 7, column: 9 }, - { line: 11, column: 9 }, - ], - }, ]); }); @@ -1024,7 +1006,7 @@ describe('Type System: Input Objects must have fields', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Input Object SomeInputObject references itself via the required fields: SomeInputObject.startLoop, AnotherInputObject.closeLoop.', + 'Input Object SomeInputObject cannot be provided a finite value because it references itself through fields: SomeInputObject.startLoop, AnotherInputObject.closeLoop.', locations: [ { line: 7, column: 9 }, { line: 11, column: 9 }, @@ -1032,20 +1014,16 @@ describe('Type System: Input Objects must have fields', () => { }, { message: - 'Input Object AnotherInputObject references itself via the required fields: AnotherInputObject.closeLoop, SomeInputObject.startLoop.', + 'Input Object AnotherInputObject cannot be provided a finite value because it references itself through fields: AnotherInputObject.startSecondLoop, YetAnotherInputObject.closeSecondLoop.', locations: [ - { line: 11, column: 9 }, - { line: 7, column: 9 }, + { line: 12, column: 9 }, + { line: 16, column: 9 }, ], }, { message: - 'Input Object YetAnotherInputObject references itself via the required fields: YetAnotherInputObject.closeSecondLoop, AnotherInputObject.closeLoop, SomeInputObject.startLoop.', - locations: [ - { line: 16, column: 9 }, - { line: 11, column: 9 }, - { line: 7, column: 9 }, - ], + 'Input Object YetAnotherInputObject cannot be provided a finite value because it references itself through fields: YetAnotherInputObject.nonNullSelf.', + locations: [{ line: 17, column: 9 }], }, ]); }); @@ -2601,7 +2579,7 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Input Object A references itself via the required fields: A.self.', + 'Input Object A cannot be provided a finite value because it references itself through fields: A.self.', locations: [{ line: 7, column: 9 }], }, ]); @@ -2625,17 +2603,9 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Input Object T references itself via the required fields: T.self.', + 'Input Object T cannot be provided a finite value because it references itself through fields: T.self.', locations: [{ line: 8, column: 9 }], }, - { - message: - 'Input Object A references itself via the required fields: A.t, T.self.', - locations: [ - { line: 12, column: 9 }, - { line: 8, column: 9 }, - ], - }, ]); }); @@ -2675,7 +2645,7 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { types, }); - expect(validateSchema(schema)).to.have.lengthOf(types.length); + expect(validateSchema(schema)).to.have.lengthOf(1); expect( getFieldsSpies.reduce((sum, spy) => sum + spy.callCount, 0), ).to.be.lessThan(500); @@ -2698,18 +2668,49 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Input Object A references itself via the required fields: A.b, B.a.', + 'Input Object A cannot be provided a finite value because it references itself through fields: A.b, B.a.', locations: [ { line: 7, column: 9 }, { line: 11, column: 9 }, ], }, + ]); + }); + + it('rejects multiple OneOf branches without duplicate cycle reports', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A @oneOf { + b: B + c: C + } + + input B { + a: A! + } + + input C { + a: A! + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Input Object B references itself via the required fields: B.a, A.b.', + 'Input Object A cannot be provided a finite value because it references itself through fields: A.b, B.a.', locations: [ - { line: 11, column: 9 }, { line: 7, column: 9 }, + { line: 12, column: 9 }, + ], + }, + { + message: + 'Input Object A cannot be provided a finite value because it references itself through fields: A.c, C.a.', + locations: [ + { line: 8, column: 9 }, + { line: 16, column: 9 }, ], }, ]); @@ -2767,7 +2768,7 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { expectJSON(validateSchema(schema)).toDeepEqual([]); }); - it('rejects a non-OneOf/non-OneOf cycle with required scalar and list fields', () => { + it('rejects a non-OneOf/non-OneOf cycle with required scalar, list, and finite input fields', () => { const schema = buildSchema(` type Query { test(arg: A): Int @@ -2775,6 +2776,7 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { input A { list: [B]! + finite: Finite! b: B! } @@ -2782,22 +2784,18 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { value: Int! a: A! } + + input Finite { + value: Int! + } `); expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Input Object A references itself via the required fields: A.b, B.a.', - locations: [ - { line: 8, column: 9 }, - { line: 13, column: 9 }, - ], - }, - { - message: - 'Input Object B references itself via the required fields: B.a, A.b.', + 'Input Object A cannot be provided a finite value because it references itself through fields: A.b, B.a.', locations: [ - { line: 13, column: 9 }, - { line: 8, column: 9 }, + { line: 9, column: 9 }, + { line: 14, column: 9 }, ], }, ]); @@ -2824,31 +2822,13 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Input Object A references itself via the required fields: A.b, B.c, C.a.', + 'Input Object A cannot be provided a finite value because it references itself through fields: A.b, B.c, C.a.', locations: [ { line: 7, column: 9 }, { line: 11, column: 9 }, { line: 15, column: 9 }, ], }, - { - message: - 'Input Object B references itself via the required fields: B.c, C.a, A.b.', - locations: [ - { line: 11, column: 9 }, - { line: 15, column: 9 }, - { line: 7, column: 9 }, - ], - }, - { - message: - 'Input Object C references itself via the required fields: C.a, A.b, B.c.', - locations: [ - { line: 15, column: 9 }, - { line: 7, column: 9 }, - { line: 11, column: 9 }, - ], - }, ]); }); }); diff --git a/src/type/validate.ts b/src/type/validate.ts index 5f89d03176..07fd21fdd5 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -461,15 +461,7 @@ function validateTypes(context: SchemaValidationContext): void { } } - // Report errors for Input Object types that have unbreakable cycles. - for (const type of typesWithUnbreakableCycles) { - const cyclePath = traceUnbreakableCycle(type, typesWithUnbreakableCycles); - const pathStr = cyclePath.map((p) => p.fieldStr).join(', '); - context.reportError( - `Input Object ${type} references itself via the required fields: ${pathStr}.`, - cyclePath.map((p) => p.astNode), - ); - } + reportInputObjectUnbreakableCycles(context, typesWithUnbreakableCycles); } function validateFields( @@ -950,72 +942,78 @@ function createInputObjectUnbreakableCycleCheck(): ( } } } +} - function getUnbreakableCycleTarget( - inputObj: GraphQLInputObjectType, - fieldType: GraphQLInputType, - ): Maybe { - if (inputObj.isOneOf) { - if (isInputObjectType(fieldType)) { - return fieldType; - } - return undefined; +function getUnbreakableCycleTarget( + inputObj: GraphQLInputObjectType, + fieldType: GraphQLInputType, +): Maybe { + if (inputObj.isOneOf) { + if (isInputObjectType(fieldType)) { + return fieldType; } + return undefined; + } - if (isNonNullType(fieldType) && isInputObjectType(fieldType.ofType)) { - return fieldType.ofType; - } + if (isNonNullType(fieldType) && isInputObjectType(fieldType.ofType)) { + return fieldType.ofType; } } -// For an Input Object type with an unbreakable cycle, traces a witness cycle -// path by following required edges to other types with unbreakable cycles. -function traceUnbreakableCycle( - startType: GraphQLInputObjectType, +function reportInputObjectUnbreakableCycles( + context: SchemaValidationContext, typesWithUnbreakableCycles: ReadonlySet, -): Array<{ fieldStr: string; astNode: Maybe }> { - const path: Array<{ fieldStr: string; astNode: Maybe }> = []; - const seen = new Set(); - - let current: Maybe = startType; - while (current != null && !seen.has(current)) { - seen.add(current); - let next: Maybe; - - for (const field of Object.values(current.getFields())) { - let target: Maybe; - - if (current.isOneOf) { - if ( - isInputObjectType(field.type) && - typesWithUnbreakableCycles.has(field.type) - ) { - target = field.type; - } - } else if (isNonNullType(field.type)) { - const nullableType = field.type.ofType; - if ( - isInputObjectType(nullableType) && - typesWithUnbreakableCycles.has(nullableType) - ) { - target = nullableType; - } +): void { + // Tracks already visited types to ensure that cycles are not redundantly + // reported. + const visitedTypes = new Set(); + + // Array of fields used to produce meaningful errors. + const fieldPath: Array<{ fieldStr: string; astNode: Maybe }> = []; + + // Position in the field path. + const fieldPathIndexByType = new Map(); + + for (const type of typesWithUnbreakableCycles) { + reportCycleRecursive(type); + } + + function reportCycleRecursive(inputObj: GraphQLInputObjectType): void { + if (visitedTypes.has(inputObj)) { + return; + } + + visitedTypes.add(inputObj); + fieldPathIndexByType.set(inputObj, fieldPath.length); + + for (const field of Object.values(inputObj.getFields())) { + const target = getUnbreakableCycleTarget(inputObj, field.type); + if (target == null || !typesWithUnbreakableCycles.has(target)) { + continue; } - if (target != null) { - path.push({ - fieldStr: `${current}.${field.name}`, - astNode: field.astNode, - }); - next = target; - break; + const cycleIndex = fieldPathIndexByType.get(target); + fieldPath.push({ + fieldStr: `${inputObj}.${field.name}`, + astNode: field.astNode, + }); + + if (cycleIndex === undefined) { + reportCycleRecursive(target); + } else { + const cyclePath = fieldPath.slice(cycleIndex); + const pathStr = cyclePath.map((p) => p.fieldStr).join(', '); + context.reportError( + `Input Object ${target} cannot be provided a finite value because it references itself through fields: ${pathStr}.`, + cyclePath.map((p) => p.astNode), + ); } + + fieldPath.pop(); } - current = next; + fieldPathIndexByType.delete(inputObj); } - - return path; } function createInputObjectDefaultValueCircularRefsValidator( From 9ed77fcb2b429b781e8792d3a5a9f9c04273c251 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Wed, 10 Jun 2026 15:34:08 +0300 Subject: [PATCH 06/17] remove unnecessary Maybe --- src/type/validate.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/type/validate.ts b/src/type/validate.ts index 07fd21fdd5..77a9734792 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -825,7 +825,7 @@ function createInputObjectUnbreakableCycleCheck(): ( for (const field of fields) { const target = getUnbreakableCycleTarget(candidate, field.type); - if (target == null) { + if (target === undefined) { typesToRemove.push(candidate); break; } @@ -848,7 +848,7 @@ function createInputObjectUnbreakableCycleCheck(): ( for (const field of fields) { const target = getUnbreakableCycleTarget(candidate, field.type); - if (target == null) { + if (target === undefined) { continue; } @@ -936,7 +936,7 @@ function createInputObjectUnbreakableCycleCheck(): ( for (const field of Object.values(type.getFields())) { const target = getUnbreakableCycleTarget(type, field.type); - if (target != null) { + if (target !== undefined) { collect(target); } } @@ -947,12 +947,12 @@ function createInputObjectUnbreakableCycleCheck(): ( function getUnbreakableCycleTarget( inputObj: GraphQLInputObjectType, fieldType: GraphQLInputType, -): Maybe { +): GraphQLInputObjectType | undefined { if (inputObj.isOneOf) { if (isInputObjectType(fieldType)) { return fieldType; } - return undefined; + return; } if (isNonNullType(fieldType) && isInputObjectType(fieldType.ofType)) { @@ -988,7 +988,7 @@ function reportInputObjectUnbreakableCycles( for (const field of Object.values(inputObj.getFields())) { const target = getUnbreakableCycleTarget(inputObj, field.type); - if (target == null || !typesWithUnbreakableCycles.has(target)) { + if (target === undefined || !typesWithUnbreakableCycles.has(target)) { continue; } From bf6fa838356c2ada4babbba8c5f705a5a2f15a98 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Wed, 10 Jun 2026 15:40:59 +0300 Subject: [PATCH 07/17] what's normal --- src/type/__tests__/validation-test.ts | 2 +- src/type/validate.ts | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 90c370bfba..1afb3b1750 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -2585,7 +2585,7 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { ]); }); - it('rejects a normal Input Object requiring an unbreakable OneOf cycle', () => { + it('rejects a non-OneOf Input Object requiring an unbreakable OneOf cycle', () => { const schema = buildSchema(` type Query { t(arg: T): Int diff --git a/src/type/validate.ts b/src/type/validate.ts index 77a9734792..bbe3b6ba69 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -803,8 +803,8 @@ function createInputObjectUnbreakableCycleCheck(): ( candidates.set(candidate, []); } - // Tracks unresolved fields for normal Input Objects. - const normalTypeStates = new Map< + // Tracks unresolved fields for non-OneOf Input Objects. + const nonOneOfTypeStates = new Map< GraphQLInputObjectType, { hasKnownCycleField: boolean; unresolvedFieldCount: number } >(); @@ -841,7 +841,7 @@ function createInputObjectUnbreakableCycleCheck(): ( } } } else { - // Normal Input Objects have an unbreakable cycle if any non-null field has one. + // Non-OneOf Input Objects have an unbreakable cycle if any non-null field has one. let hasKnownCycleField = false; let unresolvedFieldCount = 0; @@ -865,7 +865,7 @@ function createInputObjectUnbreakableCycleCheck(): ( } } - normalTypeStates.set(candidate, { + nonOneOfTypeStates.set(candidate, { hasKnownCycleField, unresolvedFieldCount, }); @@ -896,7 +896,7 @@ function createInputObjectUnbreakableCycleCheck(): ( if (dependent.isOneOf) { typesToRemove.push(dependent); } else { - const state = normalTypeStates.get(dependent); + const state = nonOneOfTypeStates.get(dependent); invariant(state !== undefined); --state.unresolvedFieldCount; From 89e089cef5a80c6e34d8f022ac631c736f974731 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Wed, 10 Jun 2026 18:21:56 +0300 Subject: [PATCH 08/17] Tighten input object cycle cache baseline --- src/__testUtils__/__tests__/spyOn-test.ts | 41 +++++++++++++++++++++++ src/__testUtils__/spyOn.ts | 16 +++++++-- src/type/__tests__/validation-test.ts | 10 ++++-- 3 files changed, 62 insertions(+), 5 deletions(-) diff --git a/src/__testUtils__/__tests__/spyOn-test.ts b/src/__testUtils__/__tests__/spyOn-test.ts index 2e6208c343..15ef532eb5 100644 --- a/src/__testUtils__/__tests__/spyOn-test.ts +++ b/src/__testUtils__/__tests__/spyOn-test.ts @@ -27,6 +27,23 @@ describe('spyOn', () => { expect(obj.addToBase(5)).to.equal(15); expect(obj.addToBase.callCount).to.equal(1); }); + + it('passes an empty stack to the matcher when stack traces are unavailable', () => { + const originalStackTraceLimit = Error.stackTraceLimit; + (Error as { stackTraceLimit: number | undefined }).stackTraceLimit = + undefined; + + try { + const spy = spyOn(() => 42, { + stackMatcher: (stack) => stack === '', + }); + + expect(spy()).to.equal(42); + expect(spy.callCount).to.equal(1); + } finally { + Error.stackTraceLimit = originalStackTraceLimit; + } + }); }); describe('spyOnMethod', () => { @@ -58,6 +75,30 @@ describe('spyOnMethod', () => { expect(spy.callCount).to.equal(1); }); + it('can count only method invocations matching the call stack', () => { + const calculator = { + add(a: number, b: number) { + return a + b; + }, + }; + + const spy = spyOnMethod(calculator, 'add', { + stackMatcher: (stack) => stack.includes('callTrackedMethod'), + }); + + expect(callTrackedMethod()).to.equal(5); + expect(callUntrackedMethod()).to.equal(9); + expect(spy.callCount).to.equal(1); + + function callTrackedMethod(): number { + return calculator.add(2, 3); + } + + function callUntrackedMethod(): number { + return calculator.add(4, 5); + } + }); + it('throws when target property is not a function', () => { const obj: { maybeMethod?: (value: string) => string } = {}; diff --git a/src/__testUtils__/spyOn.ts b/src/__testUtils__/spyOn.ts index 9ac38e3450..3b7f0975b9 100644 --- a/src/__testUtils__/spyOn.ts +++ b/src/__testUtils__/spyOn.ts @@ -5,13 +5,22 @@ export interface MethodSpy { restore: () => void; } +export interface SpyOptions { + readonly stackMatcher?: (stack: string) => boolean; +} + export type SpyFn = T & MethodSpy; -export function spyOn(fn: T): SpyFn { +export function spyOn(fn: T, options?: SpyOptions): SpyFn { let callCount = 0; const spy = function (this: unknown, ...args: Parameters): ReturnType { - callCount += 1; + if ( + options?.stackMatcher === undefined || + options.stackMatcher(new Error().stack ?? '') + ) { + callCount += 1; + } return fn.apply(this, args) as ReturnType; }; @@ -28,6 +37,7 @@ export function spyOn(fn: T): SpyFn { export function spyOnMethod( target: T, key: keyof T, + options?: SpyOptions, ): MethodSpy { const original = target[key]; const wasOwnProperty = Object.hasOwn(target, key); @@ -38,7 +48,7 @@ export function spyOnMethod( ); } - const spy = spyOn(original as AnyFn); + const spy = spyOn(original as AnyFn, options); target[key] = spy as T[keyof T]; const methodSpy: MethodSpy = { diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 1afb3b1750..23508afecc 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -2630,7 +2630,13 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { }); } - const getFieldsSpies = types.map((type) => spyOnMethod(type, 'getFields')); + const getFieldsSpies = types.map((type) => + spyOnMethod(type, 'getFields', { + stackMatcher: (stack) => + stack.includes('inputObjectHasUnbreakableCycle') || + stack.includes('reportInputObjectUnbreakableCycles'), + }), + ); const schema = new GraphQLSchema({ query: new GraphQLObjectType({ @@ -2648,7 +2654,7 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { expect(validateSchema(schema)).to.have.lengthOf(1); expect( getFieldsSpies.reduce((sum, spy) => sum + spy.callCount, 0), - ).to.be.lessThan(500); + ).to.equal(51); }); it('rejects a mixed OneOf/non-OneOf cycle with no escapes', () => { From 5a57936869020b5e00d902aaf0c76436fa51eb78 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Thu, 11 Jun 2026 16:51:16 +0300 Subject: [PATCH 09/17] Use global input object cycle analysis The previous InputObjectHasUnbreakableCycle check ran once for every Input Object. Even with memoization, a type whose result was still unknown had to collect the Input Objects reachable from that root before it could prove which nodes were breakable. Shared OneOf and non-null Input Object subgraphs could therefore be walked from multiple roots before the memo settled. Instead, initialize one state for every Input Object while validating the schema. detectInputObjectUnbreakableCycles then performs a single shallow pass over those states, recording only the edges that can force an unbreakable cycle: OneOf fields whose type is another Input Object, and non-OneOf fields whose type is a non-null Input Object. Fields with scalar, enum, list, or nullable escapes never become targets. Types with an immediate escape are pushed onto a worklist, and the worklist propagates that breakability through reverse dependents until only states with unbreakable cycles remain. Once that fixed point is reached, detectInputObjectUnbreakableCycles reports cycles directly from the remaining states. Reporting cannot safely happen during the initial edge-discovery pass because a state that first appears cyclic may later be proven breakable by worklist propagation from one of its targets. For example, consider a chain like T16 @oneOf { a: T15, b: T15 }, ... T1 @oneOf { a: T0, b: T0 }, T0 @oneOf { self: T0 }. The old per-root check can rediscover the same T15..T0 tail while checking different roots. The global pass calls getFields once per Input Object, records each Tn -> Tn-1 dependency once, and then propagates results through the dependent lists. That avoids repeated reachable-subgraph collection on schemas with shared input-object tails, which is the validateSchema regression this fixes. --- src/type/__tests__/validation-test.ts | 7 +- src/type/validate.ts | 294 ++++++++++---------------- 2 files changed, 113 insertions(+), 188 deletions(-) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 23508afecc..08249e1fdd 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -2609,7 +2609,7 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { ]); }); - it('caches shared unbreakable OneOf subgraphs', () => { + it('checks each shared unbreakable OneOf subgraph once', () => { const chainLength = 16; const types: Array = []; types[0] = new GraphQLInputObjectType({ @@ -2633,8 +2633,7 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { const getFieldsSpies = types.map((type) => spyOnMethod(type, 'getFields', { stackMatcher: (stack) => - stack.includes('inputObjectHasUnbreakableCycle') || - stack.includes('reportInputObjectUnbreakableCycles'), + stack.includes('detectInputObjectUnbreakableCycles'), }), ); @@ -2654,7 +2653,7 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { expect(validateSchema(schema)).to.have.lengthOf(1); expect( getFieldsSpies.reduce((sum, spy) => sum + spy.callCount, 0), - ).to.equal(51); + ).to.equal(17); }); it('rejects a mixed OneOf/non-OneOf cycle with no escapes', () => { diff --git a/src/type/validate.ts b/src/type/validate.ts index bbe3b6ba69..3020027068 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -405,14 +405,13 @@ function validateName( } function validateTypes(context: SchemaValidationContext): void { - const inputObjectUnbreakableCycleCheck = - createInputObjectUnbreakableCycleCheck(); const validateInputObjectDefaultValueCircularRefs = createInputObjectDefaultValueCircularRefsValidator(context); const typeMap = context.schema.getTypeMap(); - - // Collect Input Object types that have unbreakable cycles. - const typesWithUnbreakableCycles = new Set(); + const cycleStates = new Map< + GraphQLInputObjectType, + InputObjectUnbreakableCycleState + >(); for (const type of Object.values(typeMap)) { // Ensure all provided types are in fact GraphQL type. @@ -451,17 +450,26 @@ function validateTypes(context: SchemaValidationContext): void { // Ensure Input Object fields are valid. validateInputFields(context, type); - // Ensure Input Objects do not have unbreakable cycles. - if (inputObjectUnbreakableCycleCheck(type)) { - typesWithUnbreakableCycles.add(type); - } + initializeInputObjectUnbreakableCycleState(type); // Ensure Input Objects do not contain invalid default value circular references. validateInputObjectDefaultValueCircularRefs(type); } } - reportInputObjectUnbreakableCycles(context, typesWithUnbreakableCycles); + detectInputObjectUnbreakableCycles(context, cycleStates); + + function initializeInputObjectUnbreakableCycleState( + inputObj: GraphQLInputObjectType, + ): void { + cycleStates.set(inputObj, { + inputObj, + targets: [], + dependents: [], + unresolvedTargetCount: 0, + hasUnbreakableCycle: true, + }); + } } function validateFields( @@ -777,193 +785,83 @@ function validateOneOfInputObjectField( } } -// Implements the spec's InputObjectHasUnbreakableCycle algorithm. -// Tracks already checked types to ensure that types are not redundantly checked. -function createInputObjectUnbreakableCycleCheck(): ( - inputObj: GraphQLInputObjectType, -) => boolean { - const cycleMemo = new Map(); - - return inputObjectHasUnbreakableCycle; - - function inputObjectHasUnbreakableCycle( - inputObj: GraphQLInputObjectType, - ): boolean { - const cachedResult = cycleMemo.get(inputObj); - if (cachedResult !== undefined) { - return cachedResult; - } - - // Unknown reachable types mapped to types that depend on them. - const candidates = new AccumulatorMap< - GraphQLInputObjectType, - GraphQLInputObjectType - >(); - for (const candidate of collectReachableInputObjects(inputObj)) { - candidates.set(candidate, []); - } - - // Tracks unresolved fields for non-OneOf Input Objects. - const nonOneOfTypeStates = new Map< - GraphQLInputObjectType, - { hasKnownCycleField: boolean; unresolvedFieldCount: number } - >(); - - // Types proven not to have cycles and ready to remove. - const typesToRemove: Array = []; - - for (const candidate of candidates.keys()) { - const fields = Object.values(candidate.getFields()); - - if (candidate.isOneOf) { - // OneOf Input Objects have an unbreakable cycle if every field leads to an unbreakable cycle. - if (fields.length === 0) { - typesToRemove.push(candidate); - continue; - } - - for (const field of fields) { - const target = getUnbreakableCycleTarget(candidate, field.type); - - if (target === undefined) { - typesToRemove.push(candidate); - break; - } - - const targetResult = cycleMemo.get(target); - if (targetResult === false) { - typesToRemove.push(candidate); - break; - } - - if (targetResult === undefined) { - candidates.add(target, candidate); - } - } - } else { - // Non-OneOf Input Objects have an unbreakable cycle if any non-null field has one. - let hasKnownCycleField = false; - let unresolvedFieldCount = 0; - - for (const field of fields) { - const target = getUnbreakableCycleTarget(candidate, field.type); - - if (target === undefined) { - continue; - } - - const targetResult = cycleMemo.get(target); - if (targetResult === false) { - continue; - } - - if (targetResult === true) { - hasKnownCycleField = true; - } else { - ++unresolvedFieldCount; - candidates.add(target, candidate); - } - } +interface InputObjectUnbreakableCycleTarget { + field: GraphQLInputField; + target: GraphQLInputObjectType; +} - nonOneOfTypeStates.set(candidate, { - hasKnownCycleField, - unresolvedFieldCount, - }); +interface InputObjectUnbreakableCycleState { + inputObj: GraphQLInputObjectType; + targets: Array; + dependents: Array; + unresolvedTargetCount: number; + hasUnbreakableCycle: boolean; +} - if (!hasKnownCycleField && unresolvedFieldCount === 0) { - typesToRemove.push(candidate); - } - } - } +// Implements the spec's InputObjectHasUnbreakableCycle algorithm for all Input +// Objects in one pass by propagating known breakable types through reverse edges. +function detectInputObjectUnbreakableCycles( + context: SchemaValidationContext, + cycleStates: ReadonlyMap< + GraphQLInputObjectType, + InputObjectUnbreakableCycleState + >, +): void { + const typesWithoutUnbreakableCycles: Array = + []; - while (typesToRemove.length > 0) { - const type = typesToRemove.pop(); - invariant(type != null); + for (const state of cycleStates.values()) { + const inputObj = state.inputObj; + const fields = Object.values(inputObj.getFields()); - const dependents = candidates.get(type); - if (dependents === undefined) { + for (const field of fields) { + const target = getUnbreakableCycleTarget(inputObj, field.type); + if (target === undefined) { continue; } - candidates.delete(type); - cycleMemo.set(type, false); - - for (const dependent of dependents) { - if (!candidates.has(dependent)) { - continue; - } - - if (dependent.isOneOf) { - typesToRemove.push(dependent); - } else { - const state = nonOneOfTypeStates.get(dependent); - invariant(state !== undefined); - - --state.unresolvedFieldCount; - if (!state.hasKnownCycleField && state.unresolvedFieldCount === 0) { - typesToRemove.push(dependent); - } - } + state.targets.push({ field, target }); + const targetState = cycleStates.get(target); + if (targetState !== undefined) { + targetState.dependents.push(state); } } - for (const candidate of candidates.keys()) { - cycleMemo.set(candidate, true); + if (inputObj.isOneOf) { + // OneOf Input Objects have an unbreakable cycle if every field leads to an unbreakable cycle. + if (fields.length === 0 || state.targets.length < fields.length) { + markInputObjectHasNoUnbreakableCycle(state); + } + } else { + // Non-OneOf Input Objects have an unbreakable cycle if any non-null field has one. + state.unresolvedTargetCount = state.targets.length; + if (state.targets.length === 0) { + markInputObjectHasNoUnbreakableCycle(state); + } } - - const result = cycleMemo.get(inputObj); - invariant(result !== undefined); - return result; } - function collectReachableInputObjects( - inputObj: GraphQLInputObjectType, - ): Set { - const reachable = new Set(); - const visited = new Set(); - - collect(inputObj); - return reachable; - - function collect(type: GraphQLInputObjectType): void { - if (visited.has(type) || cycleMemo.has(type)) { - return; + let nextBreakableState: InputObjectUnbreakableCycleState | undefined; + while ( + (nextBreakableState = typesWithoutUnbreakableCycles.pop()) !== undefined + ) { + for (const dependentState of nextBreakableState.dependents) { + if (!dependentState.hasUnbreakableCycle) { + continue; } - visited.add(type); - reachable.add(type); - - for (const field of Object.values(type.getFields())) { - const target = getUnbreakableCycleTarget(type, field.type); - - if (target !== undefined) { - collect(target); - } + if (dependentState.inputObj.isOneOf) { + markInputObjectHasNoUnbreakableCycle(dependentState); + continue; } - } - } -} -function getUnbreakableCycleTarget( - inputObj: GraphQLInputObjectType, - fieldType: GraphQLInputType, -): GraphQLInputObjectType | undefined { - if (inputObj.isOneOf) { - if (isInputObjectType(fieldType)) { - return fieldType; + --dependentState.unresolvedTargetCount; + if (dependentState.unresolvedTargetCount === 0) { + markInputObjectHasNoUnbreakableCycle(dependentState); + } } - return; - } - - if (isNonNullType(fieldType) && isInputObjectType(fieldType.ofType)) { - return fieldType.ofType; } -} -function reportInputObjectUnbreakableCycles( - context: SchemaValidationContext, - typesWithUnbreakableCycles: ReadonlySet, -): void { // Tracks already visited types to ensure that cycles are not redundantly // reported. const visitedTypes = new Set(); @@ -974,11 +872,23 @@ function reportInputObjectUnbreakableCycles( // Position in the field path. const fieldPathIndexByType = new Map(); - for (const type of typesWithUnbreakableCycles) { - reportCycleRecursive(type); + for (const state of cycleStates.values()) { + if (state.hasUnbreakableCycle) { + reportCycleRecursive(state); + } + } + + function markInputObjectHasNoUnbreakableCycle( + breakableState: InputObjectUnbreakableCycleState, + ): void { + if (breakableState.hasUnbreakableCycle) { + breakableState.hasUnbreakableCycle = false; + typesWithoutUnbreakableCycles.push(breakableState); + } } - function reportCycleRecursive(inputObj: GraphQLInputObjectType): void { + function reportCycleRecursive(state: InputObjectUnbreakableCycleState): void { + const inputObj = state.inputObj; if (visitedTypes.has(inputObj)) { return; } @@ -986,9 +896,9 @@ function reportInputObjectUnbreakableCycles( visitedTypes.add(inputObj); fieldPathIndexByType.set(inputObj, fieldPath.length); - for (const field of Object.values(inputObj.getFields())) { - const target = getUnbreakableCycleTarget(inputObj, field.type); - if (target === undefined || !typesWithUnbreakableCycles.has(target)) { + for (const { field, target } of state.targets) { + const targetState = cycleStates.get(target); + if (targetState?.hasUnbreakableCycle !== true) { continue; } @@ -999,7 +909,7 @@ function reportInputObjectUnbreakableCycles( }); if (cycleIndex === undefined) { - reportCycleRecursive(target); + reportCycleRecursive(targetState); } else { const cyclePath = fieldPath.slice(cycleIndex); const pathStr = cyclePath.map((p) => p.fieldStr).join(', '); @@ -1016,6 +926,22 @@ function reportInputObjectUnbreakableCycles( } } +function getUnbreakableCycleTarget( + inputObj: GraphQLInputObjectType, + fieldType: GraphQLInputType, +): GraphQLInputObjectType | undefined { + if (inputObj.isOneOf) { + if (isInputObjectType(fieldType)) { + return fieldType; + } + return; + } + + if (isNonNullType(fieldType) && isInputObjectType(fieldType.ofType)) { + return fieldType.ofType; + } +} + function createInputObjectDefaultValueCircularRefsValidator( context: SchemaValidationContext, ): (inputObj: GraphQLInputObjectType) => void { From dc9ff4475b17a0e4a56bdf73cccb1f6492cdc5d1 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Thu, 11 Jun 2026 17:11:08 +0300 Subject: [PATCH 10/17] Use finite value names for cycle diagnostics --- src/type/__tests__/validation-test.ts | 2 +- src/type/validate.ts | 77 +++++++++++++-------------- 2 files changed, 39 insertions(+), 40 deletions(-) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 08249e1fdd..e862855d50 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -2633,7 +2633,7 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { const getFieldsSpies = types.map((type) => spyOnMethod(type, 'getFields', { stackMatcher: (stack) => - stack.includes('detectInputObjectUnbreakableCycles'), + stack.includes('detectInputObjectNonFiniteValues'), }), ); diff --git a/src/type/validate.ts b/src/type/validate.ts index 3020027068..e09122869f 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -408,9 +408,9 @@ function validateTypes(context: SchemaValidationContext): void { const validateInputObjectDefaultValueCircularRefs = createInputObjectDefaultValueCircularRefsValidator(context); const typeMap = context.schema.getTypeMap(); - const cycleStates = new Map< + const finiteValueStates = new Map< GraphQLInputObjectType, - InputObjectUnbreakableCycleState + InputObjectFiniteValueState >(); for (const type of Object.values(typeMap)) { @@ -450,24 +450,24 @@ function validateTypes(context: SchemaValidationContext): void { // Ensure Input Object fields are valid. validateInputFields(context, type); - initializeInputObjectUnbreakableCycleState(type); + initializeInputObjectFiniteValueState(type); // Ensure Input Objects do not contain invalid default value circular references. validateInputObjectDefaultValueCircularRefs(type); } } - detectInputObjectUnbreakableCycles(context, cycleStates); + detectInputObjectNonFiniteValues(context, finiteValueStates); - function initializeInputObjectUnbreakableCycleState( + function initializeInputObjectFiniteValueState( inputObj: GraphQLInputObjectType, ): void { - cycleStates.set(inputObj, { + finiteValueStates.set(inputObj, { inputObj, targets: [], dependents: [], unresolvedTargetCount: 0, - hasUnbreakableCycle: true, + hasFiniteValue: false, }); } } @@ -785,43 +785,42 @@ function validateOneOfInputObjectField( } } -interface InputObjectUnbreakableCycleTarget { +interface InputObjectFiniteValueTarget { field: GraphQLInputField; target: GraphQLInputObjectType; } -interface InputObjectUnbreakableCycleState { +interface InputObjectFiniteValueState { inputObj: GraphQLInputObjectType; - targets: Array; - dependents: Array; + targets: Array; + dependents: Array; unresolvedTargetCount: number; - hasUnbreakableCycle: boolean; + hasFiniteValue: boolean; } // Implements the spec's InputObjectHasUnbreakableCycle algorithm for all Input // Objects in one pass by propagating known breakable types through reverse edges. -function detectInputObjectUnbreakableCycles( +function detectInputObjectNonFiniteValues( context: SchemaValidationContext, - cycleStates: ReadonlyMap< + finiteValueStates: ReadonlyMap< GraphQLInputObjectType, - InputObjectUnbreakableCycleState + InputObjectFiniteValueState >, ): void { - const typesWithoutUnbreakableCycles: Array = - []; + const inputObjectsWithFiniteValues: Array = []; - for (const state of cycleStates.values()) { + for (const state of finiteValueStates.values()) { const inputObj = state.inputObj; const fields = Object.values(inputObj.getFields()); for (const field of fields) { - const target = getUnbreakableCycleTarget(inputObj, field.type); + const target = getFiniteValueTarget(inputObj, field.type); if (target === undefined) { continue; } state.targets.push({ field, target }); - const targetState = cycleStates.get(target); + const targetState = finiteValueStates.get(target); if (targetState !== undefined) { targetState.dependents.push(state); } @@ -830,34 +829,34 @@ function detectInputObjectUnbreakableCycles( if (inputObj.isOneOf) { // OneOf Input Objects have an unbreakable cycle if every field leads to an unbreakable cycle. if (fields.length === 0 || state.targets.length < fields.length) { - markInputObjectHasNoUnbreakableCycle(state); + markInputObjectHasFiniteValue(state); } } else { // Non-OneOf Input Objects have an unbreakable cycle if any non-null field has one. state.unresolvedTargetCount = state.targets.length; if (state.targets.length === 0) { - markInputObjectHasNoUnbreakableCycle(state); + markInputObjectHasFiniteValue(state); } } } - let nextBreakableState: InputObjectUnbreakableCycleState | undefined; + let nextFiniteValueState: InputObjectFiniteValueState | undefined; while ( - (nextBreakableState = typesWithoutUnbreakableCycles.pop()) !== undefined + (nextFiniteValueState = inputObjectsWithFiniteValues.pop()) !== undefined ) { - for (const dependentState of nextBreakableState.dependents) { - if (!dependentState.hasUnbreakableCycle) { + for (const dependentState of nextFiniteValueState.dependents) { + if (dependentState.hasFiniteValue) { continue; } if (dependentState.inputObj.isOneOf) { - markInputObjectHasNoUnbreakableCycle(dependentState); + markInputObjectHasFiniteValue(dependentState); continue; } --dependentState.unresolvedTargetCount; if (dependentState.unresolvedTargetCount === 0) { - markInputObjectHasNoUnbreakableCycle(dependentState); + markInputObjectHasFiniteValue(dependentState); } } } @@ -872,22 +871,22 @@ function detectInputObjectUnbreakableCycles( // Position in the field path. const fieldPathIndexByType = new Map(); - for (const state of cycleStates.values()) { - if (state.hasUnbreakableCycle) { + for (const state of finiteValueStates.values()) { + if (!state.hasFiniteValue) { reportCycleRecursive(state); } } - function markInputObjectHasNoUnbreakableCycle( - breakableState: InputObjectUnbreakableCycleState, + function markInputObjectHasFiniteValue( + finiteValueState: InputObjectFiniteValueState, ): void { - if (breakableState.hasUnbreakableCycle) { - breakableState.hasUnbreakableCycle = false; - typesWithoutUnbreakableCycles.push(breakableState); + if (!finiteValueState.hasFiniteValue) { + finiteValueState.hasFiniteValue = true; + inputObjectsWithFiniteValues.push(finiteValueState); } } - function reportCycleRecursive(state: InputObjectUnbreakableCycleState): void { + function reportCycleRecursive(state: InputObjectFiniteValueState): void { const inputObj = state.inputObj; if (visitedTypes.has(inputObj)) { return; @@ -897,8 +896,8 @@ function detectInputObjectUnbreakableCycles( fieldPathIndexByType.set(inputObj, fieldPath.length); for (const { field, target } of state.targets) { - const targetState = cycleStates.get(target); - if (targetState?.hasUnbreakableCycle !== true) { + const targetState = finiteValueStates.get(target); + if (targetState?.hasFiniteValue !== false) { continue; } @@ -926,7 +925,7 @@ function detectInputObjectUnbreakableCycles( } } -function getUnbreakableCycleTarget( +function getFiniteValueTarget( inputObj: GraphQLInputObjectType, fieldType: GraphQLInputType, ): GraphQLInputObjectType | undefined { From c46723e6cd4d6ed07c4e78a8be6163e6e1ca2251 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Thu, 11 Jun 2026 18:03:40 +0300 Subject: [PATCH 11/17] Trim input object cycle test schemas --- src/type/__tests__/validation-test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index e862855d50..7cf55fa23a 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -2476,7 +2476,6 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { it('accepts a OneOf Input Object referencing an already checked input object', () => { const schema = buildSchema(` type Query { - b(arg: B): Int a(arg: A): Int } @@ -2588,7 +2587,6 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { it('rejects a non-OneOf Input Object requiring an unbreakable OneOf cycle', () => { const schema = buildSchema(` type Query { - t(arg: T): Int a(arg: A): Int } @@ -2604,7 +2602,7 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { { message: 'Input Object T cannot be provided a finite value because it references itself through fields: T.self.', - locations: [{ line: 8, column: 9 }], + locations: [{ line: 7, column: 9 }], }, ]); }); From 72eab8f44aaf615526a3ecd46ad89693296f666f Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Thu, 11 Jun 2026 18:07:30 +0300 Subject: [PATCH 12/17] Remove redundant OneOf missing fields test --- src/type/__tests__/validation-test.ts | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 7cf55fa23a..9cb54879ff 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -890,24 +890,6 @@ describe('Type System: Input Objects must have fields', () => { ]); }); - it('rejects a OneOf Input Object type with missing fields', () => { - const schema = buildSchema(` - type Query { - field(arg: SomeInputObject): String - } - - input SomeInputObject @oneOf - `); - - expectJSON(validateSchema(schema)).toDeepEqual([ - { - message: - 'Input Object type SomeInputObject must define one or more fields.', - locations: [{ line: 6, column: 7 }], - }, - ]); - }); - it('accepts an Input Object with breakable circular reference', () => { const schema = buildSchema(` type Query { From ba3566bf4e271ea970ae994870f37d0e192c779b Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Thu, 11 Jun 2026 18:08:17 +0300 Subject: [PATCH 13/17] Group finite input object tests with OneOf validation --- src/type/__tests__/validation-test.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 9cb54879ff..e6fc9bf4e3 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -2409,9 +2409,6 @@ describe('Type System: OneOf Input Object fields must be nullable', () => { }, ]); }); -}); - -describe('Type System: Input Objects must not have unbreakable cycles', () => { it('accepts a OneOf Input Object with a scalar field', () => { const schema = buildSchema(` type Query { From a96bbb9e7034701011e1e950c71998dbcc02b2d4 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Thu, 11 Jun 2026 18:11:06 +0300 Subject: [PATCH 14/17] Order OneOf validation tests by outcome --- src/type/__tests__/validation-test.ts | 181 +++++++++++++------------- 1 file changed, 91 insertions(+), 90 deletions(-) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index e6fc9bf4e3..73a31fab5a 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -2371,44 +2371,6 @@ describe('Type System: Input Object field default values must be valid', () => { }); describe('Type System: OneOf Input Object fields must be nullable', () => { - it('rejects non-nullable fields', () => { - const schema = buildSchema(` - type Query { - test(arg: SomeInputObject): String - } - - input SomeInputObject @oneOf { - a: String - b: String! - } - `); - expectJSON(validateSchema(schema)).toDeepEqual([ - { - message: 'OneOf input field SomeInputObject.b must be nullable.', - locations: [{ line: 8, column: 12 }], - }, - ]); - }); - - it('rejects fields with default values', () => { - const schema = buildSchema(` - type Query { - test(arg: SomeInputObject): String - } - - input SomeInputObject @oneOf { - a: String - b: String = "foo" - } - `); - expectJSON(validateSchema(schema)).toDeepEqual([ - { - message: - 'OneOf input field SomeInputObject.b cannot have a default value.', - locations: [{ line: 8, column: 9 }], - }, - ]); - }); it('accepts a OneOf Input Object with a scalar field', () => { const schema = buildSchema(` type Query { @@ -2544,6 +2506,97 @@ describe('Type System: OneOf Input Object fields must be nullable', () => { expectJSON(validateSchema(schema)).toDeepEqual([]); }); + it('accepts a OneOf/non-OneOf with scalar escape', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A @oneOf { + b: B + escape: Int + } + + input B { + a: A! + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a non-OneOf/non-OneOf cycle with a nullable escape', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A { + b: B! + } + + input B { + a: A + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a non-OneOf/non-OneOf cycle with a list escape', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A { + b: [B!]! + } + + input B { + a: A! + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('rejects non-nullable fields', () => { + const schema = buildSchema(` + type Query { + test(arg: SomeInputObject): String + } + + input SomeInputObject @oneOf { + a: String + b: String! + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: 'OneOf input field SomeInputObject.b must be nullable.', + locations: [{ line: 8, column: 12 }], + }, + ]); + }); + + it('rejects fields with default values', () => { + const schema = buildSchema(` + type Query { + test(arg: SomeInputObject): String + } + + input SomeInputObject @oneOf { + a: String + b: String = "foo" + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'OneOf input field SomeInputObject.b cannot have a default value.', + locations: [{ line: 8, column: 9 }], + }, + ]); + }); + it('rejects a self-referencing OneOf type with no escapes', () => { const schema = buildSchema(` type Query { @@ -2698,58 +2751,6 @@ describe('Type System: OneOf Input Object fields must be nullable', () => { ]); }); - it('accepts a OneOf/non-OneOf with scalar escape', () => { - const schema = buildSchema(` - type Query { - test(arg: A): Int - } - - input A @oneOf { - b: B - escape: Int - } - - input B { - a: A! - } - `); - expectJSON(validateSchema(schema)).toDeepEqual([]); - }); - - it('accepts a non-OneOf/non-OneOf cycle with a nullable escape', () => { - const schema = buildSchema(` - type Query { - test(arg: A): Int - } - - input A { - b: B! - } - - input B { - a: A - } - `); - expectJSON(validateSchema(schema)).toDeepEqual([]); - }); - - it('accepts a non-OneOf/non-OneOf cycle with a list escape', () => { - const schema = buildSchema(` - type Query { - test(arg: A): Int - } - - input A { - b: [B!]! - } - - input B { - a: A! - } - `); - expectJSON(validateSchema(schema)).toDeepEqual([]); - }); - it('rejects a non-OneOf/non-OneOf cycle with required scalar, list, and finite input fields', () => { const schema = buildSchema(` type Query { From c1707a7835c16a5f2880ea17bd462515e9f0fef1 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Thu, 11 Jun 2026 18:12:10 +0300 Subject: [PATCH 15/17] Clarify non-null list escape coverage --- src/type/__tests__/validation-test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 73a31fab5a..554d9ccd59 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -2541,7 +2541,7 @@ describe('Type System: OneOf Input Object fields must be nullable', () => { expectJSON(validateSchema(schema)).toDeepEqual([]); }); - it('accepts a non-OneOf/non-OneOf cycle with a list escape', () => { + it('accepts a non-OneOf/non-OneOf cycle with a non-null list of non-null items escape', () => { const schema = buildSchema(` type Query { test(arg: A): Int From 61d1feda6cdec34771e2353543611425e0e4c314 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Thu, 11 Jun 2026 18:20:23 +0300 Subject: [PATCH 16/17] Remove redundant OneOf scalar escape test --- src/type/__tests__/validation-test.ts | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 554d9ccd59..2aa1a1b842 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -2453,24 +2453,6 @@ describe('Type System: OneOf Input Object fields must be nullable', () => { expectJSON(validateSchema(schema)).toDeepEqual([]); }); - it('accepts a OneOf Input Object with an input object field and scalar escape', () => { - const schema = buildSchema(` - type Query { - test(arg: A): Int - } - - input A @oneOf { - b: B - escape: Int - } - - input B { - value: Int - } - `); - expectJSON(validateSchema(schema)).toDeepEqual([]); - }); - it('accepts a OneOf/OneOf cycle with a scalar escape', () => { const schema = buildSchema(` type Query { From b967e36e4a6a03ddd9ee744ff047004fcc30c1ee Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Thu, 11 Jun 2026 18:20:45 +0300 Subject: [PATCH 17/17] add test for multiple non-OneOf Input Object cycles to demonstrate that OneOf behavior is parallel to non-OneOf --- src/type/__tests__/validation-test.ts | 40 +++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 2aa1a1b842..f8c31d3184 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -1010,6 +1010,46 @@ describe('Type System: Input Objects must have fields', () => { ]); }); + it('rejects an Input Object with multiple non-breakable circular references', () => { + const schema = buildSchema(` + type Query { + field(arg: A): String + } + + input A { + b: B! + c: C! + } + + input B { + a: A! + } + + input C { + a: A! + } + `); + + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Input Object A cannot be provided a finite value because it references itself through fields: A.b, B.a.', + locations: [ + { line: 7, column: 9 }, + { line: 12, column: 9 }, + ], + }, + { + message: + 'Input Object A cannot be provided a finite value because it references itself through fields: A.c, C.a.', + locations: [ + { line: 8, column: 9 }, + { line: 16, column: 9 }, + ], + }, + ]); + }); + it('accepts Input Objects with default values without circular references (SDL)', () => { const validSchema = buildSchema(` type Query {