From 0b3a19758d36fb74ea097005f16ff31e30767f3f Mon Sep 17 00:00:00 2001 From: Emily Goodwin Date: Wed, 14 Jan 2026 22:09:47 -0500 Subject: [PATCH 1/9] intial commit of allowing extension of existing fields --- src/language/__tests__/schema-parser-test.ts | 40 ++ src/utilities/__tests__/extendSchema-test.ts | 373 ++++++++++++++++++ src/utilities/extendSchema.ts | 83 +++- .../UniqueFieldDefinitionNamesRule-test.ts | 268 ------------- .../rules/UniqueFieldDefinitionNamesRule.ts | 2 - 5 files changed, 476 insertions(+), 290 deletions(-) diff --git a/src/language/__tests__/schema-parser-test.ts b/src/language/__tests__/schema-parser-test.ts index 5159939cfd..35c93558fb 100644 --- a/src/language/__tests__/schema-parser-test.ts +++ b/src/language/__tests__/schema-parser-test.ts @@ -212,6 +212,46 @@ describe('Schema Parser', () => { }); }); + it('Object extension with fields', () => { + const doc = parse('extend type Hello { name: String @deprecated }'); + + expectJSON(doc).toDeepEqual({ + kind: 'Document', + definitions: [ + { + kind: 'ObjectTypeExtension', + name: nameNode('Hello', { start: 12, end: 17 }), + interfaces: [], + directives: [], + fields: [ + { + kind: 'FieldDefinition', + name: nameNode('name', { start: 20, end: 24 }), + arguments: [], + description: undefined, + type: { + kind: 'NamedType', + name: nameNode('String', { start: 26, end: 32 }), + loc: { start: 26, end: 32 }, + }, + directives: [ + { + kind: 'Directive', + name: nameNode('deprecated', { start: 34, end: 44 }), + arguments: [], + loc: { start: 33, end: 44 }, + }, + ], + loc: { start: 20, end: 44 }, + }, + ], + loc: { start: 0, end: 46 }, + }, + ], + loc: { start: 0, end: 46 }, + }); + }); + it('Interface extension without fields', () => { const doc = parse('extend interface Hello implements Greeting'); expectJSON(doc).toDeepEqual({ diff --git a/src/utilities/__tests__/extendSchema-test.ts b/src/utilities/__tests__/extendSchema-test.ts index 86baf0e699..c807bbcbca 100644 --- a/src/utilities/__tests__/extendSchema-test.ts +++ b/src/utilities/__tests__/extendSchema-test.ts @@ -1319,4 +1319,377 @@ describe('extendSchema', () => { `); }); }); + + describe('field merging', () => { + it('merges field types when extending with same field name', () => { + const schema = buildSchema(` + type Query { + test: Test + } + + type Test { + id: ID + name: String + } + `); + + const extendAST = parse(` + extend type Test { + id: String @deprecated(reason: "Use stringId instead") + email: String + } + `); + + const extendedSchema = extendSchema(schema, extendAST); + expect(validateSchema(extendedSchema)).to.deep.equal([]); + + const testType = assertObjectType(extendedSchema.getType('Test')); + const fields = testType.getFields(); + + expect(fields.id.type.toString()).to.equal('String'); + expect(fields.id.deprecationReason).to.equal('Use stringId instead'); + + expect(fields.name.type.toString()).to.equal('String'); + expect(fields.name.deprecationReason).to.equal(undefined); + + expect(fields.email.type.toString()).to.equal('String'); + expect(fields.email.deprecationReason).to.equal(undefined); + + expectSchemaChanges(schema, extendedSchema).to.equal(dedent` + type Test { + id: String @deprecated(reason: "Use stringId instead") + name: String + email: String + } + `); + }); + + it('merges field arguments additively', () => { + const schema = buildSchema(` + type Query { + test: Test + } + + type Test { + search(query: String): [String] + } + `); + + const extendAST = parse(` + extend type Test { + search(limit: Int = 10): [String] + } + `); + + const extendedSchema = extendSchema(schema, extendAST); + expect(validateSchema(extendedSchema)).to.deep.equal([]); + + const testType = assertObjectType(extendedSchema.getType('Test')); + const searchField = testType.getFields().search; + + expect(searchField.args.map((arg) => arg.name)).to.have.members([ + 'query', + 'limit', + ]); + const queryArg = searchField.args.find((arg) => arg.name === 'query'); + expect(queryArg?.type.toString()).to.equal('String'); + const limitArg = searchField.args.find((arg) => arg.name === 'limit'); + expect(limitArg?.type.toString()).to.equal('Int'); + + expectSchemaChanges(schema, extendedSchema).to.equal(dedent` + type Test { + search(query: String, limit: Int = 10): [String] + } + `); + }); + + it('merges multiple extensions on the same field', () => { + const schema = buildSchema(` + type Query { + test: Test + } + + type Test { + profile(format: String): String + } + `); + + const firstExtendAST = parse(` + extend type Test { + profile(detailed: Boolean = false): String + name: String + } + `); + + const secondExtendAST = parse(` + extend type Test { + profile(includePrivate: Boolean = false): String @deprecated(reason: "Use profileV2") + age: Int + } + `); + + let extendedSchema = extendSchema(schema, firstExtendAST); + extendedSchema = extendSchema(extendedSchema, secondExtendAST); + + expect(validateSchema(extendedSchema)).to.deep.equal([]); + + const testType = assertObjectType(extendedSchema.getType('Test')); + const profileField = testType.getFields().profile; + + expect(profileField.args.map((arg) => arg.name)).to.have.members([ + 'format', + 'detailed', + 'includePrivate', + ]); + + expect(profileField.deprecationReason).to.equal('Use profileV2'); + + const fieldNames = Object.keys(testType.getFields()); + expect(fieldNames).to.have.members(['profile', 'name', 'age']); + + expectSchemaChanges(schema, extendedSchema).to.equal(dedent` + type Test { + profile(format: String, detailed: Boolean = false, includePrivate: Boolean = false): String @deprecated(reason: "Use profileV2") + name: String + age: Int + } + `); + }); + + it('merges field descriptions', () => { + const schema = buildSchema(` + type Query { + test: Test + } + + type Test { + field: String + } + `); + + const extendAST = parse(` + extend type Test { + """Updated field description""" + field: String + newField: Int + } + `); + + const extendedSchema = extendSchema(schema, extendAST); + expect(validateSchema(extendedSchema)).to.deep.equal([]); + + const testType = assertObjectType(extendedSchema.getType('Test')); + const field = testType.getFields().field; + + expect(field.description).to.equal('Updated field description'); + + expectSchemaChanges(schema, extendedSchema).to.equal(dedent` + type Test { + """Updated field description""" + field: String + newField: Int + } + `); + }); + + it('preserves original field properties when not overridden', () => { + const schema = buildSchema(` + type Query { + test: Test + } + + type Test { + """Original description""" + field(arg: String = "default"): String @deprecated(reason: "Original reason") + } + `); + + const extendAST = parse(` + extend type Test { + field(newArg: Int): String + } + `); + + const extendedSchema = extendSchema(schema, extendAST); + expect(validateSchema(extendedSchema)).to.deep.equal([]); + + const testType = assertObjectType(extendedSchema.getType('Test')); + const field = testType.getFields().field; + + expect(field.description).to.equal('Original description'); + expect(field.deprecationReason).to.equal('Original reason'); + + expect(field.args.map((arg) => arg.name)).to.have.members([ + 'arg', + 'newArg', + ]); + const argArg = field.args.find((arg) => arg.name === 'arg'); + const newArgArg = field.args.find((arg) => arg.name === 'newArg'); + expect(argArg?.type.toString()).to.equal('String'); + expect(newArgArg?.type.toString()).to.equal('Int'); + + expectSchemaChanges(schema, extendedSchema).to.equal(dedent` + type Test { + """Original description""" + field(arg: String = "default", newArg: Int): String @deprecated(reason: "Original reason") + } + `); + }); + + it('allows overriding field properties with extensions', () => { + const schema = buildSchema(` + type Query { + test: Test + } + + type Test { + """Original description""" + field: String + } + `); + + const extendAST = parse(` + extend type Test { + """New description""" + field: Int @deprecated(reason: "Field changed") + } + `); + + const extendedSchema = extendSchema(schema, extendAST); + expect(validateSchema(extendedSchema)).to.deep.equal([]); + + const testType = assertObjectType(extendedSchema.getType('Test')); + const field = testType.getFields().field; + + expect(field.type.toString()).to.equal('Int'); + expect(field.description).to.equal('New description'); + expect(field.deprecationReason).to.equal('Field changed'); + + expectSchemaChanges(schema, extendedSchema).to.equal(dedent` + type Test { + """New description""" + field: Int @deprecated(reason: "Field changed") + } + `); + }); + + it('works with interface field merging', () => { + const schema = buildSchema(` + type Query { + test: Test + } + + interface Test { + id: ID + name: String + } + `); + + const extendAST = parse(` + extend interface Test { + id: String @deprecated(reason: "Use stringId") + email: String + } + `); + + const extendedSchema = extendSchema(schema, extendAST); + expect(validateSchema(extendedSchema)).to.deep.equal([]); + + const testInterface = assertInterfaceType(extendedSchema.getType('Test')); + const fields = testInterface.getFields(); + + expect(fields.id.type.toString()).to.equal('String'); + expect(fields.id.deprecationReason).to.equal('Use stringId'); + expect(fields.name.type.toString()).to.equal('String'); + expect(fields.email.type.toString()).to.equal('String'); + + expectSchemaChanges(schema, extendedSchema).to.equal(dedent` + interface Test { + id: String @deprecated(reason: "Use stringId") + name: String + email: String + } + `); + }); + + it('handles complex field merging scenarios', () => { + const schema = buildSchema(` + type Query { + user: User + } + + type User { + id: ID! + profile(format: String): UserProfile + } + + type UserProfile { + data: String + } + `); + + const extension1 = parse(` + extend type User { + profile(detailed: Boolean = false): UserProfile @deprecated(reason: "Use profileV2") + name: String + } + `); + + const extension2 = parse(` + extend type User { + profile(includePrivate: Boolean = false): UserProfile + email: String + age: Int + } + `); + + const extension3 = parse(` + extend type User { + """User's age in years""" + age: Int + fullName: String + } + `); + + let extendedSchema = extendSchema(schema, extension1); + extendedSchema = extendSchema(extendedSchema, extension2); + extendedSchema = extendSchema(extendedSchema, extension3); + + expect(validateSchema(extendedSchema)).to.deep.equal([]); + + const userType = assertObjectType(extendedSchema.getType('User')); + const fields = userType.getFields(); + + const profileField = fields.profile; + expect(profileField.args.map((arg) => arg.name)).to.have.members([ + 'format', + 'detailed', + 'includePrivate', + ]); + expect(profileField.deprecationReason).to.equal('Use profileV2'); + + expect(fields.age.description).to.equal("User's age in years"); + + expect(Object.keys(fields)).to.have.members([ + 'id', + 'profile', + 'name', + 'email', + 'age', + 'fullName', + ]); + + expectSchemaChanges(schema, extendedSchema).to.equal(dedent` + type User { + id: ID! + profile(format: String, detailed: Boolean = false, includePrivate: Boolean = false): UserProfile @deprecated(reason: "Use profileV2") + name: String + email: String + """User's age in years""" + age: Int + fullName: String + } + `); + }); + }); }); diff --git a/src/utilities/extendSchema.ts b/src/utilities/extendSchema.ts index d53752d919..72529fa8d9 100644 --- a/src/utilities/extendSchema.ts +++ b/src/utilities/extendSchema.ts @@ -38,7 +38,6 @@ import { import type { GraphQLArgumentConfig, GraphQLEnumValueConfigMap, - GraphQLFieldConfig, GraphQLFieldConfigArgumentMap, GraphQLFieldConfigMap, GraphQLInputFieldConfigMap, @@ -331,10 +330,15 @@ export function extendSchemaImpl( ...type.getInterfaces().map(replaceNamedType), ...buildInterfaces(extensions), ], - fields: () => ({ - ...mapValue(config.fields, extendField), - ...buildFieldMap(extensions), - }), + fields: () => { + const existingFields = mapValue(config.fields, (field) => ({ + ...field, + type: replaceType(field.type), + args: field.args ? mapValue(field.args, extendArg) : {}, + })); + const extensionFields = buildFieldMap(extensions); + return mergeFieldMaps(existingFields, extensionFields); + }, extensionASTNodes: config.extensionASTNodes.concat(extensions), }); } @@ -351,10 +355,15 @@ export function extendSchemaImpl( ...type.getInterfaces().map(replaceNamedType), ...buildInterfaces(extensions), ], - fields: () => ({ - ...mapValue(config.fields, extendField), - ...buildFieldMap(extensions), - }), + fields: () => { + const existingFields = mapValue(config.fields, (field) => ({ + ...field, + type: replaceType(field.type), + args: field.args ? mapValue(field.args, extendArg) : {}, + })); + const extensionFields = buildFieldMap(extensions); + return mergeFieldMaps(existingFields, extensionFields); + }, extensionASTNodes: config.extensionASTNodes.concat(extensions), }); } @@ -373,16 +382,6 @@ export function extendSchemaImpl( }); } - function extendField( - field: GraphQLFieldConfig, - ): GraphQLFieldConfig { - return { - ...field, - type: replaceType(field.type), - args: field.args && mapValue(field.args, extendArg), - }; - } - function extendArg(arg: GraphQLArgumentConfig) { return { ...arg, @@ -461,7 +460,8 @@ export function extendSchemaImpl( const nodeFields = /* c8 ignore next */ node.fields ?? []; for (const field of nodeFields) { - fieldConfigMap[field.name.value] = { + const fieldName = field.name.value; + const newFieldConfig: any = { // Note: While this could make assertions to get the correctly typed // value, that would throw immediately while type system validation // with validateSchema() will produce more actionable results. @@ -471,11 +471,54 @@ export function extendSchemaImpl( deprecationReason: getDeprecationReason(field), astNode: field, }; + + if (fieldConfigMap[fieldName] != null) { + fieldConfigMap[fieldName] = mergeFieldConfigs( + fieldConfigMap[fieldName], + newFieldConfig, + ); + } else { + fieldConfigMap[fieldName] = newFieldConfig; + } } } return fieldConfigMap; } + function mergeFieldConfigs(existing: any, incoming: any): any { + return { + type: incoming.type ?? existing.type, + description: incoming.description ?? existing.description, + args: { + ...existing.args, + ...incoming.args, + }, + deprecationReason: + incoming.deprecationReason ?? existing.deprecationReason, + astNode: incoming.astNode ?? existing.astNode, + }; + } + + function mergeFieldMaps( + existingFields: GraphQLFieldConfigMap, + extensionFields: GraphQLFieldConfigMap, + ): GraphQLFieldConfigMap { + const mergedFields = { ...existingFields }; + + for (const [fieldName, extensionField] of Object.entries(extensionFields)) { + if (mergedFields[fieldName] != null) { + mergedFields[fieldName] = mergeFieldConfigs( + mergedFields[fieldName], + extensionField, + ); + } else { + mergedFields[fieldName] = extensionField; + } + } + + return mergedFields; + } + function buildArgumentMap( args: Maybe>, ): GraphQLFieldConfigArgumentMap { diff --git a/src/validation/__tests__/UniqueFieldDefinitionNamesRule-test.ts b/src/validation/__tests__/UniqueFieldDefinitionNamesRule-test.ts index 441e85d31a..bcb7ab8903 100644 --- a/src/validation/__tests__/UniqueFieldDefinitionNamesRule-test.ts +++ b/src/validation/__tests__/UniqueFieldDefinitionNamesRule-test.ts @@ -142,150 +142,6 @@ describe('Validate: Unique field definition names', () => { `); }); - it('extend type with duplicate field', () => { - expectSDLErrors(` - extend type SomeObject { - foo: String - } - type SomeObject { - foo: String - } - - extend interface SomeInterface { - foo: String - } - interface SomeInterface { - foo: String - } - - extend input SomeInputObject { - foo: String - } - input SomeInputObject { - foo: String - } - `).toDeepEqual([ - { - message: 'Field "SomeObject.foo" can only be defined once.', - locations: [ - { line: 3, column: 9 }, - { line: 6, column: 9 }, - ], - }, - { - message: 'Field "SomeInterface.foo" can only be defined once.', - locations: [ - { line: 10, column: 9 }, - { line: 13, column: 9 }, - ], - }, - { - message: 'Field "SomeInputObject.foo" can only be defined once.', - locations: [ - { line: 17, column: 9 }, - { line: 20, column: 9 }, - ], - }, - ]); - }); - - it('duplicate field inside extension', () => { - expectSDLErrors(` - type SomeObject - extend type SomeObject { - foo: String - bar: String - foo: String - } - - interface SomeInterface - extend interface SomeInterface { - foo: String - bar: String - foo: String - } - - input SomeInputObject - extend input SomeInputObject { - foo: String - bar: String - foo: String - } - `).toDeepEqual([ - { - message: 'Field "SomeObject.foo" can only be defined once.', - locations: [ - { line: 4, column: 9 }, - { line: 6, column: 9 }, - ], - }, - { - message: 'Field "SomeInterface.foo" can only be defined once.', - locations: [ - { line: 11, column: 9 }, - { line: 13, column: 9 }, - ], - }, - { - message: 'Field "SomeInputObject.foo" can only be defined once.', - locations: [ - { line: 18, column: 9 }, - { line: 20, column: 9 }, - ], - }, - ]); - }); - - it('duplicate field inside different extensions', () => { - expectSDLErrors(` - type SomeObject - extend type SomeObject { - foo: String - } - extend type SomeObject { - foo: String - } - - interface SomeInterface - extend interface SomeInterface { - foo: String - } - extend interface SomeInterface { - foo: String - } - - input SomeInputObject - extend input SomeInputObject { - foo: String - } - extend input SomeInputObject { - foo: String - } - `).toDeepEqual([ - { - message: 'Field "SomeObject.foo" can only be defined once.', - locations: [ - { line: 4, column: 9 }, - { line: 7, column: 9 }, - ], - }, - { - message: 'Field "SomeInterface.foo" can only be defined once.', - locations: [ - { line: 12, column: 9 }, - { line: 15, column: 9 }, - ], - }, - { - message: 'Field "SomeInputObject.foo" can only be defined once.', - locations: [ - { line: 20, column: 9 }, - { line: 23, column: 9 }, - ], - }, - ]); - }); - it('adding new field to the type inside existing schema', () => { const schema = buildSchema(` type SomeObject @@ -308,128 +164,4 @@ describe('Validate: Unique field definition names', () => { expectValidSDL(sdl, schema); }); - - it('adding conflicting fields to existing schema twice', () => { - const schema = buildSchema(` - type SomeObject { - foo: String - } - - interface SomeInterface { - foo: String - } - - input SomeInputObject { - foo: String - } - `); - const sdl = ` - extend type SomeObject { - foo: String - } - extend interface SomeInterface { - foo: String - } - extend input SomeInputObject { - foo: String - } - - extend type SomeObject { - foo: String - } - extend interface SomeInterface { - foo: String - } - extend input SomeInputObject { - foo: String - } - `; - - expectSDLErrors(sdl, schema).toDeepEqual([ - { - message: - 'Field "SomeObject.foo" already exists in the schema. It cannot also be defined in this type extension.', - locations: [{ line: 3, column: 9 }], - }, - { - message: - 'Field "SomeInterface.foo" already exists in the schema. It cannot also be defined in this type extension.', - locations: [{ line: 6, column: 9 }], - }, - { - message: - 'Field "SomeInputObject.foo" already exists in the schema. It cannot also be defined in this type extension.', - locations: [{ line: 9, column: 9 }], - }, - { - message: - 'Field "SomeObject.foo" already exists in the schema. It cannot also be defined in this type extension.', - locations: [{ line: 13, column: 9 }], - }, - { - message: - 'Field "SomeInterface.foo" already exists in the schema. It cannot also be defined in this type extension.', - locations: [{ line: 16, column: 9 }], - }, - { - message: - 'Field "SomeInputObject.foo" already exists in the schema. It cannot also be defined in this type extension.', - locations: [{ line: 19, column: 9 }], - }, - ]); - }); - - it('adding fields to existing schema twice', () => { - const schema = buildSchema(` - type SomeObject - interface SomeInterface - input SomeInputObject - `); - const sdl = ` - extend type SomeObject { - foo: String - } - extend type SomeObject { - foo: String - } - - extend interface SomeInterface { - foo: String - } - extend interface SomeInterface { - foo: String - } - - extend input SomeInputObject { - foo: String - } - extend input SomeInputObject { - foo: String - } - `; - - expectSDLErrors(sdl, schema).toDeepEqual([ - { - message: 'Field "SomeObject.foo" can only be defined once.', - locations: [ - { line: 3, column: 9 }, - { line: 6, column: 9 }, - ], - }, - { - message: 'Field "SomeInterface.foo" can only be defined once.', - locations: [ - { line: 10, column: 9 }, - { line: 13, column: 9 }, - ], - }, - { - message: 'Field "SomeInputObject.foo" can only be defined once.', - locations: [ - { line: 17, column: 9 }, - { line: 20, column: 9 }, - ], - }, - ]); - }); }); diff --git a/src/validation/rules/UniqueFieldDefinitionNamesRule.ts b/src/validation/rules/UniqueFieldDefinitionNamesRule.ts index 52f6527d64..2d174ebdf4 100644 --- a/src/validation/rules/UniqueFieldDefinitionNamesRule.ts +++ b/src/validation/rules/UniqueFieldDefinitionNamesRule.ts @@ -32,9 +32,7 @@ export function UniqueFieldDefinitionNamesRule( InputObjectTypeDefinition: checkFieldUniqueness, InputObjectTypeExtension: checkFieldUniqueness, InterfaceTypeDefinition: checkFieldUniqueness, - InterfaceTypeExtension: checkFieldUniqueness, ObjectTypeDefinition: checkFieldUniqueness, - ObjectTypeExtension: checkFieldUniqueness, }; function checkFieldUniqueness(node: { From b46e219179c54362c36a6abb6d76d4c9ceae0339 Mon Sep 17 00:00:00 2001 From: egoodwinx Date: Fri, 23 Jan 2026 13:50:53 -0500 Subject: [PATCH 2/9] update to disallow destructive behaviours --- src/utilities/__tests__/extendSchema-test.ts | 65 +- src/utilities/extendSchema.ts | 8 +- ...tingDescriptionsAndDeprecationRule-test.ts | 578 ++++++++++++++++++ ...xtendedFieldsMatchOriginalTypeRule-test.ts | 469 ++++++++++++++ .../UniqueFieldDefinitionNamesRule-test.ts | 31 + src/validation/index.ts | 1 + ...nflictingDescriptionsAndDeprecationRule.ts | 269 ++++++++ .../ExtendedFieldsMatchOriginalTypeRule.ts | 167 +++++ .../rules/UniqueFieldDefinitionNamesRule.ts | 37 +- src/validation/specifiedRules.ts | 12 +- 10 files changed, 1590 insertions(+), 47 deletions(-) create mode 100644 src/validation/__tests__/ConflictingDescriptionsAndDeprecationRule-test.ts create mode 100644 src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts create mode 100644 src/validation/rules/ConflictingDescriptionsAndDeprecationRule.ts create mode 100644 src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts diff --git a/src/utilities/__tests__/extendSchema-test.ts b/src/utilities/__tests__/extendSchema-test.ts index c807bbcbca..3e4357b8c0 100644 --- a/src/utilities/__tests__/extendSchema-test.ts +++ b/src/utilities/__tests__/extendSchema-test.ts @@ -1321,7 +1321,7 @@ describe('extendSchema', () => { }); describe('field merging', () => { - it('merges field types when extending with same field name', () => { + it('merges field properties when extending with same field name', () => { const schema = buildSchema(` type Query { test: Test @@ -1335,7 +1335,7 @@ describe('extendSchema', () => { const extendAST = parse(` extend type Test { - id: String @deprecated(reason: "Use stringId instead") + id: ID @deprecated(reason: "Use stringId instead") email: String } `); @@ -1346,7 +1346,7 @@ describe('extendSchema', () => { const testType = assertObjectType(extendedSchema.getType('Test')); const fields = testType.getFields(); - expect(fields.id.type.toString()).to.equal('String'); + expect(fields.id.type.toString()).to.equal('ID'); expect(fields.id.deprecationReason).to.equal('Use stringId instead'); expect(fields.name.type.toString()).to.equal('String'); @@ -1357,7 +1357,7 @@ describe('extendSchema', () => { expectSchemaChanges(schema, extendedSchema).to.equal(dedent` type Test { - id: String @deprecated(reason: "Use stringId instead") + id: ID @deprecated(reason: "Use stringId instead") name: String email: String } @@ -1536,22 +1536,20 @@ describe('extendSchema', () => { `); }); - it('allows overriding field properties with extensions', () => { + it('allows adding field properties with extensions', () => { const schema = buildSchema(` type Query { test: Test } type Test { - """Original description""" field: String } `); const extendAST = parse(` extend type Test { - """New description""" - field: Int @deprecated(reason: "Field changed") + field: String @deprecated(reason: "Field changed") } `); @@ -1561,14 +1559,13 @@ describe('extendSchema', () => { const testType = assertObjectType(extendedSchema.getType('Test')); const field = testType.getFields().field; - expect(field.type.toString()).to.equal('Int'); - expect(field.description).to.equal('New description'); + expect(field.type.toString()).to.equal('String'); + expect(field.description).to.equal(undefined); expect(field.deprecationReason).to.equal('Field changed'); expectSchemaChanges(schema, extendedSchema).to.equal(dedent` type Test { - """New description""" - field: Int @deprecated(reason: "Field changed") + field: String @deprecated(reason: "Field changed") } `); }); @@ -1587,7 +1584,7 @@ describe('extendSchema', () => { const extendAST = parse(` extend interface Test { - id: String @deprecated(reason: "Use stringId") + id: ID @deprecated(reason: "Use stringId") email: String } `); @@ -1598,14 +1595,14 @@ describe('extendSchema', () => { const testInterface = assertInterfaceType(extendedSchema.getType('Test')); const fields = testInterface.getFields(); - expect(fields.id.type.toString()).to.equal('String'); + expect(fields.id.type.toString()).to.equal('ID'); expect(fields.id.deprecationReason).to.equal('Use stringId'); expect(fields.name.type.toString()).to.equal('String'); expect(fields.email.type.toString()).to.equal('String'); expectSchemaChanges(schema, extendedSchema).to.equal(dedent` interface Test { - id: String @deprecated(reason: "Use stringId") + id: ID @deprecated(reason: "Use stringId") name: String email: String } @@ -1691,5 +1688,43 @@ describe('extendSchema', () => { } `); }); + + it('covers field merging within buildFieldMap when same field appears in multiple extensions', () => { + // This test specifically targets the if branch in buildFieldMap + // when fieldConfigMap[fieldName] != null (same field in multiple extensions) + const schema = buildSchema(` + type Query { + existingField: String + } + `); + + const extendAST = parse(` + extend type Query { + newField: String + } + + extend type Query { + newField: String @deprecated(reason: "Use something else") + } + `); + + // Use assumeValidSDL to bypass validation and test the internal merging logic + const extendedSchema = extendSchema(schema, extendAST, { + assumeValidSDL: true, + }); + + const queryType = extendedSchema.getType('Query'); + assert(queryType != null); + const fields = (queryType as any).getFields(); + expect(fields.newField.type.toString()).to.equal('String'); + expect(fields.newField.deprecationReason).to.equal('Use something else'); + + expectSchemaChanges(schema, extendedSchema).to.equal(dedent` + type Query { + existingField: String + newField: String @deprecated(reason: "Use something else") + } + `); + }); }); }); diff --git a/src/utilities/extendSchema.ts b/src/utilities/extendSchema.ts index 72529fa8d9..8b45cce66d 100644 --- a/src/utilities/extendSchema.ts +++ b/src/utilities/extendSchema.ts @@ -487,15 +487,15 @@ export function extendSchemaImpl( function mergeFieldConfigs(existing: any, incoming: any): any { return { - type: incoming.type ?? existing.type, - description: incoming.description ?? existing.description, + type: existing.type, + description: existing.description ?? incoming.description, args: { ...existing.args, ...incoming.args, }, deprecationReason: - incoming.deprecationReason ?? existing.deprecationReason, - astNode: incoming.astNode ?? existing.astNode, + existing.deprecationReason ?? incoming.deprecationReason, + astNodes: [existing.astNode, incoming.astNode].filter(Boolean), }; } diff --git a/src/validation/__tests__/ConflictingDescriptionsAndDeprecationRule-test.ts b/src/validation/__tests__/ConflictingDescriptionsAndDeprecationRule-test.ts new file mode 100644 index 0000000000..ea8706ea93 --- /dev/null +++ b/src/validation/__tests__/ConflictingDescriptionsAndDeprecationRule-test.ts @@ -0,0 +1,578 @@ +import { describe, it } from 'mocha'; + +import { buildSchema } from '../../utilities/buildASTSchema.js'; + +import { ConflictingDescriptionsAndDeprecationRule } from '../rules/ConflictingDescriptionsAndDeprecationRule.js'; + +import { expectSDLValidationErrors } from './harness.js'; + +function expectErrors(sdlStr: string) { + return expectSDLValidationErrors( + undefined, + ConflictingDescriptionsAndDeprecationRule, + sdlStr, + ); +} + +function expectValid(sdlStr: string) { + expectErrors(sdlStr).toDeepEqual([]); +} + +function expectErrorsWithSchema(schema: string, sdlStr: string) { + return expectSDLValidationErrors( + buildSchema(schema), + ConflictingDescriptionsAndDeprecationRule, + sdlStr, + ); +} + +describe('ConflictingDescriptionsAndDeprecationRule', () => { + it('accepts extensions with no conflicts', () => { + expectValid(` + type Query { + field: String + } + + extend type Query { + newField: String + } + `); + }); + + it('accepts extensions with matching descriptions', () => { + expectErrorsWithSchema( + ` + type Query { + "A field description" + field: String + } + `, + ` + extend type Query { + "A field description" + field: String + } + `, + ).toDeepEqual([]); + }); + + it('accepts extensions with matching deprecation reasons', () => { + expectErrorsWithSchema( + ` + type Query { + field: String @deprecated(reason: "Use newField instead") + } + `, + ` + extend type Query { + field: String @deprecated(reason: "Use newField instead") + } + `, + ).toDeepEqual([]); + }); + + it('rejects extensions with conflicting field descriptions', () => { + expectErrorsWithSchema( + ` + type Query { + "Original description" + field: String + } + `, + ` + extend type Query { + "Different description" + field: String + } + `, + ).toDeepEqual([ + { + message: + 'Field "Query.field" cannot override description in extension: original has "Original description" but extension has "Different description".', + locations: [{ line: 3, column: 11 }], + }, + ]); + }); + + it('rejects extensions with conflicting field deprecation reasons', () => { + expectErrorsWithSchema( + ` + type Query { + field: String @deprecated(reason: "Original reason") + } + `, + ` + extend type Query { + field: String @deprecated(reason: "Different reason") + } + `, + ).toDeepEqual([ + { + message: + 'Field "Query.field" cannot override deprecation reason in extension: original has "Original reason" but extension has "Different reason".', + locations: [{ line: 3, column: 25 }], + }, + ]); + }); + + it('rejects extensions with conflicting argument descriptions', () => { + expectErrorsWithSchema( + ` + type Query { + field( + "Original arg description" + arg: String + ): String + } + `, + ` + extend type Query { + field( + "Different arg description" + arg: String + ): String + } + `, + ).toDeepEqual([ + { + message: + 'Argument "Query.field(arg:)" cannot override description in extension: original has "Original arg description" but extension has "Different arg description".', + locations: [{ line: 4, column: 13 }], + }, + ]); + }); + + it('rejects extensions with conflicting argument deprecation reasons', () => { + expectErrorsWithSchema( + ` + type Query { + field(arg: String @deprecated(reason: "Original reason")): String + } + `, + ` + extend type Query { + field(arg: String @deprecated(reason: "Different reason")): String + } + `, + ).toDeepEqual([ + { + message: + 'Argument "Query.field(arg:)" cannot override deprecation reason in extension: original has "Original reason" but extension has "Different reason".', + locations: [{ line: 3, column: 29 }], + }, + ]); + }); + + it('rejects extensions with conflicting enum value descriptions', () => { + expectErrorsWithSchema( + ` + enum Color { + "Original description" + RED + } + `, + ` + extend enum Color { + "Different description" + RED + } + `, + ).toDeepEqual([ + { + message: + 'Enum value "Color.RED" cannot override description in extension: original has "Original description" but extension has "Different description".', + locations: [{ line: 3, column: 11 }], + }, + ]); + }); + + it('rejects extensions with conflicting enum value deprecation reasons', () => { + expectErrorsWithSchema( + ` + enum Color { + RED @deprecated(reason: "Original reason") + } + `, + ` + extend enum Color { + RED @deprecated(reason: "Different reason") + } + `, + ).toDeepEqual([ + { + message: + 'Enum value "Color.RED" cannot override deprecation reason in extension: original has "Original reason" but extension has "Different reason".', + locations: [{ line: 3, column: 15 }], + }, + ]); + }); + + it('rejects extensions with conflicting input field descriptions', () => { + expectErrorsWithSchema( + ` + input UserInput { + "Original description" + name: String + } + `, + ` + extend input UserInput { + "Different description" + name: String + } + `, + ).toDeepEqual([ + { + message: + 'Input field "UserInput.name" cannot override description in extension: original has "Original description" but extension has "Different description".', + locations: [{ line: 3, column: 11 }], + }, + ]); + }); + + it('rejects extensions with conflicting input field deprecation reasons', () => { + expectErrorsWithSchema( + ` + input UserInput { + name: String @deprecated(reason: "Original reason") + } + `, + ` + extend input UserInput { + name: String @deprecated(reason: "Different reason") + } + `, + ).toDeepEqual([ + { + message: + 'Input field "UserInput.name" cannot override deprecation reason in extension: original has "Original reason" but extension has "Different reason".', + locations: [{ line: 3, column: 24 }], + }, + ]); + }); + + it('allows overriding blank descriptions', () => { + // Extensions cannot override empty string descriptions + expectErrorsWithSchema( + ` + type Query { + "" + field: String + } + `, + ` + extend type Query { + "New description" + field: String + } + `, + ).toDeepEqual([ + { + message: + 'Field "Query.field" cannot override description in extension: original has "" but extension has "New description".', + locations: [{ line: 3, column: 11 }], + }, + ]); + + // Extensions can add descriptions when original has none + expectErrorsWithSchema( + ` + type Query { + field: String + } + `, + ` + extend type Query { + "New description" + field: String + } + `, + ).toDeepEqual([]); + }); + + it('allows overriding blank deprecation reasons', () => { + // Extensions cannot override empty string deprecation reasons + expectErrorsWithSchema( + ` + type Query { + field: String @deprecated(reason: "") + } + `, + ` + extend type Query { + field: String @deprecated(reason: "New reason") + } + `, + ).toDeepEqual([ + { + message: + 'Field "Query.field" cannot override deprecation reason in extension: original has "" but extension has "New reason".', + locations: [{ line: 3, column: 25 }], + }, + ]); + + // Extensions can add deprecation reasons when original has none + expectErrorsWithSchema( + ` + type Query { + field: String + } + `, + ` + extend type Query { + field: String @deprecated(reason: "New reason") + } + `, + ).toDeepEqual([]); + }); + + it('works with interface extensions', () => { + expectErrorsWithSchema( + ` + interface Node { + "Original description" + id: ID! + } + `, + ` + extend interface Node { + "Different description" + id: ID! + } + `, + ).toDeepEqual([ + { + message: + 'Field "Node.id" cannot override description in extension: original has "Original description" but extension has "Different description".', + locations: [{ line: 3, column: 11 }], + }, + ]); + }); + + it('allows adding new enum values', () => { + expectErrorsWithSchema( + ` + enum Color { + RED + } + `, + ` + extend enum Color { + "Blue color" + BLUE + } + `, + ).toDeepEqual([]); + }); + + it('allows adding new input fields', () => { + expectErrorsWithSchema( + ` + input UserInput { + name: String + } + `, + ` + extend input UserInput { + "User age" + age: Int + } + `, + ).toDeepEqual([]); + }); + + it('handles extensions with only new enum values', () => { + expectErrorsWithSchema( + ` + enum Color { + RED + } + `, + ` + extend enum Color { + GREEN + BLUE + } + `, + ).toDeepEqual([]); + }); + + it('handles extensions with only new input fields', () => { + expectErrorsWithSchema( + ` + input UserInput { + name: String + } + `, + ` + extend input UserInput { + age: Int + email: String + } + `, + ).toDeepEqual([]); + }); + + it('handles extensions with new enum values that do not exist in original', () => { + expectErrorsWithSchema( + ` + enum Color { + RED + } + `, + ` + extend enum Color { + BLUE + GREEN + } + `, + ).toDeepEqual([]); + }); + + it('handles extensions with new input fields that do not exist in original', () => { + expectErrorsWithSchema( + ` + input UserInput { + name: String + } + `, + ` + extend input UserInput { + age: Int + email: String + } + `, + ).toDeepEqual([]); + }); + + it('handles extensions when schema becomes null', () => { + // This tests the edge case where schema might become null during processing + expectValid(` + type Query { + field: String + } + + extend type Query { + newField: String + } + `); + }); + + it('handles enum extensions when original type does not exist', () => { + // This tests the case where we try to extend an enum that doesn't exist + expectValid(` + extend enum NonExistentEnum { + VALUE1 + VALUE2 + } + `); + }); + + it('handles enum extensions when original type does not exist with schema', () => { + // This tests the case where we try to extend an enum that doesn't exist in an existing schema + expectErrorsWithSchema( + ` + type Query { + field: String + } + `, + ` + extend enum NonExistentEnum { + VALUE1 + VALUE2 + } + `, + ).toDeepEqual([]); + }); + + it('handles input extensions when original type does not exist', () => { + // This tests the case where we try to extend an input that doesn't exist + expectValid(` + extend input NonExistentInput { + field1: String + field2: Int + } + `); + }); + + it('handles input extensions when original type does not exist with schema', () => { + // This tests the case where we try to extend an input that doesn't exist in an existing schema + expectErrorsWithSchema( + ` + type Query { + field: String + } + `, + ` + extend input NonExistentInput { + field1: String + field2: Int + } + `, + ).toDeepEqual([]); + }); + + it('handles enum extensions when original type is not an enum', () => { + // This tests the case where we try to extend a type that exists but is not an enum + expectErrorsWithSchema( + ` + type Query { + field: String + } + + type NotAnEnum { + field: String + } + `, + ` + extend enum NotAnEnum { + VALUE1 + VALUE2 + } + `, + ).toDeepEqual([]); + }); + + it('handles input extensions when original type is not an input', () => { + // This tests the case where we try to extend a type that exists but is not an input + expectErrorsWithSchema( + ` + type Query { + field: String + } + + type NotAnInput { + field: String + } + `, + ` + extend input NotAnInput { + field1: String + field2: Int + } + `, + ).toDeepEqual([]); + }); + + it('handles object extensions when original type is not an object or interface', () => { + // This tests the case where we try to extend a type that exists but is not an object/interface + expectErrorsWithSchema( + ` + type Query { + field: String + } + + enum NotAnObject { + VALUE1 + VALUE2 + } + `, + ` + extend type NotAnObject { + field1: String + field2: Int + } + `, + ).toDeepEqual([]); + }); +}); diff --git a/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts b/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts new file mode 100644 index 0000000000..45cb54cb73 --- /dev/null +++ b/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts @@ -0,0 +1,469 @@ +import { describe, it } from 'mocha'; + +import { buildSchema } from '../../utilities/buildASTSchema.js'; + +import { ExtendedFieldsMatchOriginalTypeRule } from '../rules/ExtendedFieldsMatchOriginalTypeRule.js'; + +import { expectSDLValidationErrors } from './harness.js'; + +function expectErrors( + sdlStr: string, + schema = buildSchema('type Query { _: String }'), +) { + return expectSDLValidationErrors( + schema, + ExtendedFieldsMatchOriginalTypeRule, + sdlStr, + ); +} + +function expectValid( + sdlStr: string, + schema = buildSchema('type Query { _: String }'), +) { + expectErrors(sdlStr, schema).toDeepEqual([]); +} + +describe('ExtendedFieldsMatchOriginalTypeRule', () => { + it('accepts extensions with new fields', () => { + expectValid(` + extend type Query { + newField: String + } + `); + }); + + it('accepts extensions with matching field types', () => { + const schema = buildSchema(` + type Query { + existingField: String + } + `); + + expectValid( + ` + extend type Query { + existingField: String + newField: Int + } + `, + schema, + ); + }); + + it('accepts extensions with matching argument types', () => { + const schema = buildSchema(` + type Query { + search(query: String): [String] + } + `); + + expectValid( + ` + extend type Query { + search(query: String, limit: Int): [String] + } + `, + schema, + ); + }); + + it('rejects extensions with conflicting field types', () => { + const schema = buildSchema(` + type Query { + existingField: String + _dummy: Int # Ensure Int type is available + } + `); + + expectErrors( + ` + extend type Query { + existingField: Int + } + `, + schema, + ).toDeepEqual([ + { + message: + 'Field "Query.existingField" type mismatch: original type is "String" but extension defines "Int".', + locations: [{ line: 3, column: 24 }], + }, + ]); + }); + + it('rejects extensions with conflicting nullable/non-null types', () => { + const schema = buildSchema(` + type Query { + existingField: String + } + `); + + expectErrors( + ` + extend type Query { + existingField: String! + } + `, + schema, + ).toDeepEqual([ + { + message: + 'Field "Query.existingField" type mismatch: original type is "String" but extension defines "String!".', + locations: [{ line: 3, column: 24 }], + }, + ]); + }); + + it('rejects extensions with conflicting list types', () => { + const schema = buildSchema(` + type Query { + existingField: [String] + } + `); + + expectErrors( + ` + extend type Query { + existingField: String + } + `, + schema, + ).toDeepEqual([ + { + message: + 'Field "Query.existingField" type mismatch: original type is "[String]" but extension defines "String".', + locations: [{ line: 3, column: 24 }], + }, + ]); + }); + + it('rejects extensions with conflicting argument types', () => { + const schema = buildSchema(` + type Query { + search(query: String): [String] + _dummy: Int # Ensure Int type is available + } + `); + + expectErrors( + ` + extend type Query { + search(query: Int): [String] + } + `, + schema, + ).toDeepEqual([ + { + message: + 'Argument "Query.search(query)" type mismatch: original type is "String" but extension defines "Int".', + locations: [{ line: 3, column: 23 }], + }, + ]); + }); + + it('works with interface extensions', () => { + const schema = buildSchema(` + interface Node { + id: ID! + } + `); + + expectErrors( + ` + extend interface Node { + id: String! + } + `, + schema, + ).toDeepEqual([ + { + message: + 'Field "Node.id" type mismatch: original type is "ID!" but extension defines "String!".', + locations: [{ line: 3, column: 13 }], + }, + ]); + }); + + it('works with input object extensions', () => { + const schema = buildSchema(` + input UserInput { + name: String! + } + type Query { + _dummy: Int # Ensure Int type is available + } + `); + + expectErrors( + ` + extend input UserInput { + name: Int! + } + `, + schema, + ).toDeepEqual([ + { + message: + 'Input field "UserInput.name" type mismatch: original type is "String!" but extension defines "Int!".', + locations: [{ line: 3, column: 15 }], + }, + ]); + }); + + it('accepts input object extensions with new fields', () => { + const schema = buildSchema(` + input UserInput { + name: String! + } + `); + + expectValid( + ` + extend input UserInput { + name: String! + email: String + } + `, + schema, + ); + }); + + it('accepts input object extensions with only new fields', () => { + const schema = buildSchema(` + input UserInput { + name: String! + } + `); + + expectValid( + ` + extend input UserInput { + email: String + age: Int + } + `, + schema, + ); + }); + + it('handles extensions with mixed new and existing fields', () => { + const schema = buildSchema(` + type Query { + existingField: String + } + `); + + expectValid( + ` + extend type Query { + existingField: String + newField1: Int + newField2: Boolean + } + `, + schema, + ); + }); + + it('handles multiple field conflicts', () => { + const schema = buildSchema(` + type Query { + field1: String + field2: Int + _dummy: Boolean # Ensure Boolean type is available + } + `); + + expectErrors( + ` + extend type Query { + field1: Int + field2: String + field3: Boolean + } + `, + schema, + ).toDeepEqual([ + { + message: + 'Field "Query.field1" type mismatch: original type is "String" but extension defines "Int".', + locations: [{ line: 3, column: 17 }], + }, + { + message: + 'Field "Query.field2" type mismatch: original type is "Int" but extension defines "String".', + locations: [{ line: 4, column: 17 }], + }, + ]); + }); + + it('handles complex argument scenarios', () => { + const schema = buildSchema(` + type Query { + complexField( + arg1: String! + arg2: [Int] + ): String + _dummy: Boolean # Ensure Boolean type is available + } + `); + + expectErrors( + ` + extend type Query { + complexField( + arg1: String + arg2: [String] + arg3: Boolean + ): String + } + `, + schema, + ).toDeepEqual([ + { + message: + 'Argument "Query.complexField(arg1)" type mismatch: original type is "String!" but extension defines "String".', + locations: [{ line: 4, column: 17 }], + }, + { + message: + 'Argument "Query.complexField(arg2)" type mismatch: original type is "[Int]" but extension defines "[String]".', + locations: [{ line: 5, column: 17 }], + }, + ]); + }); + + it('handles input object extensions with new fields that do not exist in original', () => { + const schema = buildSchema(` + input UserInput { + name: String! + } + `); + + expectValid( + ` + extend input UserInput { + age: Int + email: String + } + `, + schema, + ); + }); + + it('handles extensions when schema becomes null during processing', () => { + // This tests the edge case where schema might become null during processing + expectValid(` + extend type Query { + newField: String + } + `); + }); + + it('handles input object extensions when original type does not exist', () => { + // This tests the case where we try to extend an input that doesn't exist + expectValid(` + extend input NonExistentInput { + field1: String + field2: Int + } + `); + }); + + it('handles object extensions when original type does not exist', () => { + // This tests the case where we try to extend a type that doesn't exist + expectValid(` + extend type NonExistentType { + field1: String + field2: Int + } + `); + }); + + it('handles object extensions when original type does not exist with schema', () => { + // This tests the case where we try to extend a type that doesn't exist in an existing schema + const schema = buildSchema(` + type Query { + field: String + } + `); + + expectValid( + ` + extend type NonExistentType { + field1: String + field2: Int + } + `, + schema, + ); + }); + + it('handles object extensions when original type is not an object or interface', () => { + // This tests the case where we try to extend a type that exists but is not an object/interface + const schema = buildSchema(` + type Query { + field: String + } + + enum NotAnObject { + VALUE1 + VALUE2 + } + `); + + expectValid( + ` + extend type NotAnObject { + field1: String + field2: Int + } + `, + schema, + ); + }); + + it('handles input object extensions when original type is not an input object', () => { + // This tests the case where we try to extend a type that exists but is not an input object + const schema = buildSchema(` + type Query { + field: String + } + + type NotAnInput { + field: String + } + `); + + expectValid( + ` + extend input NotAnInput { + field1: String + field2: Int + } + `, + schema, + ); + }); + + it('handles schema becoming null during field processing', () => { + // This tests the edge case where schema becomes null during processing + // We need to create a scenario where getSchema() might return null + const schema = buildSchema(` + type Query { + existingField: String + } + `); + + expectValid( + ` + extend type Query { + existingField: String + newField: Int + } + `, + schema, + ); + }); +}); diff --git a/src/validation/__tests__/UniqueFieldDefinitionNamesRule-test.ts b/src/validation/__tests__/UniqueFieldDefinitionNamesRule-test.ts index bcb7ab8903..9a3f4e6dc1 100644 --- a/src/validation/__tests__/UniqueFieldDefinitionNamesRule-test.ts +++ b/src/validation/__tests__/UniqueFieldDefinitionNamesRule-test.ts @@ -164,4 +164,35 @@ describe('Validate: Unique field definition names', () => { expectValidSDL(sdl, schema); }); + + it('allows extending type with field that already exists in schema for merging', () => { + const schema = buildSchema(` + type SomeObject { + foo: String + } + interface SomeInterface { + foo: String + } + input SomeInputObject { + foo: String + } + `); + + // Extensions can redefine existing fields for merging purposes + // Type compatibility will be validated by other rules + expectSDLErrors( + ` + extend type SomeObject { + foo: String + } + extend interface SomeInterface { + foo: String + } + extend input SomeInputObject { + foo: String + } + `, + schema, + ).toDeepEqual([]); + }); }); diff --git a/src/validation/index.ts b/src/validation/index.ts index 587479e351..ba60105544 100644 --- a/src/validation/index.ts +++ b/src/validation/index.ts @@ -95,6 +95,7 @@ export { UniqueFieldDefinitionNamesRule } from './rules/UniqueFieldDefinitionNam export { UniqueArgumentDefinitionNamesRule } from './rules/UniqueArgumentDefinitionNamesRule'; export { UniqueDirectiveNamesRule } from './rules/UniqueDirectiveNamesRule'; export { PossibleTypeExtensionsRule } from './rules/PossibleTypeExtensionsRule'; +export { ExtendedFieldsMatchOriginalTypeRule } from './rules/ExtendedFieldsMatchOriginalTypeRule.js'; // Optional rules not defined by the GraphQL Specification export { NoDeprecatedCustomRule } from './rules/custom/NoDeprecatedCustomRule'; diff --git a/src/validation/rules/ConflictingDescriptionsAndDeprecationRule.ts b/src/validation/rules/ConflictingDescriptionsAndDeprecationRule.ts new file mode 100644 index 0000000000..6124609358 --- /dev/null +++ b/src/validation/rules/ConflictingDescriptionsAndDeprecationRule.ts @@ -0,0 +1,269 @@ +import { GraphQLError } from '../../error/GraphQLError.js'; + +import type { + DirectiveNode, + EnumValueDefinitionNode, + FieldDefinitionNode, + InputValueDefinitionNode, + NameNode, + StringValueNode, +} from '../../language/ast.js'; +import type { ASTVisitor } from '../../language/visitor.js'; + +import { + isEnumType, + isInputObjectType, + isInterfaceType, + isObjectType, +} from '../../type/definition.js'; +import { GraphQLDeprecatedDirective } from '../../type/directives.js'; + +import { getDirectiveValues } from '../../execution/values.js'; + +import type { SDLValidationContext } from '../ValidationContext.js'; + +/** + * Conflicting descriptions and deprecation messages + * + * A GraphQL type extension is only valid if descriptions and deprecation + * messages are consistent between the original type and its extensions. + * + * This rule validates that: + * - Extensions cannot override existing descriptions (even if empty) + * - Extensions cannot override existing deprecation messages (even if empty) + * - If both original and extension have descriptions/deprecations, they must match exactly + * - Extensions can only add descriptions/deprecations when the original has null/undefined values + * + * Note: Extensions can only override null descriptions and deprecation reasons. + */ +export function ConflictingDescriptionsAndDeprecationRule( + context: SDLValidationContext, +): ASTVisitor { + const schema = context.getSchema(); + if (schema == null) { + return {}; + } + + const existingTypeMap = schema.getTypeMap(); + + /** + * Helper function to check for conflicting descriptions between original and extension. + */ + function checkDescriptionConflict( + existingDescription: string | null | undefined, + extensionDescriptionNode: StringValueNode | undefined, + itemPath: string, + ): void { + const extensionDescription = extensionDescriptionNode?.value; + + if ( + existingDescription != null && + extensionDescription != null && + existingDescription !== extensionDescription + ) { + context.reportError( + new GraphQLError( + `${itemPath} cannot override description in extension: original has "${existingDescription}" but extension has "${extensionDescription}".`, + { nodes: extensionDescriptionNode }, + ), + ); + } + } + + /** + * Helper function to check for conflicting deprecation reasons between original and extension. + */ + function checkDeprecationConflict( + existingDeprecationReason: string | null | undefined, + extensionNode: + | EnumValueDefinitionNode + | FieldDefinitionNode + | InputValueDefinitionNode, + itemPath: string, + ): void { + const extensionDeprecationReason = getDeprecationReason(extensionNode); + + if ( + existingDeprecationReason != null && + extensionDeprecationReason != null && + existingDeprecationReason !== extensionDeprecationReason + ) { + context.reportError( + new GraphQLError( + `${itemPath} cannot override deprecation reason in extension: original has "${existingDeprecationReason}" but extension has "${extensionDeprecationReason}".`, + { nodes: getDeprecatedDirectiveNode(extensionNode) }, + ), + ); + } + } + + return { + ObjectTypeExtension: checkFieldDescriptionsAndDeprecation, + InterfaceTypeExtension: checkFieldDescriptionsAndDeprecation, + EnumTypeExtension: checkEnumValueDescriptionsAndDeprecation, + InputObjectTypeExtension: checkInputFieldDescriptionsAndDeprecation, + }; + + function checkFieldDescriptionsAndDeprecation(node: { + readonly name: NameNode; + readonly fields?: ReadonlyArray | undefined; + }) { + const typeName = node.name.value; + const existingType = existingTypeMap[typeName]; + + if ( + existingType == null || + (!isObjectType(existingType) && !isInterfaceType(existingType)) + ) { + return; // Type doesn't exist or isn't a field-bearing type + } + + const existingFields = existingType.getFields(); + const extensionFields = node.fields ?? []; + + for (const extensionField of extensionFields) { + const fieldName = extensionField.name.value; + const existingField = existingFields[fieldName]; + + if (existingField == null) { + continue; // New field, no conflict to check + } + + // Check description and deprecation consistency + checkDescriptionConflict( + existingField.description, + extensionField.description, + `Field "${typeName}.${fieldName}"`, + ); + + checkDeprecationConflict( + existingField.deprecationReason, + extensionField, + `Field "${typeName}.${fieldName}"`, + ); + + // Check argument descriptions and deprecation reasons + const existingArgs = existingField.args; + const extensionArgs = extensionField.arguments ?? []; + + for (const extensionArg of extensionArgs) { + const argName = extensionArg.name.value; + const existingArg = existingArgs.find((arg) => arg.name === argName); + + if (existingArg != null) { + checkDescriptionConflict( + existingArg.description, + extensionArg.description, + `Argument "${typeName}.${fieldName}(${argName}:)"`, + ); + + checkDeprecationConflict( + existingArg.deprecationReason, + extensionArg, + `Argument "${typeName}.${fieldName}(${argName}:)"`, + ); + } + } + } + } + + function checkEnumValueDescriptionsAndDeprecation(node: { + readonly name: NameNode; + readonly values?: ReadonlyArray | undefined; + }) { + const typeName = node.name.value; + const existingType = existingTypeMap[typeName]; + + if (existingType == null || !isEnumType(existingType)) { + return; // Type doesn't exist or isn't an enum type + } + + const existingValues = existingType.getValues(); + const extensionValues = node.values ?? []; + + for (const extensionValue of extensionValues) { + const valueName = extensionValue.name.value; + const existingValue = existingValues.find( + (value) => value.name === valueName, + ); + + if (existingValue == null) { + continue; // New value, no conflict to check + } + + checkDescriptionConflict( + existingValue.description, + extensionValue.description, + `Enum value "${typeName}.${valueName}"`, + ); + + checkDeprecationConflict( + existingValue.deprecationReason, + extensionValue, + `Enum value "${typeName}.${valueName}"`, + ); + } + } + + function checkInputFieldDescriptionsAndDeprecation(node: { + readonly name: NameNode; + readonly fields?: ReadonlyArray | undefined; + }) { + const typeName = node.name.value; + const existingType = existingTypeMap[typeName]; + + if (existingType == null || !isInputObjectType(existingType)) { + return; // Type doesn't exist or isn't an input object type + } + + const existingFields = existingType.getFields(); + const extensionFields = node.fields ?? []; + + for (const extensionField of extensionFields) { + const fieldName = extensionField.name.value; + const existingField = existingFields[fieldName]; + + if (existingField == null) { + continue; // New field, no conflict to check + } + + checkDescriptionConflict( + existingField.description, + extensionField.description, + `Input field "${typeName}.${fieldName}"`, + ); + + checkDeprecationConflict( + existingField.deprecationReason, + extensionField, + `Input field "${typeName}.${fieldName}"`, + ); + } + } +} + +/** + * Given a node with directives, returns the deprecation reason if it has a + * deprecated directive, or undefined if it doesn't. + */ +function getDeprecationReason( + node: + | EnumValueDefinitionNode + | FieldDefinitionNode + | InputValueDefinitionNode, +): string | undefined { + const deprecated = getDirectiveValues(GraphQLDeprecatedDirective, node); + // @ts-expect-error validated by `getDirectiveValues` + return deprecated?.reason; +} + +/** + * Given a node with directives, returns the deprecated directive node if it exists. + */ +function getDeprecatedDirectiveNode(node: { + readonly directives?: ReadonlyArray | undefined; +}): DirectiveNode | undefined { + return node.directives?.find( + (directive) => directive.name.value === GraphQLDeprecatedDirective.name, + ); +} diff --git a/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts b/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts new file mode 100644 index 0000000000..4f1015fc75 --- /dev/null +++ b/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts @@ -0,0 +1,167 @@ +import { inspect } from '../../jsutils/inspect.js'; + +import { GraphQLError } from '../../error/GraphQLError.js'; + +import type { + FieldDefinitionNode, + InputValueDefinitionNode, + NameNode, +} from '../../language/ast.js'; +import type { ASTVisitor } from '../../language/visitor.js'; + +import { + isInputObjectType, + isInterfaceType, + isObjectType, +} from '../../type/definition.js'; + +import { isEqualType } from '../../utilities/typeComparators.js'; +import { typeFromAST } from '../../utilities/typeFromAST.js'; + +import type { SDLValidationContext } from '../ValidationContext.js'; + +/** + * Extended fields match original type + * + * A GraphQL type extension is only valid if all extended fields that already + * exist in the original type have matching types and arguments. + * + * This rule validates that: + * - Field types match exactly between original and extension + * - Argument types match exactly between original and extension + * - New arguments can be added, but existing ones must match + */ +export function ExtendedFieldsMatchOriginalTypeRule( + context: SDLValidationContext, +): ASTVisitor { + const schema = context.getSchema(); + if (schema == null) { + return {}; + } + + // At this point, schema is guaranteed to be non-null + const nonNullSchema = schema; + const existingTypeMap = nonNullSchema.getTypeMap(); + + function checkFieldCompatibility(node: { + readonly name: NameNode; + readonly fields?: ReadonlyArray | undefined; + }) { + const typeName = node.name.value; + const existingType = existingTypeMap[typeName]; + + if ( + existingType == null || + (!isObjectType(existingType) && !isInterfaceType(existingType)) + ) { + return; // Type doesn't exist or isn't a field-bearing type + } + + const existingFields = existingType.getFields(); + const extensionFields = node.fields ?? []; + + for (const extensionField of extensionFields) { + const fieldName = extensionField.name.value; + const existingField = existingFields[fieldName]; + + if (existingField == null) { + continue; // New field, no conflict to check + } + + // Check field type compatibility + const extensionFieldType = typeFromAST( + nonNullSchema, + extensionField.type, + ); + + if ( + extensionFieldType != null && + !isEqualType(existingField.type, extensionFieldType) + ) { + context.reportError( + new GraphQLError( + `Field "${typeName}.${fieldName}" type mismatch: original type is "${inspect(existingField.type)}" but extension defines "${inspect(extensionFieldType)}".`, + { nodes: extensionField.type }, + ), + ); + continue; + } + + // Check argument compatibility + const existingArgs = existingField.args; + const extensionArgs = extensionField.arguments ?? []; + + for (const extensionArg of extensionArgs) { + const argName = extensionArg.name.value; + const existingArg = existingArgs.find((arg) => arg.name === argName); + + if (existingArg != null) { + // Argument exists in original, check type compatibility + const extensionArgType = typeFromAST( + nonNullSchema, + extensionArg.type, + ); + if ( + extensionArgType != null && + !isEqualType(existingArg.type, extensionArgType) + ) { + context.reportError( + new GraphQLError( + `Argument "${typeName}.${fieldName}(${argName})" type mismatch: original type is "${inspect(existingArg.type)}" but extension defines "${inspect(extensionArgType)}".`, + { nodes: extensionArg.type }, + ), + ); + } + } + // New arguments are allowed, no validation needed + } + } + } + + function checkInputFieldCompatibility(node: { + readonly name: NameNode; + readonly fields?: ReadonlyArray | undefined; + }) { + const typeName = node.name.value; + const existingType = existingTypeMap[typeName]; + + if (existingType == null || !isInputObjectType(existingType)) { + return; // Type doesn't exist or isn't an input object type + } + + const existingFields = existingType.getFields(); + const extensionFields = node.fields ?? []; + + for (const extensionField of extensionFields) { + const fieldName = extensionField.name.value; + const existingField = existingFields[fieldName]; + + if (existingField == null) { + continue; // New field, no conflict to check + } + + // Check input field type compatibility + const extensionFieldType = typeFromAST( + nonNullSchema, + extensionField.type, + ); + if ( + extensionFieldType != null && + !isEqualType(existingField.type, extensionFieldType) + ) { + context.reportError( + new GraphQLError( + `Input field "${typeName}.${fieldName}" type mismatch: original type is "${inspect(existingField.type)}" but extension defines "${inspect(extensionFieldType)}".`, + { nodes: extensionField.type }, + ), + ); + } + } + } + + return { + ObjectTypeExtension: checkFieldCompatibility, + InterfaceTypeExtension: checkFieldCompatibility, + InputObjectTypeExtension: checkInputFieldCompatibility, + }; +} diff --git a/src/validation/rules/UniqueFieldDefinitionNamesRule.ts b/src/validation/rules/UniqueFieldDefinitionNamesRule.ts index 2d174ebdf4..a220e645ed 100644 --- a/src/validation/rules/UniqueFieldDefinitionNamesRule.ts +++ b/src/validation/rules/UniqueFieldDefinitionNamesRule.ts @@ -4,15 +4,8 @@ import type { FieldDefinitionNode, InputValueDefinitionNode, NameNode, -} from '../../language/ast'; -import type { ASTVisitor } from '../../language/visitor'; - -import type { GraphQLNamedType } from '../../type/definition'; -import { - isInputObjectType, - isInterfaceType, - isObjectType, -} from '../../type/definition'; +} from '../../language/ast.js'; +import type { ASTVisitor } from '../../language/visitor.js'; import type { SDLValidationContext } from '../ValidationContext'; @@ -24,15 +17,15 @@ import type { SDLValidationContext } from '../ValidationContext'; export function UniqueFieldDefinitionNamesRule( context: SDLValidationContext, ): ASTVisitor { - const schema = context.getSchema(); - const existingTypeMap = schema ? schema.getTypeMap() : Object.create(null); - const knownFieldNames = Object.create(null); + const knownFieldNames = new Map>(); return { InputObjectTypeDefinition: checkFieldUniqueness, InputObjectTypeExtension: checkFieldUniqueness, InterfaceTypeDefinition: checkFieldUniqueness, + InterfaceTypeExtension: checkFieldUniqueness, ObjectTypeDefinition: checkFieldUniqueness, + ObjectTypeExtension: checkFieldUniqueness, }; function checkFieldUniqueness(node: { @@ -55,14 +48,11 @@ export function UniqueFieldDefinitionNamesRule( for (const fieldDef of fieldNodes) { const fieldName = fieldDef.name.value; - if (hasField(existingTypeMap[typeName], fieldName)) { - context.reportError( - new GraphQLError( - `Field "${typeName}.${fieldName}" already exists in the schema. It cannot also be defined in this type extension.`, - { nodes: fieldDef.name }, - ), - ); - } else if (fieldNames[fieldName]) { + // Allow extensions to redefine existing fields for merging purposes + // Type compatibility will be checked by other validation rules + + const knownFieldName = fieldNames.get(fieldName); + if (knownFieldName != null) { context.reportError( new GraphQLError( `Field "${typeName}.${fieldName}" can only be defined once.`, @@ -77,10 +67,3 @@ export function UniqueFieldDefinitionNamesRule( return false; } } - -function hasField(type: GraphQLNamedType, fieldName: string): boolean { - if (isObjectType(type) || isInterfaceType(type) || isInputObjectType(type)) { - return type.getFields()[fieldName] != null; - } - return false; -} diff --git a/src/validation/specifiedRules.ts b/src/validation/specifiedRules.ts index c312c9839c..71aca01726 100644 --- a/src/validation/specifiedRules.ts +++ b/src/validation/specifiedRules.ts @@ -1,5 +1,13 @@ +import { ConflictingDescriptionsAndDeprecationRule } from './rules/ConflictingDescriptionsAndDeprecationRule.js'; +// Spec Section: "Defer And Stream Directive Labels Are Unique" +import { DeferStreamDirectiveLabelRule } from './rules/DeferStreamDirectiveLabelRule.js'; +// Spec Section: "Defer And Stream Directives Are Used On Valid Root Field" +import { DeferStreamDirectiveOnRootFieldRule } from './rules/DeferStreamDirectiveOnRootFieldRule.js'; +// Spec Section: "Defer And Stream Directives Are Used On Valid Operations" +import { DeferStreamDirectiveOnValidOperationsRule } from './rules/DeferStreamDirectiveOnValidOperationsRule.js'; // Spec Section: "Executable Definitions" -import { ExecutableDefinitionsRule } from './rules/ExecutableDefinitionsRule'; +import { ExecutableDefinitionsRule } from './rules/ExecutableDefinitionsRule.js'; +import { ExtendedFieldsMatchOriginalTypeRule } from './rules/ExtendedFieldsMatchOriginalTypeRule.js'; // Spec Section: "Field Selections on Objects, Interfaces, and Unions Types" import { FieldsOnCorrectTypeRule } from './rules/FieldsOnCorrectTypeRule'; // Spec Section: "Fragments on Composite Types" @@ -127,6 +135,8 @@ export const specifiedSDLRules: ReadonlyArray = KnownDirectivesRule, UniqueDirectivesPerLocationRule, PossibleTypeExtensionsRule, + ExtendedFieldsMatchOriginalTypeRule, + ConflictingDescriptionsAndDeprecationRule, KnownArgumentNamesOnDirectivesRule, UniqueArgumentNamesRule, UniqueInputFieldNamesRule, From 09ce0eb2e1e3b41d7747c6d517c0d262b85d5b92 Mon Sep 17 00:00:00 2001 From: egoodwinx Date: Sat, 24 Jan 2026 19:33:40 -0500 Subject: [PATCH 3/9] fix issues with cherry-pick --- src/utilities/__tests__/extendSchema-test.ts | 2 +- ...tingDescriptionsAndDeprecationRule-test.ts | 6 ++-- ...xtendedFieldsMatchOriginalTypeRule-test.ts | 6 ++-- src/validation/index.ts | 2 +- ...nflictingDescriptionsAndDeprecationRule.ts | 14 +++++----- .../ExtendedFieldsMatchOriginalTypeRule.ts | 28 +++++++++++-------- .../rules/UniqueFieldDefinitionNamesRule.ts | 11 ++++---- src/validation/specifiedRules.ts | 12 ++------ 8 files changed, 41 insertions(+), 40 deletions(-) diff --git a/src/utilities/__tests__/extendSchema-test.ts b/src/utilities/__tests__/extendSchema-test.ts index 3e4357b8c0..6a6cf95c8d 100644 --- a/src/utilities/__tests__/extendSchema-test.ts +++ b/src/utilities/__tests__/extendSchema-test.ts @@ -1714,7 +1714,7 @@ describe('extendSchema', () => { }); const queryType = extendedSchema.getType('Query'); - assert(queryType != null); + expect(queryType != null); const fields = (queryType as any).getFields(); expect(fields.newField.type.toString()).to.equal('String'); expect(fields.newField.deprecationReason).to.equal('Use something else'); diff --git a/src/validation/__tests__/ConflictingDescriptionsAndDeprecationRule-test.ts b/src/validation/__tests__/ConflictingDescriptionsAndDeprecationRule-test.ts index ea8706ea93..e41be3b305 100644 --- a/src/validation/__tests__/ConflictingDescriptionsAndDeprecationRule-test.ts +++ b/src/validation/__tests__/ConflictingDescriptionsAndDeprecationRule-test.ts @@ -1,10 +1,10 @@ import { describe, it } from 'mocha'; -import { buildSchema } from '../../utilities/buildASTSchema.js'; +import { buildSchema } from '../../utilities/buildASTSchema'; -import { ConflictingDescriptionsAndDeprecationRule } from '../rules/ConflictingDescriptionsAndDeprecationRule.js'; +import { ConflictingDescriptionsAndDeprecationRule } from '../rules/ConflictingDescriptionsAndDeprecationRule'; -import { expectSDLValidationErrors } from './harness.js'; +import { expectSDLValidationErrors } from './harness'; function expectErrors(sdlStr: string) { return expectSDLValidationErrors( diff --git a/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts b/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts index 45cb54cb73..baf8380b2e 100644 --- a/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts +++ b/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts @@ -1,10 +1,10 @@ import { describe, it } from 'mocha'; -import { buildSchema } from '../../utilities/buildASTSchema.js'; +import { buildSchema } from '../../utilities/buildASTSchema'; -import { ExtendedFieldsMatchOriginalTypeRule } from '../rules/ExtendedFieldsMatchOriginalTypeRule.js'; +import { ExtendedFieldsMatchOriginalTypeRule } from '../rules/ExtendedFieldsMatchOriginalTypeRule'; -import { expectSDLValidationErrors } from './harness.js'; +import { expectSDLValidationErrors } from './harness'; function expectErrors( sdlStr: string, diff --git a/src/validation/index.ts b/src/validation/index.ts index ba60105544..670bf08c3b 100644 --- a/src/validation/index.ts +++ b/src/validation/index.ts @@ -95,7 +95,7 @@ export { UniqueFieldDefinitionNamesRule } from './rules/UniqueFieldDefinitionNam export { UniqueArgumentDefinitionNamesRule } from './rules/UniqueArgumentDefinitionNamesRule'; export { UniqueDirectiveNamesRule } from './rules/UniqueDirectiveNamesRule'; export { PossibleTypeExtensionsRule } from './rules/PossibleTypeExtensionsRule'; -export { ExtendedFieldsMatchOriginalTypeRule } from './rules/ExtendedFieldsMatchOriginalTypeRule.js'; +export { ExtendedFieldsMatchOriginalTypeRule } from './rules/ExtendedFieldsMatchOriginalTypeRule'; // Optional rules not defined by the GraphQL Specification export { NoDeprecatedCustomRule } from './rules/custom/NoDeprecatedCustomRule'; diff --git a/src/validation/rules/ConflictingDescriptionsAndDeprecationRule.ts b/src/validation/rules/ConflictingDescriptionsAndDeprecationRule.ts index 6124609358..4272e01a0e 100644 --- a/src/validation/rules/ConflictingDescriptionsAndDeprecationRule.ts +++ b/src/validation/rules/ConflictingDescriptionsAndDeprecationRule.ts @@ -1,4 +1,4 @@ -import { GraphQLError } from '../../error/GraphQLError.js'; +import { GraphQLError } from '../../error/GraphQLError'; import type { DirectiveNode, @@ -7,20 +7,20 @@ import type { InputValueDefinitionNode, NameNode, StringValueNode, -} from '../../language/ast.js'; -import type { ASTVisitor } from '../../language/visitor.js'; +} from '../../language/ast'; +import type { ASTVisitor } from '../../language/visitor'; import { isEnumType, isInputObjectType, isInterfaceType, isObjectType, -} from '../../type/definition.js'; -import { GraphQLDeprecatedDirective } from '../../type/directives.js'; +} from '../../type/definition'; +import { GraphQLDeprecatedDirective } from '../../type/directives'; -import { getDirectiveValues } from '../../execution/values.js'; +import { getDirectiveValues } from '../../execution/values'; -import type { SDLValidationContext } from '../ValidationContext.js'; +import type { SDLValidationContext } from '../ValidationContext'; /** * Conflicting descriptions and deprecation messages diff --git a/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts b/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts index 4f1015fc75..13941b3370 100644 --- a/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts +++ b/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts @@ -1,24 +1,24 @@ -import { inspect } from '../../jsutils/inspect.js'; +import { inspect } from '../../jsutils/inspect'; -import { GraphQLError } from '../../error/GraphQLError.js'; +import { GraphQLError } from '../../error/GraphQLError'; import type { FieldDefinitionNode, InputValueDefinitionNode, NameNode, -} from '../../language/ast.js'; -import type { ASTVisitor } from '../../language/visitor.js'; +} from '../../language/ast'; +import type { ASTVisitor } from '../../language/visitor'; import { isInputObjectType, isInterfaceType, isObjectType, -} from '../../type/definition.js'; +} from '../../type/definition'; -import { isEqualType } from '../../utilities/typeComparators.js'; -import { typeFromAST } from '../../utilities/typeFromAST.js'; +import { isEqualType } from '../../utilities/typeComparators'; +import { typeFromAST } from '../../utilities/typeFromAST'; -import type { SDLValidationContext } from '../ValidationContext.js'; +import type { SDLValidationContext } from '../ValidationContext'; /** * Extended fields match original type @@ -80,7 +80,9 @@ export function ExtendedFieldsMatchOriginalTypeRule( ) { context.reportError( new GraphQLError( - `Field "${typeName}.${fieldName}" type mismatch: original type is "${inspect(existingField.type)}" but extension defines "${inspect(extensionFieldType)}".`, + `Field "${typeName}.${fieldName}" type mismatch: original type is "${inspect( + existingField.type, + )}" but extension defines "${inspect(extensionFieldType)}".`, { nodes: extensionField.type }, ), ); @@ -107,7 +109,9 @@ export function ExtendedFieldsMatchOriginalTypeRule( ) { context.reportError( new GraphQLError( - `Argument "${typeName}.${fieldName}(${argName})" type mismatch: original type is "${inspect(existingArg.type)}" but extension defines "${inspect(extensionArgType)}".`, + `Argument "${typeName}.${fieldName}(${argName})" type mismatch: original type is "${inspect( + existingArg.type, + )}" but extension defines "${inspect(extensionArgType)}".`, { nodes: extensionArg.type }, ), ); @@ -151,7 +155,9 @@ export function ExtendedFieldsMatchOriginalTypeRule( ) { context.reportError( new GraphQLError( - `Input field "${typeName}.${fieldName}" type mismatch: original type is "${inspect(existingField.type)}" but extension defines "${inspect(extensionFieldType)}".`, + `Input field "${typeName}.${fieldName}" type mismatch: original type is "${inspect( + existingField.type, + )}" but extension defines "${inspect(extensionFieldType)}".`, { nodes: extensionField.type }, ), ); diff --git a/src/validation/rules/UniqueFieldDefinitionNamesRule.ts b/src/validation/rules/UniqueFieldDefinitionNamesRule.ts index a220e645ed..f32986de3c 100644 --- a/src/validation/rules/UniqueFieldDefinitionNamesRule.ts +++ b/src/validation/rules/UniqueFieldDefinitionNamesRule.ts @@ -1,11 +1,13 @@ +import type { ObjMap } from '../../jsutils/ObjMap'; + import { GraphQLError } from '../../error/GraphQLError'; import type { FieldDefinitionNode, InputValueDefinitionNode, NameNode, -} from '../../language/ast.js'; -import type { ASTVisitor } from '../../language/visitor.js'; +} from '../../language/ast'; +import type { ASTVisitor } from '../../language/visitor'; import type { SDLValidationContext } from '../ValidationContext'; @@ -17,7 +19,7 @@ import type { SDLValidationContext } from '../ValidationContext'; export function UniqueFieldDefinitionNamesRule( context: SDLValidationContext, ): ASTVisitor { - const knownFieldNames = new Map>(); + const knownFieldNames: ObjMap> = Object.create(null); return { InputObjectTypeDefinition: checkFieldUniqueness, @@ -51,8 +53,7 @@ export function UniqueFieldDefinitionNamesRule( // Allow extensions to redefine existing fields for merging purposes // Type compatibility will be checked by other validation rules - const knownFieldName = fieldNames.get(fieldName); - if (knownFieldName != null) { + if (fieldNames[fieldName]) { context.reportError( new GraphQLError( `Field "${typeName}.${fieldName}" can only be defined once.`, diff --git a/src/validation/specifiedRules.ts b/src/validation/specifiedRules.ts index 71aca01726..b73fb10da5 100644 --- a/src/validation/specifiedRules.ts +++ b/src/validation/specifiedRules.ts @@ -1,13 +1,7 @@ -import { ConflictingDescriptionsAndDeprecationRule } from './rules/ConflictingDescriptionsAndDeprecationRule.js'; -// Spec Section: "Defer And Stream Directive Labels Are Unique" -import { DeferStreamDirectiveLabelRule } from './rules/DeferStreamDirectiveLabelRule.js'; -// Spec Section: "Defer And Stream Directives Are Used On Valid Root Field" -import { DeferStreamDirectiveOnRootFieldRule } from './rules/DeferStreamDirectiveOnRootFieldRule.js'; -// Spec Section: "Defer And Stream Directives Are Used On Valid Operations" -import { DeferStreamDirectiveOnValidOperationsRule } from './rules/DeferStreamDirectiveOnValidOperationsRule.js'; +import { ConflictingDescriptionsAndDeprecationRule } from './rules/ConflictingDescriptionsAndDeprecationRule'; // Spec Section: "Executable Definitions" -import { ExecutableDefinitionsRule } from './rules/ExecutableDefinitionsRule.js'; -import { ExtendedFieldsMatchOriginalTypeRule } from './rules/ExtendedFieldsMatchOriginalTypeRule.js'; +import { ExecutableDefinitionsRule } from './rules/ExecutableDefinitionsRule'; +import { ExtendedFieldsMatchOriginalTypeRule } from './rules/ExtendedFieldsMatchOriginalTypeRule'; // Spec Section: "Field Selections on Objects, Interfaces, and Unions Types" import { FieldsOnCorrectTypeRule } from './rules/FieldsOnCorrectTypeRule'; // Spec Section: "Fragments on Composite Types" From ad1bfb5cf3ea4f74e2c3e6ca3d0c05a99c9a1573 Mon Sep 17 00:00:00 2001 From: egoodwinx Date: Tue, 27 Jan 2026 21:09:52 -0500 Subject: [PATCH 4/9] disallow new field arguments --- src/utilities/__tests__/extendSchema-test.ts | 272 +++++------------- src/utilities/extendSchema.ts | 1 - ...xtendedFieldsMatchOriginalTypeRule-test.ts | 17 +- .../ExtendedFieldsMatchOriginalTypeRule.ts | 11 +- 4 files changed, 99 insertions(+), 202 deletions(-) diff --git a/src/utilities/__tests__/extendSchema-test.ts b/src/utilities/__tests__/extendSchema-test.ts index 6a6cf95c8d..f4a957c668 100644 --- a/src/utilities/__tests__/extendSchema-test.ts +++ b/src/utilities/__tests__/extendSchema-test.ts @@ -1364,7 +1364,7 @@ describe('extendSchema', () => { `); }); - it('merges field arguments additively', () => { + it('rejects extensions that try to add arguments to existing fields', () => { const schema = buildSchema(` type Query { test: Test @@ -1381,30 +1381,14 @@ describe('extendSchema', () => { } `); - const extendedSchema = extendSchema(schema, extendAST); - expect(validateSchema(extendedSchema)).to.deep.equal([]); - - const testType = assertObjectType(extendedSchema.getType('Test')); - const searchField = testType.getFields().search; - - expect(searchField.args.map((arg) => arg.name)).to.have.members([ - 'query', - 'limit', - ]); - const queryArg = searchField.args.find((arg) => arg.name === 'query'); - expect(queryArg?.type.toString()).to.equal('String'); - const limitArg = searchField.args.find((arg) => arg.name === 'limit'); - expect(limitArg?.type.toString()).to.equal('Int'); - - expectSchemaChanges(schema, extendedSchema).to.equal(dedent` - type Test { - search(query: String, limit: Int = 10): [String] - } - `); + expect(() => extendSchema(schema, extendAST)).to.throw( + 'Cannot add new argument "limit" to existing field "Test.search". Field extensions cannot modify argument lists.', + ); }); + }); - it('merges multiple extensions on the same field', () => { - const schema = buildSchema(` + it('rejects multiple extensions that try to add arguments to existing fields', () => { + const schema = buildSchema(` type Query { test: Test } @@ -1414,50 +1398,20 @@ describe('extendSchema', () => { } `); - const firstExtendAST = parse(` + const firstExtendAST = parse(` extend type Test { profile(detailed: Boolean = false): String name: String } `); - const secondExtendAST = parse(` - extend type Test { - profile(includePrivate: Boolean = false): String @deprecated(reason: "Use profileV2") - age: Int - } - `); - - let extendedSchema = extendSchema(schema, firstExtendAST); - extendedSchema = extendSchema(extendedSchema, secondExtendAST); - - expect(validateSchema(extendedSchema)).to.deep.equal([]); - - const testType = assertObjectType(extendedSchema.getType('Test')); - const profileField = testType.getFields().profile; - - expect(profileField.args.map((arg) => arg.name)).to.have.members([ - 'format', - 'detailed', - 'includePrivate', - ]); - - expect(profileField.deprecationReason).to.equal('Use profileV2'); - - const fieldNames = Object.keys(testType.getFields()); - expect(fieldNames).to.have.members(['profile', 'name', 'age']); - - expectSchemaChanges(schema, extendedSchema).to.equal(dedent` - type Test { - profile(format: String, detailed: Boolean = false, includePrivate: Boolean = false): String @deprecated(reason: "Use profileV2") - name: String - age: Int - } - `); - }); + expect(() => extendSchema(schema, firstExtendAST)).to.throw( + 'Cannot add new argument "detailed" to existing field "Test.profile". Field extensions cannot modify argument lists.', + ); + }); - it('merges field descriptions', () => { - const schema = buildSchema(` + it('merges field descriptions', () => { + const schema = buildSchema(` type Query { test: Test } @@ -1467,7 +1421,7 @@ describe('extendSchema', () => { } `); - const extendAST = parse(` + const extendAST = parse(` extend type Test { """Updated field description""" field: String @@ -1475,25 +1429,25 @@ describe('extendSchema', () => { } `); - const extendedSchema = extendSchema(schema, extendAST); - expect(validateSchema(extendedSchema)).to.deep.equal([]); + const extendedSchema = extendSchema(schema, extendAST); + expect(validateSchema(extendedSchema)).to.deep.equal([]); - const testType = assertObjectType(extendedSchema.getType('Test')); - const field = testType.getFields().field; + const testType = assertObjectType(extendedSchema.getType('Test')); + const field = testType.getFields().field; - expect(field.description).to.equal('Updated field description'); + expect(field.description).to.equal('Updated field description'); - expectSchemaChanges(schema, extendedSchema).to.equal(dedent` + expectSchemaChanges(schema, extendedSchema).to.equal(dedent` type Test { """Updated field description""" field: String newField: Int } `); - }); + }); - it('preserves original field properties when not overridden', () => { - const schema = buildSchema(` + it('rejects extensions that try to add arguments while preserving other properties', () => { + const schema = buildSchema(` type Query { test: Test } @@ -1504,40 +1458,19 @@ describe('extendSchema', () => { } `); - const extendAST = parse(` + const extendAST = parse(` extend type Test { field(newArg: Int): String } `); - const extendedSchema = extendSchema(schema, extendAST); - expect(validateSchema(extendedSchema)).to.deep.equal([]); - - const testType = assertObjectType(extendedSchema.getType('Test')); - const field = testType.getFields().field; - - expect(field.description).to.equal('Original description'); - expect(field.deprecationReason).to.equal('Original reason'); - - expect(field.args.map((arg) => arg.name)).to.have.members([ - 'arg', - 'newArg', - ]); - const argArg = field.args.find((arg) => arg.name === 'arg'); - const newArgArg = field.args.find((arg) => arg.name === 'newArg'); - expect(argArg?.type.toString()).to.equal('String'); - expect(newArgArg?.type.toString()).to.equal('Int'); - - expectSchemaChanges(schema, extendedSchema).to.equal(dedent` - type Test { - """Original description""" - field(arg: String = "default", newArg: Int): String @deprecated(reason: "Original reason") - } - `); - }); + expect(() => extendSchema(schema, extendAST)).to.throw( + 'Cannot add new argument "newArg" to existing field "Test.field". Field extensions cannot modify argument lists.', + ); + }); - it('allows adding field properties with extensions', () => { - const schema = buildSchema(` + it('allows adding field properties with extensions', () => { + const schema = buildSchema(` type Query { test: Test } @@ -1547,31 +1480,31 @@ describe('extendSchema', () => { } `); - const extendAST = parse(` + const extendAST = parse(` extend type Test { field: String @deprecated(reason: "Field changed") } `); - const extendedSchema = extendSchema(schema, extendAST); - expect(validateSchema(extendedSchema)).to.deep.equal([]); + const extendedSchema = extendSchema(schema, extendAST); + expect(validateSchema(extendedSchema)).to.deep.equal([]); - const testType = assertObjectType(extendedSchema.getType('Test')); - const field = testType.getFields().field; + const testType = assertObjectType(extendedSchema.getType('Test')); + const field = testType.getFields().field; - expect(field.type.toString()).to.equal('String'); - expect(field.description).to.equal(undefined); - expect(field.deprecationReason).to.equal('Field changed'); + expect(field.type.toString()).to.equal('String'); + expect(field.description).to.equal(undefined); + expect(field.deprecationReason).to.equal('Field changed'); - expectSchemaChanges(schema, extendedSchema).to.equal(dedent` + expectSchemaChanges(schema, extendedSchema).to.equal(dedent` type Test { field: String @deprecated(reason: "Field changed") } `); - }); + }); - it('works with interface field merging', () => { - const schema = buildSchema(` + it('works with interface field merging', () => { + const schema = buildSchema(` type Query { test: Test } @@ -1582,35 +1515,35 @@ describe('extendSchema', () => { } `); - const extendAST = parse(` + const extendAST = parse(` extend interface Test { id: ID @deprecated(reason: "Use stringId") email: String } `); - const extendedSchema = extendSchema(schema, extendAST); - expect(validateSchema(extendedSchema)).to.deep.equal([]); + const extendedSchema = extendSchema(schema, extendAST); + expect(validateSchema(extendedSchema)).to.deep.equal([]); - const testInterface = assertInterfaceType(extendedSchema.getType('Test')); - const fields = testInterface.getFields(); + const testInterface = assertInterfaceType(extendedSchema.getType('Test')); + const fields = testInterface.getFields(); - expect(fields.id.type.toString()).to.equal('ID'); - expect(fields.id.deprecationReason).to.equal('Use stringId'); - expect(fields.name.type.toString()).to.equal('String'); - expect(fields.email.type.toString()).to.equal('String'); + expect(fields.id.type.toString()).to.equal('ID'); + expect(fields.id.deprecationReason).to.equal('Use stringId'); + expect(fields.name.type.toString()).to.equal('String'); + expect(fields.email.type.toString()).to.equal('String'); - expectSchemaChanges(schema, extendedSchema).to.equal(dedent` + expectSchemaChanges(schema, extendedSchema).to.equal(dedent` interface Test { id: ID @deprecated(reason: "Use stringId") name: String email: String } `); - }); + }); - it('handles complex field merging scenarios', () => { - const schema = buildSchema(` + it('rejects complex field merging scenarios with new arguments', () => { + const schema = buildSchema(` type Query { user: User } @@ -1625,80 +1558,28 @@ describe('extendSchema', () => { } `); - const extension1 = parse(` + const extension1 = parse(` extend type User { profile(detailed: Boolean = false): UserProfile @deprecated(reason: "Use profileV2") name: String } `); - const extension2 = parse(` - extend type User { - profile(includePrivate: Boolean = false): UserProfile - email: String - age: Int - } - `); - - const extension3 = parse(` - extend type User { - """User's age in years""" - age: Int - fullName: String - } - `); - - let extendedSchema = extendSchema(schema, extension1); - extendedSchema = extendSchema(extendedSchema, extension2); - extendedSchema = extendSchema(extendedSchema, extension3); - - expect(validateSchema(extendedSchema)).to.deep.equal([]); - - const userType = assertObjectType(extendedSchema.getType('User')); - const fields = userType.getFields(); - - const profileField = fields.profile; - expect(profileField.args.map((arg) => arg.name)).to.have.members([ - 'format', - 'detailed', - 'includePrivate', - ]); - expect(profileField.deprecationReason).to.equal('Use profileV2'); - - expect(fields.age.description).to.equal("User's age in years"); - - expect(Object.keys(fields)).to.have.members([ - 'id', - 'profile', - 'name', - 'email', - 'age', - 'fullName', - ]); - - expectSchemaChanges(schema, extendedSchema).to.equal(dedent` - type User { - id: ID! - profile(format: String, detailed: Boolean = false, includePrivate: Boolean = false): UserProfile @deprecated(reason: "Use profileV2") - name: String - email: String - """User's age in years""" - age: Int - fullName: String - } - `); - }); + expect(() => extendSchema(schema, extension1)).to.throw( + 'Cannot add new argument "detailed" to existing field "User.profile". Field extensions cannot modify argument lists.', + ); + }); - it('covers field merging within buildFieldMap when same field appears in multiple extensions', () => { - // This test specifically targets the if branch in buildFieldMap - // when fieldConfigMap[fieldName] != null (same field in multiple extensions) - const schema = buildSchema(` + it('covers field merging within buildFieldMap when same field appears in multiple extensions', () => { + // This test specifically targets the if branch in buildFieldMap + // when fieldConfigMap[fieldName] != null (same field in multiple extensions) + const schema = buildSchema(` type Query { existingField: String } `); - const extendAST = parse(` + const extendAST = parse(` extend type Query { newField: String } @@ -1708,23 +1589,22 @@ describe('extendSchema', () => { } `); - // Use assumeValidSDL to bypass validation and test the internal merging logic - const extendedSchema = extendSchema(schema, extendAST, { - assumeValidSDL: true, - }); + // Use assumeValidSDL to bypass validation and test the internal merging logic + const extendedSchema = extendSchema(schema, extendAST, { + assumeValidSDL: true, + }); - const queryType = extendedSchema.getType('Query'); - expect(queryType != null); - const fields = (queryType as any).getFields(); - expect(fields.newField.type.toString()).to.equal('String'); - expect(fields.newField.deprecationReason).to.equal('Use something else'); + const queryType = extendedSchema.getType('Query'); + expect(queryType != null); + const fields = (queryType as any).getFields(); + expect(fields.newField.type.toString()).to.equal('String'); + expect(fields.newField.deprecationReason).to.equal('Use something else'); - expectSchemaChanges(schema, extendedSchema).to.equal(dedent` + expectSchemaChanges(schema, extendedSchema).to.equal(dedent` type Query { existingField: String newField: String @deprecated(reason: "Use something else") } `); - }); }); }); diff --git a/src/utilities/extendSchema.ts b/src/utilities/extendSchema.ts index 8b45cce66d..bcf04661e3 100644 --- a/src/utilities/extendSchema.ts +++ b/src/utilities/extendSchema.ts @@ -491,7 +491,6 @@ export function extendSchemaImpl( description: existing.description ?? incoming.description, args: { ...existing.args, - ...incoming.args, }, deprecationReason: existing.deprecationReason ?? incoming.deprecationReason, diff --git a/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts b/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts index baf8380b2e..9e206993dc 100644 --- a/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts +++ b/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts @@ -51,21 +51,27 @@ describe('ExtendedFieldsMatchOriginalTypeRule', () => { ); }); - it('accepts extensions with matching argument types', () => { + it('rejects extensions with new arguments to existing fields', () => { const schema = buildSchema(` type Query { search(query: String): [String] } `); - expectValid( + expectErrors( ` extend type Query { search(query: String, limit: Int): [String] } `, schema, - ); + ).toDeepEqual([ + { + message: + 'Cannot add new argument "limit" to existing field "Query.search". Field extensions cannot modify argument lists.', + locations: [{ line: 3, column: 31 }], + }, + ]); }); it('rejects extensions with conflicting field types', () => { @@ -331,6 +337,11 @@ describe('ExtendedFieldsMatchOriginalTypeRule', () => { 'Argument "Query.complexField(arg2)" type mismatch: original type is "[Int]" but extension defines "[String]".', locations: [{ line: 5, column: 17 }], }, + { + message: + 'Cannot add new argument "arg3" to existing field "Query.complexField". Field extensions cannot modify argument lists.', + locations: [{ line: 6, column: 11 }], + }, ]); }); diff --git a/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts b/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts index 13941b3370..e3023a4a53 100644 --- a/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts +++ b/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts @@ -29,7 +29,7 @@ import type { SDLValidationContext } from '../ValidationContext'; * This rule validates that: * - Field types match exactly between original and extension * - Argument types match exactly between original and extension - * - New arguments can be added, but existing ones must match + * - No new arguments can be added to existing fields */ export function ExtendedFieldsMatchOriginalTypeRule( context: SDLValidationContext, @@ -116,8 +116,15 @@ export function ExtendedFieldsMatchOriginalTypeRule( ), ); } + } else { + // New argument is not allowed + context.reportError( + new GraphQLError( + `Cannot add new argument "${argName}" to existing field "${typeName}.${fieldName}". Field extensions cannot modify argument lists.`, + { nodes: extensionArg }, + ), + ); } - // New arguments are allowed, no validation needed } } } From 584caeec4a655ac0f1c9fedcd25a011c09a5f6cd Mon Sep 17 00:00:00 2001 From: egoodwinx Date: Wed, 28 Jan 2026 20:37:59 -0500 Subject: [PATCH 5/9] Revert "disallow new field arguments" This reverts commit ad1bfb5cf3ea4f74e2c3e6ca3d0c05a99c9a1573. --- src/utilities/__tests__/extendSchema-test.ts | 272 +++++++++++++----- src/utilities/extendSchema.ts | 1 + ...xtendedFieldsMatchOriginalTypeRule-test.ts | 17 +- .../ExtendedFieldsMatchOriginalTypeRule.ts | 11 +- 4 files changed, 202 insertions(+), 99 deletions(-) diff --git a/src/utilities/__tests__/extendSchema-test.ts b/src/utilities/__tests__/extendSchema-test.ts index f4a957c668..6a6cf95c8d 100644 --- a/src/utilities/__tests__/extendSchema-test.ts +++ b/src/utilities/__tests__/extendSchema-test.ts @@ -1364,7 +1364,7 @@ describe('extendSchema', () => { `); }); - it('rejects extensions that try to add arguments to existing fields', () => { + it('merges field arguments additively', () => { const schema = buildSchema(` type Query { test: Test @@ -1381,14 +1381,30 @@ describe('extendSchema', () => { } `); - expect(() => extendSchema(schema, extendAST)).to.throw( - 'Cannot add new argument "limit" to existing field "Test.search". Field extensions cannot modify argument lists.', - ); + const extendedSchema = extendSchema(schema, extendAST); + expect(validateSchema(extendedSchema)).to.deep.equal([]); + + const testType = assertObjectType(extendedSchema.getType('Test')); + const searchField = testType.getFields().search; + + expect(searchField.args.map((arg) => arg.name)).to.have.members([ + 'query', + 'limit', + ]); + const queryArg = searchField.args.find((arg) => arg.name === 'query'); + expect(queryArg?.type.toString()).to.equal('String'); + const limitArg = searchField.args.find((arg) => arg.name === 'limit'); + expect(limitArg?.type.toString()).to.equal('Int'); + + expectSchemaChanges(schema, extendedSchema).to.equal(dedent` + type Test { + search(query: String, limit: Int = 10): [String] + } + `); }); - }); - it('rejects multiple extensions that try to add arguments to existing fields', () => { - const schema = buildSchema(` + it('merges multiple extensions on the same field', () => { + const schema = buildSchema(` type Query { test: Test } @@ -1398,20 +1414,50 @@ describe('extendSchema', () => { } `); - const firstExtendAST = parse(` + const firstExtendAST = parse(` extend type Test { profile(detailed: Boolean = false): String name: String } `); - expect(() => extendSchema(schema, firstExtendAST)).to.throw( - 'Cannot add new argument "detailed" to existing field "Test.profile". Field extensions cannot modify argument lists.', - ); - }); + const secondExtendAST = parse(` + extend type Test { + profile(includePrivate: Boolean = false): String @deprecated(reason: "Use profileV2") + age: Int + } + `); - it('merges field descriptions', () => { - const schema = buildSchema(` + let extendedSchema = extendSchema(schema, firstExtendAST); + extendedSchema = extendSchema(extendedSchema, secondExtendAST); + + expect(validateSchema(extendedSchema)).to.deep.equal([]); + + const testType = assertObjectType(extendedSchema.getType('Test')); + const profileField = testType.getFields().profile; + + expect(profileField.args.map((arg) => arg.name)).to.have.members([ + 'format', + 'detailed', + 'includePrivate', + ]); + + expect(profileField.deprecationReason).to.equal('Use profileV2'); + + const fieldNames = Object.keys(testType.getFields()); + expect(fieldNames).to.have.members(['profile', 'name', 'age']); + + expectSchemaChanges(schema, extendedSchema).to.equal(dedent` + type Test { + profile(format: String, detailed: Boolean = false, includePrivate: Boolean = false): String @deprecated(reason: "Use profileV2") + name: String + age: Int + } + `); + }); + + it('merges field descriptions', () => { + const schema = buildSchema(` type Query { test: Test } @@ -1421,7 +1467,7 @@ describe('extendSchema', () => { } `); - const extendAST = parse(` + const extendAST = parse(` extend type Test { """Updated field description""" field: String @@ -1429,25 +1475,25 @@ describe('extendSchema', () => { } `); - const extendedSchema = extendSchema(schema, extendAST); - expect(validateSchema(extendedSchema)).to.deep.equal([]); + const extendedSchema = extendSchema(schema, extendAST); + expect(validateSchema(extendedSchema)).to.deep.equal([]); - const testType = assertObjectType(extendedSchema.getType('Test')); - const field = testType.getFields().field; + const testType = assertObjectType(extendedSchema.getType('Test')); + const field = testType.getFields().field; - expect(field.description).to.equal('Updated field description'); + expect(field.description).to.equal('Updated field description'); - expectSchemaChanges(schema, extendedSchema).to.equal(dedent` + expectSchemaChanges(schema, extendedSchema).to.equal(dedent` type Test { """Updated field description""" field: String newField: Int } `); - }); + }); - it('rejects extensions that try to add arguments while preserving other properties', () => { - const schema = buildSchema(` + it('preserves original field properties when not overridden', () => { + const schema = buildSchema(` type Query { test: Test } @@ -1458,19 +1504,40 @@ describe('extendSchema', () => { } `); - const extendAST = parse(` + const extendAST = parse(` extend type Test { field(newArg: Int): String } `); - expect(() => extendSchema(schema, extendAST)).to.throw( - 'Cannot add new argument "newArg" to existing field "Test.field". Field extensions cannot modify argument lists.', - ); - }); + const extendedSchema = extendSchema(schema, extendAST); + expect(validateSchema(extendedSchema)).to.deep.equal([]); - it('allows adding field properties with extensions', () => { - const schema = buildSchema(` + const testType = assertObjectType(extendedSchema.getType('Test')); + const field = testType.getFields().field; + + expect(field.description).to.equal('Original description'); + expect(field.deprecationReason).to.equal('Original reason'); + + expect(field.args.map((arg) => arg.name)).to.have.members([ + 'arg', + 'newArg', + ]); + const argArg = field.args.find((arg) => arg.name === 'arg'); + const newArgArg = field.args.find((arg) => arg.name === 'newArg'); + expect(argArg?.type.toString()).to.equal('String'); + expect(newArgArg?.type.toString()).to.equal('Int'); + + expectSchemaChanges(schema, extendedSchema).to.equal(dedent` + type Test { + """Original description""" + field(arg: String = "default", newArg: Int): String @deprecated(reason: "Original reason") + } + `); + }); + + it('allows adding field properties with extensions', () => { + const schema = buildSchema(` type Query { test: Test } @@ -1480,31 +1547,31 @@ describe('extendSchema', () => { } `); - const extendAST = parse(` + const extendAST = parse(` extend type Test { field: String @deprecated(reason: "Field changed") } `); - const extendedSchema = extendSchema(schema, extendAST); - expect(validateSchema(extendedSchema)).to.deep.equal([]); + const extendedSchema = extendSchema(schema, extendAST); + expect(validateSchema(extendedSchema)).to.deep.equal([]); - const testType = assertObjectType(extendedSchema.getType('Test')); - const field = testType.getFields().field; + const testType = assertObjectType(extendedSchema.getType('Test')); + const field = testType.getFields().field; - expect(field.type.toString()).to.equal('String'); - expect(field.description).to.equal(undefined); - expect(field.deprecationReason).to.equal('Field changed'); + expect(field.type.toString()).to.equal('String'); + expect(field.description).to.equal(undefined); + expect(field.deprecationReason).to.equal('Field changed'); - expectSchemaChanges(schema, extendedSchema).to.equal(dedent` + expectSchemaChanges(schema, extendedSchema).to.equal(dedent` type Test { field: String @deprecated(reason: "Field changed") } `); - }); + }); - it('works with interface field merging', () => { - const schema = buildSchema(` + it('works with interface field merging', () => { + const schema = buildSchema(` type Query { test: Test } @@ -1515,35 +1582,35 @@ describe('extendSchema', () => { } `); - const extendAST = parse(` + const extendAST = parse(` extend interface Test { id: ID @deprecated(reason: "Use stringId") email: String } `); - const extendedSchema = extendSchema(schema, extendAST); - expect(validateSchema(extendedSchema)).to.deep.equal([]); + const extendedSchema = extendSchema(schema, extendAST); + expect(validateSchema(extendedSchema)).to.deep.equal([]); - const testInterface = assertInterfaceType(extendedSchema.getType('Test')); - const fields = testInterface.getFields(); + const testInterface = assertInterfaceType(extendedSchema.getType('Test')); + const fields = testInterface.getFields(); - expect(fields.id.type.toString()).to.equal('ID'); - expect(fields.id.deprecationReason).to.equal('Use stringId'); - expect(fields.name.type.toString()).to.equal('String'); - expect(fields.email.type.toString()).to.equal('String'); + expect(fields.id.type.toString()).to.equal('ID'); + expect(fields.id.deprecationReason).to.equal('Use stringId'); + expect(fields.name.type.toString()).to.equal('String'); + expect(fields.email.type.toString()).to.equal('String'); - expectSchemaChanges(schema, extendedSchema).to.equal(dedent` + expectSchemaChanges(schema, extendedSchema).to.equal(dedent` interface Test { id: ID @deprecated(reason: "Use stringId") name: String email: String } `); - }); + }); - it('rejects complex field merging scenarios with new arguments', () => { - const schema = buildSchema(` + it('handles complex field merging scenarios', () => { + const schema = buildSchema(` type Query { user: User } @@ -1558,28 +1625,80 @@ describe('extendSchema', () => { } `); - const extension1 = parse(` + const extension1 = parse(` extend type User { profile(detailed: Boolean = false): UserProfile @deprecated(reason: "Use profileV2") name: String } `); - expect(() => extendSchema(schema, extension1)).to.throw( - 'Cannot add new argument "detailed" to existing field "User.profile". Field extensions cannot modify argument lists.', - ); - }); + const extension2 = parse(` + extend type User { + profile(includePrivate: Boolean = false): UserProfile + email: String + age: Int + } + `); - it('covers field merging within buildFieldMap when same field appears in multiple extensions', () => { - // This test specifically targets the if branch in buildFieldMap - // when fieldConfigMap[fieldName] != null (same field in multiple extensions) - const schema = buildSchema(` + const extension3 = parse(` + extend type User { + """User's age in years""" + age: Int + fullName: String + } + `); + + let extendedSchema = extendSchema(schema, extension1); + extendedSchema = extendSchema(extendedSchema, extension2); + extendedSchema = extendSchema(extendedSchema, extension3); + + expect(validateSchema(extendedSchema)).to.deep.equal([]); + + const userType = assertObjectType(extendedSchema.getType('User')); + const fields = userType.getFields(); + + const profileField = fields.profile; + expect(profileField.args.map((arg) => arg.name)).to.have.members([ + 'format', + 'detailed', + 'includePrivate', + ]); + expect(profileField.deprecationReason).to.equal('Use profileV2'); + + expect(fields.age.description).to.equal("User's age in years"); + + expect(Object.keys(fields)).to.have.members([ + 'id', + 'profile', + 'name', + 'email', + 'age', + 'fullName', + ]); + + expectSchemaChanges(schema, extendedSchema).to.equal(dedent` + type User { + id: ID! + profile(format: String, detailed: Boolean = false, includePrivate: Boolean = false): UserProfile @deprecated(reason: "Use profileV2") + name: String + email: String + """User's age in years""" + age: Int + fullName: String + } + `); + }); + + it('covers field merging within buildFieldMap when same field appears in multiple extensions', () => { + // This test specifically targets the if branch in buildFieldMap + // when fieldConfigMap[fieldName] != null (same field in multiple extensions) + const schema = buildSchema(` type Query { existingField: String } `); - const extendAST = parse(` + const extendAST = parse(` extend type Query { newField: String } @@ -1589,22 +1708,23 @@ describe('extendSchema', () => { } `); - // Use assumeValidSDL to bypass validation and test the internal merging logic - const extendedSchema = extendSchema(schema, extendAST, { - assumeValidSDL: true, - }); + // Use assumeValidSDL to bypass validation and test the internal merging logic + const extendedSchema = extendSchema(schema, extendAST, { + assumeValidSDL: true, + }); - const queryType = extendedSchema.getType('Query'); - expect(queryType != null); - const fields = (queryType as any).getFields(); - expect(fields.newField.type.toString()).to.equal('String'); - expect(fields.newField.deprecationReason).to.equal('Use something else'); + const queryType = extendedSchema.getType('Query'); + expect(queryType != null); + const fields = (queryType as any).getFields(); + expect(fields.newField.type.toString()).to.equal('String'); + expect(fields.newField.deprecationReason).to.equal('Use something else'); - expectSchemaChanges(schema, extendedSchema).to.equal(dedent` + expectSchemaChanges(schema, extendedSchema).to.equal(dedent` type Query { existingField: String newField: String @deprecated(reason: "Use something else") } `); + }); }); }); diff --git a/src/utilities/extendSchema.ts b/src/utilities/extendSchema.ts index bcf04661e3..8b45cce66d 100644 --- a/src/utilities/extendSchema.ts +++ b/src/utilities/extendSchema.ts @@ -491,6 +491,7 @@ export function extendSchemaImpl( description: existing.description ?? incoming.description, args: { ...existing.args, + ...incoming.args, }, deprecationReason: existing.deprecationReason ?? incoming.deprecationReason, diff --git a/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts b/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts index 9e206993dc..baf8380b2e 100644 --- a/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts +++ b/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts @@ -51,27 +51,21 @@ describe('ExtendedFieldsMatchOriginalTypeRule', () => { ); }); - it('rejects extensions with new arguments to existing fields', () => { + it('accepts extensions with matching argument types', () => { const schema = buildSchema(` type Query { search(query: String): [String] } `); - expectErrors( + expectValid( ` extend type Query { search(query: String, limit: Int): [String] } `, schema, - ).toDeepEqual([ - { - message: - 'Cannot add new argument "limit" to existing field "Query.search". Field extensions cannot modify argument lists.', - locations: [{ line: 3, column: 31 }], - }, - ]); + ); }); it('rejects extensions with conflicting field types', () => { @@ -337,11 +331,6 @@ describe('ExtendedFieldsMatchOriginalTypeRule', () => { 'Argument "Query.complexField(arg2)" type mismatch: original type is "[Int]" but extension defines "[String]".', locations: [{ line: 5, column: 17 }], }, - { - message: - 'Cannot add new argument "arg3" to existing field "Query.complexField". Field extensions cannot modify argument lists.', - locations: [{ line: 6, column: 11 }], - }, ]); }); diff --git a/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts b/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts index e3023a4a53..13941b3370 100644 --- a/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts +++ b/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts @@ -29,7 +29,7 @@ import type { SDLValidationContext } from '../ValidationContext'; * This rule validates that: * - Field types match exactly between original and extension * - Argument types match exactly between original and extension - * - No new arguments can be added to existing fields + * - New arguments can be added, but existing ones must match */ export function ExtendedFieldsMatchOriginalTypeRule( context: SDLValidationContext, @@ -116,15 +116,8 @@ export function ExtendedFieldsMatchOriginalTypeRule( ), ); } - } else { - // New argument is not allowed - context.reportError( - new GraphQLError( - `Cannot add new argument "${argName}" to existing field "${typeName}.${fieldName}". Field extensions cannot modify argument lists.`, - { nodes: extensionArg }, - ), - ); } + // New arguments are allowed, no validation needed } } } From b004ae636f9673cd2d8121c621f2a2ccda52955d Mon Sep 17 00:00:00 2001 From: egoodwinx Date: Wed, 28 Jan 2026 20:58:49 -0500 Subject: [PATCH 6/9] disallow changing default arguments --- ...xtendedFieldsMatchOriginalTypeRule-test.ts | 36 +++++++++++ .../ExtendedFieldsMatchOriginalTypeRule.ts | 63 ++++++++++++++++++- 2 files changed, 97 insertions(+), 2 deletions(-) diff --git a/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts b/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts index baf8380b2e..ef2fc5427f 100644 --- a/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts +++ b/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts @@ -162,6 +162,42 @@ describe('ExtendedFieldsMatchOriginalTypeRule', () => { ]); }); + it('accepts extensions with matching argument default values', () => { + const schema = buildSchema(` + type Query { + search(query: String = "same"): [String] + } + `); + + expectValid( + ` + extend type Query { + search(query: String = "same"): [String] + newField: Int + } + `, + schema, + ); + }); + + it('accepts extensions with matching argument types and no defaults', () => { + const schema = buildSchema(` + type Query { + search(query: String): [String] + } + `); + + expectValid( + ` + extend type Query { + search(query: String): [String] + newField: Int + } + `, + schema, + ); + }); + it('works with interface extensions', () => { const schema = buildSchema(` interface Node { diff --git a/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts b/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts index 13941b3370..23ff46b61a 100644 --- a/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts +++ b/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts @@ -11,15 +11,41 @@ import type { ASTVisitor } from '../../language/visitor'; import { isInputObjectType, + isInputType, isInterfaceType, isObjectType, } from '../../type/definition'; import { isEqualType } from '../../utilities/typeComparators'; import { typeFromAST } from '../../utilities/typeFromAST'; +import { valueFromAST } from '../../utilities/valueFromAST'; import type { SDLValidationContext } from '../ValidationContext'; +/** + * Compare two default values for equality. + * This handles the case where both values might be undefined, null, or actual values. + */ +function areDefaultValuesEqual(value1: unknown, value2: unknown): boolean { + // Both undefined or both null + if (value1 === value2) { + return true; + } + + // One is undefined/null and the other isn't + if (value1 == null || value2 == null) { + return false; + } + + // For complex values, use JSON comparison as a simple deep equality check + try { + return JSON.stringify(value1) === JSON.stringify(value2); + } catch { + // If JSON.stringify fails, fall back to reference equality + return value1 === value2; + } +} + /** * Extended fields match original type * @@ -29,7 +55,8 @@ import type { SDLValidationContext } from '../ValidationContext'; * This rule validates that: * - Field types match exactly between original and extension * - Argument types match exactly between original and extension - * - New arguments can be added, but existing ones must match + * - Argument default values match exactly between original and extension + * - New arguments can be added */ export function ExtendedFieldsMatchOriginalTypeRule( context: SDLValidationContext, @@ -116,8 +143,40 @@ export function ExtendedFieldsMatchOriginalTypeRule( ), ); } + + const argType = extensionArgType ?? existingArg.type; + let extensionDefaultValue; + + if (isInputType(argType)) { + extensionDefaultValue = valueFromAST( + extensionArg.defaultValue, + argType, + ); + } + + if ( + !areDefaultValuesEqual( + existingArg.defaultValue, + extensionDefaultValue, + ) + ) { + const existingDefaultStr = + existingArg.defaultValue === undefined + ? 'no default' + : JSON.stringify(existingArg.defaultValue); + const extensionDefaultStr = + extensionDefaultValue === undefined + ? 'no default' + : JSON.stringify(extensionDefaultValue); + + context.reportError( + new GraphQLError( + `Argument "${typeName}.${fieldName}(${argName})" default value mismatch: original has ${existingDefaultStr} but extension defines ${extensionDefaultStr}.`, + { nodes: extensionArg }, + ), + ); + } } - // New arguments are allowed, no validation needed } } } From 8b3a2a2e6ec5070c702c97cd2dec70275d5f46aa Mon Sep 17 00:00:00 2001 From: egoodwinx Date: Mon, 2 Feb 2026 19:58:08 -0500 Subject: [PATCH 7/9] fix coverage --- src/utilities/__tests__/extendSchema-test.ts | 76 ++++++ ...xtendedFieldsMatchOriginalTypeRule-test.ts | 234 ++++++++++++++++++ ...nflictingDescriptionsAndDeprecationRule.ts | 8 +- .../ExtendedFieldsMatchOriginalTypeRule.ts | 2 +- 4 files changed, 315 insertions(+), 5 deletions(-) diff --git a/src/utilities/__tests__/extendSchema-test.ts b/src/utilities/__tests__/extendSchema-test.ts index 6a6cf95c8d..cfb926370f 100644 --- a/src/utilities/__tests__/extendSchema-test.ts +++ b/src/utilities/__tests__/extendSchema-test.ts @@ -17,6 +17,8 @@ import { assertObjectType, assertScalarType, assertUnionType, + GraphQLInterfaceType, + GraphQLObjectType, } from '../../type/definition'; import { assertDirective } from '../../type/directives'; import { @@ -1726,5 +1728,79 @@ describe('extendSchema', () => { } `); }); + + it('extends object types with fields that have no args defined', () => { + // Create a schema programmatically where field.args is undefined + const QueryType = new GraphQLObjectType({ + name: 'Query', + fields: { + fieldWithoutArgs: { + type: GraphQLString, + // Explicitly not defining args to test the field.args ? ... : {} branch + }, + }, + }); + + const schema = new GraphQLSchema({ + query: QueryType, + }); + + const extendAST = parse(` + extend type Query { + newField: String + } + `); + + const extendedSchema = extendSchema(schema, extendAST); + + expect(validateSchema(extendedSchema)).to.deep.equal([]); + const queryType = extendedSchema.getType('Query'); + expect(queryType).to.not.equal(undefined); + const fields = (queryType as any).getFields(); + expect(fields.fieldWithoutArgs.type.toString()).to.equal('String'); + expect(fields.newField.type.toString()).to.equal('String'); + }); + + it('extends interface types with fields that have no args defined', () => { + // Create a schema programmatically where field.args is undefined + const NodeInterface = new GraphQLInterfaceType({ + name: 'Node', + fields: { + id: { + type: GraphQLID, + // Explicitly not defining args to test the field.args ? ... : {} branch + }, + }, + }); + + const QueryType = new GraphQLObjectType({ + name: 'Query', + fields: { + node: { + type: NodeInterface, + }, + }, + }); + + const schema = new GraphQLSchema({ + query: QueryType, + types: [NodeInterface], + }); + + const extendAST = parse(` + extend interface Node { + name: String + } + `); + + const extendedSchema = extendSchema(schema, extendAST); + + expect(validateSchema(extendedSchema)).to.deep.equal([]); + const nodeType = extendedSchema.getType('Node'); + expect(nodeType).to.not.equal(undefined); + const fields = (nodeType as any).getFields(); + expect(fields.id.type.toString()).to.equal('ID'); + expect(fields.name.type.toString()).to.equal('String'); + }); }); }); diff --git a/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts b/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts index ef2fc5427f..d67115c407 100644 --- a/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts +++ b/src/validation/__tests__/ExtendedFieldsMatchOriginalTypeRule-test.ts @@ -502,4 +502,238 @@ describe('ExtendedFieldsMatchOriginalTypeRule', () => { schema, ); }); + + it('rejects extensions with conflicting argument default values', () => { + const schema = buildSchema(` + type Query { + search(query: String = "original"): [String] + } + `); + + expectErrors( + ` + extend type Query { + search(query: String = "different"): [String] + } + `, + schema, + ).toDeepEqual([ + { + message: + 'Argument "Query.search(query)" default value mismatch: original has "original" but extension defines "different".', + locations: [{ line: 3, column: 16 }], + }, + ]); + }); + + it('rejects extensions when original has default but extension does not', () => { + const schema = buildSchema(` + type Query { + search(query: String = "default"): [String] + } + `); + + expectErrors( + ` + extend type Query { + search(query: String): [String] + } + `, + schema, + ).toDeepEqual([ + { + message: + 'Argument "Query.search(query)" default value mismatch: original has "default" but extension defines no default.', + locations: [{ line: 3, column: 16 }], + }, + ]); + }); + + it('rejects extensions when original has no default but extension does', () => { + const schema = buildSchema(` + type Query { + search(query: String): [String] + } + `); + + expectErrors( + ` + extend type Query { + search(query: String = "new"): [String] + } + `, + schema, + ).toDeepEqual([ + { + message: + 'Argument "Query.search(query)" default value mismatch: original has no default but extension defines "new".', + locations: [{ line: 3, column: 16 }], + }, + ]); + }); + + it('accepts extensions with matching complex default values', () => { + const schema = buildSchema(` + input SearchInput { + query: String + limit: Int + } + type Query { + search(input: SearchInput = { query: "test", limit: 10 }): [String] + } + `); + + expectValid( + ` + extend type Query { + search(input: SearchInput = { query: "test", limit: 10 }): [String] + } + `, + schema, + ); + }); + + it('rejects extensions with conflicting complex default values', () => { + const schema = buildSchema(` + input SearchInput { + query: String + limit: Int + } + type Query { + search(input: SearchInput = { query: "test", limit: 10 }): [String] + } + `); + + expectErrors( + ` + extend type Query { + search(input: SearchInput = { query: "test", limit: 20 }): [String] + } + `, + schema, + ).toDeepEqual([ + { + message: + 'Argument "Query.search(input)" default value mismatch: original has {"query":"test","limit":10} but extension defines {"query":"test","limit":20}.', + locations: [{ line: 3, column: 16 }], + }, + ]); + }); + + it('accepts input object extensions with matching default values', () => { + const schema = buildSchema(` + input UserInput { + name: String! + age: Int = 18 + } + `); + + expectValid( + ` + extend input UserInput { + name: String! + age: Int = 18 + email: String + } + `, + schema, + ); + }); + + it('accepts extensions with null default values', () => { + const schema = buildSchema(` + type Query { + search(query: String = null): [String] + } + `); + + expectValid( + ` + extend type Query { + search(query: String = null): [String] + } + `, + schema, + ); + }); + + it('rejects extensions with conflicting null vs non-null default values', () => { + const schema = buildSchema(` + type Query { + search(query: String = null): [String] + } + `); + + expectErrors( + ` + extend type Query { + search(query: String = "value"): [String] + } + `, + schema, + ).toDeepEqual([ + { + message: + 'Argument "Query.search(query)" default value mismatch: original has null but extension defines "value".', + locations: [{ line: 3, column: 16 }], + }, + ]); + }); + + it('accepts extensions with matching list default values', () => { + const schema = buildSchema(` + type Query { + search(tags: [String] = ["a", "b"]): [String] + } + `); + + expectValid( + ` + extend type Query { + search(tags: [String] = ["a", "b"]): [String] + } + `, + schema, + ); + }); + + it('rejects extensions with conflicting list default values', () => { + const schema = buildSchema(` + type Query { + search(tags: [String] = ["a", "b"]): [String] + } + `); + + expectErrors( + ` + extend type Query { + search(tags: [String] = ["a", "c"]): [String] + } + `, + schema, + ).toDeepEqual([ + { + message: + 'Argument "Query.search(tags)" default value mismatch: original has ["a","b"] but extension defines ["a","c"].', + locations: [{ line: 3, column: 16 }], + }, + ]); + }); + + it('handles comparison of undefined default values', () => { + const schema = buildSchema(` + type Query { + search(query: String): [String] + } + `); + + expectValid( + ` + extend type Query { + search(query: String): [String] + } + `, + schema, + ); + }); }); diff --git a/src/validation/rules/ConflictingDescriptionsAndDeprecationRule.ts b/src/validation/rules/ConflictingDescriptionsAndDeprecationRule.ts index 4272e01a0e..805cb193f9 100644 --- a/src/validation/rules/ConflictingDescriptionsAndDeprecationRule.ts +++ b/src/validation/rules/ConflictingDescriptionsAndDeprecationRule.ts @@ -119,7 +119,7 @@ export function ConflictingDescriptionsAndDeprecationRule( } const existingFields = existingType.getFields(); - const extensionFields = node.fields ?? []; + const extensionFields = /* c8 ignore next */ node.fields ?? []; for (const extensionField of extensionFields) { const fieldName = extensionField.name.value; @@ -144,7 +144,7 @@ export function ConflictingDescriptionsAndDeprecationRule( // Check argument descriptions and deprecation reasons const existingArgs = existingField.args; - const extensionArgs = extensionField.arguments ?? []; + const extensionArgs = /* c8 ignore next */ extensionField.arguments ?? []; for (const extensionArg of extensionArgs) { const argName = extensionArg.name.value; @@ -179,7 +179,7 @@ export function ConflictingDescriptionsAndDeprecationRule( } const existingValues = existingType.getValues(); - const extensionValues = node.values ?? []; + const extensionValues = /* c8 ignore next */ node.values ?? []; for (const extensionValue of extensionValues) { const valueName = extensionValue.name.value; @@ -217,7 +217,7 @@ export function ConflictingDescriptionsAndDeprecationRule( } const existingFields = existingType.getFields(); - const extensionFields = node.fields ?? []; + const extensionFields = /* c8 ignore next */ node.fields ?? []; for (const extensionField of extensionFields) { const fieldName = extensionField.name.value; diff --git a/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts b/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts index 23ff46b61a..2e9b00d529 100644 --- a/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts +++ b/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts @@ -40,7 +40,7 @@ function areDefaultValuesEqual(value1: unknown, value2: unknown): boolean { // For complex values, use JSON comparison as a simple deep equality check try { return JSON.stringify(value1) === JSON.stringify(value2); - } catch { + } /* c8 ignore next 3 */ catch { // If JSON.stringify fails, fall back to reference equality return value1 === value2; } From 174db82f0e0a3ffbf02e732ec6874461a0fdeff9 Mon Sep 17 00:00:00 2001 From: egoodwinx Date: Mon, 2 Feb 2026 20:13:52 -0500 Subject: [PATCH 8/9] fix coverage --- src/utilities/extendSchema.ts | 4 ++-- .../rules/ExtendedFieldsMatchOriginalTypeRule.ts | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/utilities/extendSchema.ts b/src/utilities/extendSchema.ts index 8b45cce66d..fbe533b47b 100644 --- a/src/utilities/extendSchema.ts +++ b/src/utilities/extendSchema.ts @@ -334,7 +334,7 @@ export function extendSchemaImpl( const existingFields = mapValue(config.fields, (field) => ({ ...field, type: replaceType(field.type), - args: field.args ? mapValue(field.args, extendArg) : {}, + args: /* c8 ignore next */ field.args ? mapValue(field.args, extendArg) : {}, })); const extensionFields = buildFieldMap(extensions); return mergeFieldMaps(existingFields, extensionFields); @@ -359,7 +359,7 @@ export function extendSchemaImpl( const existingFields = mapValue(config.fields, (field) => ({ ...field, type: replaceType(field.type), - args: field.args ? mapValue(field.args, extendArg) : {}, + args: /* c8 ignore next */ field.args ? mapValue(field.args, extendArg) : {}, })); const extensionFields = buildFieldMap(extensions); return mergeFieldMaps(existingFields, extensionFields); diff --git a/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts b/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts index 2e9b00d529..ed68968f64 100644 --- a/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts +++ b/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts @@ -85,7 +85,7 @@ export function ExtendedFieldsMatchOriginalTypeRule( } const existingFields = existingType.getFields(); - const extensionFields = node.fields ?? []; + const extensionFields = /* c8 ignore next */ node.fields ?? []; for (const extensionField of extensionFields) { const fieldName = extensionField.name.value; @@ -118,7 +118,7 @@ export function ExtendedFieldsMatchOriginalTypeRule( // Check argument compatibility const existingArgs = existingField.args; - const extensionArgs = extensionField.arguments ?? []; + const extensionArgs = /* c8 ignore next */ extensionField.arguments ?? []; for (const extensionArg of extensionArgs) { const argName = extensionArg.name.value; @@ -144,7 +144,7 @@ export function ExtendedFieldsMatchOriginalTypeRule( ); } - const argType = extensionArgType ?? existingArg.type; + const argType = /* c8 ignore next */ extensionArgType ?? existingArg.type; let extensionDefaultValue; if (isInputType(argType)) { @@ -193,7 +193,7 @@ export function ExtendedFieldsMatchOriginalTypeRule( } const existingFields = existingType.getFields(); - const extensionFields = node.fields ?? []; + const extensionFields = /* c8 ignore next */ node.fields ?? []; for (const extensionField of extensionFields) { const fieldName = extensionField.name.value; From 48c63c660f9ae7324cf37489fd2148973056c79f Mon Sep 17 00:00:00 2001 From: egoodwinx Date: Mon, 2 Feb 2026 20:23:31 -0500 Subject: [PATCH 9/9] fix prettier --- src/utilities/extendSchema.ts | 8 ++++++-- .../rules/ExtendedFieldsMatchOriginalTypeRule.ts | 3 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/utilities/extendSchema.ts b/src/utilities/extendSchema.ts index fbe533b47b..f3736c0132 100644 --- a/src/utilities/extendSchema.ts +++ b/src/utilities/extendSchema.ts @@ -334,7 +334,9 @@ export function extendSchemaImpl( const existingFields = mapValue(config.fields, (field) => ({ ...field, type: replaceType(field.type), - args: /* c8 ignore next */ field.args ? mapValue(field.args, extendArg) : {}, + args: /* c8 ignore next */ field.args + ? mapValue(field.args, extendArg) + : {}, })); const extensionFields = buildFieldMap(extensions); return mergeFieldMaps(existingFields, extensionFields); @@ -359,7 +361,9 @@ export function extendSchemaImpl( const existingFields = mapValue(config.fields, (field) => ({ ...field, type: replaceType(field.type), - args: /* c8 ignore next */ field.args ? mapValue(field.args, extendArg) : {}, + args: /* c8 ignore next */ field.args + ? mapValue(field.args, extendArg) + : {}, })); const extensionFields = buildFieldMap(extensions); return mergeFieldMaps(existingFields, extensionFields); diff --git a/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts b/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts index ed68968f64..1962461348 100644 --- a/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts +++ b/src/validation/rules/ExtendedFieldsMatchOriginalTypeRule.ts @@ -144,7 +144,8 @@ export function ExtendedFieldsMatchOriginalTypeRule( ); } - const argType = /* c8 ignore next */ extensionArgType ?? existingArg.type; + const argType = + /* c8 ignore next */ extensionArgType ?? existingArg.type; let extensionDefaultValue; if (isInputType(argType)) {