From c18b33cf48d4e75b74d14b4ecc3a7dd6eb421467 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 3 Jun 2025 17:07:55 +0300 Subject: [PATCH 1/3] move validation of names into validateSchema --- src/index.ts | 1 + src/language/__tests__/schema-parser-test.ts | 22 ++ src/type/__tests__/assertHasValidName-test.ts | 82 ++++ src/type/__tests__/definition-test.ts | 96 ----- src/type/__tests__/directive-test.ts | 23 -- src/type/__tests__/validation-test.ts | 355 ++++++++++++++---- src/type/assertHasValidName.ts | 86 +++++ src/type/definition.ts | 21 +- src/type/directives.ts | 3 +- src/type/index.ts | 1 + src/type/validate.ts | 47 +-- 11 files changed, 507 insertions(+), 230 deletions(-) create mode 100644 src/type/__tests__/assertHasValidName-test.ts create mode 100644 src/type/assertHasValidName.ts diff --git a/src/index.ts b/src/index.ts index 7bba9d8eee..d092a93699 100644 --- a/src/index.ts +++ b/src/index.ts @@ -151,6 +151,7 @@ export { validateSchema, assertValidSchema, // Upholds the spec rules about naming. + assertHasValidName, assertName, assertEnumValueName, } from './type/index.js'; diff --git a/src/language/__tests__/schema-parser-test.ts b/src/language/__tests__/schema-parser-test.ts index 3780c8330f..b893056315 100644 --- a/src/language/__tests__/schema-parser-test.ts +++ b/src/language/__tests__/schema-parser-test.ts @@ -109,6 +109,28 @@ describe('Schema Parser', () => { }); }); + it('parses type with reserved name', () => { + const doc = parse(dedent` + type __Reserved + `); + + expectJSON(doc).toDeepEqual({ + kind: 'Document', + definitions: [ + { + kind: 'ObjectTypeDefinition', + name: nameNode('__Reserved', { start: 5, end: 15 }), + description: undefined, + interfaces: undefined, + directives: undefined, + fields: undefined, + loc: { start: 0, end: 15 }, + }, + ], + loc: { start: 0, end: 15 }, + }); + }); + it('parses type with description string', () => { const doc = parse(dedent` "Description" diff --git a/src/type/__tests__/assertHasValidName-test.ts b/src/type/__tests__/assertHasValidName-test.ts new file mode 100644 index 0000000000..f1bdca11a4 --- /dev/null +++ b/src/type/__tests__/assertHasValidName-test.ts @@ -0,0 +1,82 @@ +import { expect } from 'chai'; +import { describe, it } from 'mocha'; + +import { assertHasValidName } from '../assertHasValidName.js'; +import type { GraphQLEnumValue } from '../index.js'; +import { __Type, GraphQLEnumType, GraphQLObjectType } from '../index.js'; + +describe(assertHasValidName.name, () => { + it('passthrough valid name', () => { + expect( + assertHasValidName( + new GraphQLObjectType({ + name: '_ValidName123', + fields: {}, + }), + ), + ).to.equal('_ValidName123'); + }); + + it('throws on empty strings', () => { + expect(() => + assertHasValidName( + new GraphQLObjectType({ + name: '', + fields: {}, + }), + ), + ).to.throw('Expected name of type "" to be a non-empty string.'); + }); + + it('throws for names with invalid characters', () => { + expect(() => + assertHasValidName( + new GraphQLObjectType({ + name: 'Some-Object', + fields: {}, + }), + ), + ).to.throw('Name of type "Some-Object" must only contain [_a-zA-Z0-9].'); + }); + + it('throws for names starting with invalid characters', () => { + expect(() => + assertHasValidName( + new GraphQLObjectType({ + name: '1_ObjectType', + fields: {}, + }), + ), + ).to.throw('Name of type "1_ObjectType" must start with [_a-zA-Z].'); + }); + + it('throws for reserved names', () => { + expect(() => + assertHasValidName( + new GraphQLObjectType({ + name: '__SomeObject', + fields: {}, + }), + ), + ).to.throw( + 'Name of type "__SomeObject" must not begin with "__", which is reserved by GraphQL introspection.', + ); + }); + + it('allows reserved names when specified', () => { + expect(assertHasValidName(__Type, true)).to.equal('__Type'); + }); + + it('throws for reserved names in enum values', () => { + const someEnum = new GraphQLEnumType({ + name: 'SomeEnum', + values: { + true: {}, + }, + }); + const value = someEnum.getValue('true') as GraphQLEnumValue; + expect(() => assertHasValidName(value)).to.throw( + 'Name "true" of enum value "SomeEnum.true" cannot be: true.', + ); + }); +}); diff --git a/src/type/__tests__/definition-test.ts b/src/type/__tests__/definition-test.ts index 8d9eeb4044..703499d2ab 100644 --- a/src/type/__tests__/definition-test.ts +++ b/src/type/__tests__/definition-test.ts @@ -414,52 +414,6 @@ describe('Type System: Objects', () => { }); expect(() => objType.getFields()).to.not.throw(); }); - - it('rejects an Object type with invalid name', () => { - expect( - () => new GraphQLObjectType({ name: 'bad-name', fields: {} }), - ).to.throw('Names must only contain [_a-zA-Z0-9] but "bad-name" does not.'); - }); - - it('rejects an Object type with incorrectly named fields', () => { - const objType = new GraphQLObjectType({ - name: 'SomeObject', - fields: { - 'bad-name': { type: ScalarType }, - }, - }); - expect(() => objType.getFields()).to.throw( - 'Names must only contain [_a-zA-Z0-9] but "bad-name" does not.', - ); - }); - - it('rejects an Object type with a field function that returns incorrect type', () => { - const objType = new GraphQLObjectType({ - name: 'SomeObject', - // @ts-expect-error (Wrong type of return) - fields() { - return [{ field: ScalarType }]; - }, - }); - expect(() => objType.getFields()).to.throw(); - }); - - it('rejects an Object type with incorrectly named field args', () => { - const objType = new GraphQLObjectType({ - name: 'SomeObject', - fields: { - badField: { - type: ScalarType, - args: { - 'bad-name': { type: ScalarType }, - }, - }, - }, - }); - expect(() => objType.getFields()).to.throw( - 'Names must only contain [_a-zA-Z0-9] but "bad-name" does not.', - ); - }); }); describe('Type System: Interfaces', () => { @@ -563,12 +517,6 @@ describe('Type System: Interfaces', () => { }); expect(implementing.getInterfaces()).to.deep.equal([InterfaceType]); }); - - it('rejects an Interface type with invalid name', () => { - expect( - () => new GraphQLInterfaceType({ name: 'bad-name', fields: {} }), - ).to.throw('Names must only contain [_a-zA-Z0-9] but "bad-name" does not.'); - }); }); describe('Type System: Unions', () => { @@ -635,12 +583,6 @@ describe('Type System: Unions', () => { }); expect(unionType.getTypes()).to.deep.equal([]); }); - - it('rejects an Union type with invalid name', () => { - expect( - () => new GraphQLUnionType({ name: 'bad-name', types: [] }), - ).to.throw('Names must only contain [_a-zA-Z0-9] but "bad-name" does not.'); - }); }); describe('Type System: Enums', () => { @@ -786,24 +728,6 @@ describe('Type System: Enums', () => { expect(enumType.getValue('FOO')).has.property('value', 10); expect(enumType.getValue('BAR')).has.property('value', 20); }); - - it('rejects an Enum type with invalid name', () => { - expect( - () => new GraphQLEnumType({ name: 'bad-name', values: {} }), - ).to.throw('Names must only contain [_a-zA-Z0-9] but "bad-name" does not.'); - }); - - it('rejects an Enum type with incorrectly named values', () => { - expect( - () => - new GraphQLEnumType({ - name: 'SomeEnum', - values: { - 'bad-name': {}, - }, - }), - ).to.throw('Names must only contain [_a-zA-Z0-9] but "bad-name" does not.'); - }); }); describe('Type System: Input Objects', () => { @@ -887,26 +811,6 @@ describe('Type System: Input Objects', () => { astNode: undefined, }); }); - - it('rejects an Input Object type with invalid name', () => { - expect( - () => new GraphQLInputObjectType({ name: 'bad-name', fields: {} }), - ).to.throw( - 'Names must only contain [_a-zA-Z0-9] but "bad-name" does not.', - ); - }); - - it('rejects an Input Object type with incorrectly named fields', () => { - const inputObjType = new GraphQLInputObjectType({ - name: 'SomeInputObject', - fields: { - 'bad-name': { type: ScalarType }, - }, - }); - expect(() => inputObjType.getFields()).to.throw( - 'Names must only contain [_a-zA-Z0-9] but "bad-name" does not.', - ); - }); }); it('Deprecation reason is preserved on fields', () => { diff --git a/src/type/__tests__/directive-test.ts b/src/type/__tests__/directive-test.ts index 90510bd0f9..fc4da8bd97 100644 --- a/src/type/__tests__/directive-test.ts +++ b/src/type/__tests__/directive-test.ts @@ -91,27 +91,4 @@ describe('Type System: Directive', () => { '[object GraphQLDirective]', ); }); - - it('rejects a directive with invalid name', () => { - expect( - () => - new GraphQLDirective({ - name: 'bad-name', - locations: [DirectiveLocation.QUERY], - }), - ).to.throw('Names must only contain [_a-zA-Z0-9] but "bad-name" does not.'); - }); - - it('rejects a directive with incorrectly named arg', () => { - expect( - () => - new GraphQLDirective({ - name: 'Foo', - locations: [DirectiveLocation.QUERY], - args: { - 'bad-name': { type: GraphQLString }, - }, - }), - ).to.throw('Names must only contain [_a-zA-Z0-9] but "bad-name" does not.'); - }); }); diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index c1ed85d7b6..939b2d3829 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -531,6 +531,284 @@ describe('Type System: Root types must all be different if provided', () => { }); }); +describe('Type System: Schema elements must be properly named', () => { + it('accepts types with valid names', () => { + const schema = buildSchema(` + directive @SomeDirective(someArg: String) on QUERY + + type Query { + f1: SomeObject + f2: SomeInterface + f3: SomeUnion + f4: SomeEnum + f5: SomeScalar + } + + input SomeInputObject { + someField: String + } + + type SomeObject { + someField(someArg: SomeInputObject): String + } + + interface SomeInterface { + someField: String + } + + type SomeUnionMember { + field: String + } + + union SomeUnion = SomeUnionMember + + enum SomeEnum { + SOME_ENUM_VALUE + } + + scalar SomeScalar + `); + + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('rejects types with invalid names', () => { + const schema = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'Query', + fields: { + f1: { + type: new GraphQLObjectType({ + name: 'Some-Object', + fields: { + 'some-Field': { + type: GraphQLString, + args: { + 'some-Arg': { + type: new GraphQLInputObjectType({ + name: 'Some-Input-Object', + fields: { + 'some-Field': { type: GraphQLString }, + }, + }), + }, + }, + }, + }, + }), + }, + f2: { + type: new GraphQLInterfaceType({ + name: 'Some-Interface', + fields: { 'some-Field': { type: GraphQLString } }, + }), + }, + f3: { + type: new GraphQLUnionType({ + name: 'Some-Union', + types: [ + new GraphQLObjectType({ + name: 'SomeUnionMember', + fields: { field: { type: GraphQLString } }, + }), + ], + }), + }, + f4: { + type: new GraphQLEnumType({ + name: 'Some-Enum', + values: { 'SOME-ENUM-VALUE': {}, true: {}, false: {}, null: {} }, + }), + }, + f5: { + type: new GraphQLScalarType({ + name: 'Some-Scalar', + }), + }, + }, + }), + directives: [ + new GraphQLDirective({ + name: 'Some-Directive', + args: { + 'some-Arg': { + type: GraphQLString, + }, + }, + locations: [DirectiveLocation.QUERY], + }), + ], + }); + + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Name of directive "@Some-Directive" must only contain [_a-zA-Z0-9].', + }, + { + message: + 'Name "some-Arg" of argument "@Some-Directive(some-Arg:)" must only contain [_a-zA-Z0-9].', + }, + { + message: 'Name of type "Some-Object" must only contain [_a-zA-Z0-9].', + }, + { + message: + 'Name "some-Field" of field "Some-Object.some-Field" must only contain [_a-zA-Z0-9].', + }, + { + message: + 'Name "some-Arg" of argument "Some-Object.some-Field(some-Arg:)" must only contain [_a-zA-Z0-9].', + }, + { + message: + 'Name of type "Some-Input-Object" must only contain [_a-zA-Z0-9].', + }, + { + message: + 'Name "some-Field" of input field "Some-Input-Object.some-Field" must only contain [_a-zA-Z0-9].', + }, + { + message: + 'Name of type "Some-Interface" must only contain [_a-zA-Z0-9].', + }, + { + message: + 'Name "some-Field" of field "Some-Interface.some-Field" must only contain [_a-zA-Z0-9].', + }, + { + message: 'Name of type "Some-Union" must only contain [_a-zA-Z0-9].', + }, + { + message: 'Name of type "Some-Enum" must only contain [_a-zA-Z0-9].', + }, + { + message: + 'Name "SOME-ENUM-VALUE" of enum value "Some-Enum.SOME-ENUM-VALUE" must only contain [_a-zA-Z0-9].', + }, + { + message: 'Name "true" of enum value "Some-Enum.true" cannot be: true.', + }, + { + message: + 'Name "false" of enum value "Some-Enum.false" cannot be: false.', + }, + { + message: 'Name "null" of enum value "Some-Enum.null" cannot be: null.', + }, + { + message: 'Name of type "Some-Scalar" must only contain [_a-zA-Z0-9].', + }, + ]); + }); + + it('rejects types with reserved names', () => { + const schema = buildSchema(` + directive @__SomeDirective(__someArg: String) on QUERY + + type Query { + f1: __SomeObject + f2: __SomeInterface + f3: __SomeUnion + f4: __SomeEnum + f5: __SomeScalar + } + + input __SomeInputObject { + __someField: String + } + + type __SomeObject { + __someField(__someArg: __SomeInputObject): String + } + + interface __SomeInterface { + __someField: String + } + + type SomeUnionMember { + field: String + } + + union __SomeUnion = SomeUnionMember + + enum __SomeEnum { + __SOME_ENUM_VALUE + } + + scalar __SomeScalar + `); + + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Name of directive "@__SomeDirective" must not begin with "__", which is reserved by GraphQL introspection.', + locations: [{ line: 2, column: 7 }], + }, + { + message: + 'Name "__someArg" of argument "@__SomeDirective(__someArg:)" must not begin with "__", which is reserved by GraphQL introspection.', + locations: [{ line: 2, column: 34 }], + }, + { + message: + 'Name of type "__SomeInputObject" must not begin with "__", which is reserved by GraphQL introspection.', + locations: [{ line: 12, column: 7 }], + }, + { + message: + 'Name "__someField" of input field "__SomeInputObject.__someField" must not begin with "__", which is reserved by GraphQL introspection.', + locations: [{ line: 13, column: 9 }], + }, + { + message: + 'Name of type "__SomeObject" must not begin with "__", which is reserved by GraphQL introspection.', + locations: [{ line: 16, column: 7 }], + }, + { + message: + 'Name "__someField" of field "__SomeObject.__someField" must not begin with "__", which is reserved by GraphQL introspection.', + locations: [{ line: 17, column: 9 }], + }, + { + message: + 'Name "__someArg" of argument "__SomeObject.__someField(__someArg:)" must not begin with "__", which is reserved by GraphQL introspection.', + locations: [{ line: 17, column: 21 }], + }, + { + message: + 'Name of type "__SomeInterface" must not begin with "__", which is reserved by GraphQL introspection.', + locations: [{ line: 20, column: 7 }], + }, + { + message: + 'Name "__someField" of field "__SomeInterface.__someField" must not begin with "__", which is reserved by GraphQL introspection.', + locations: [{ line: 21, column: 9 }], + }, + { + message: + 'Name of type "__SomeUnion" must not begin with "__", which is reserved by GraphQL introspection.', + locations: [{ line: 28, column: 7 }], + }, + { + message: + 'Name of type "__SomeEnum" must not begin with "__", which is reserved by GraphQL introspection.', + locations: [{ line: 30, column: 7 }], + }, + { + message: + 'Name "__SOME_ENUM_VALUE" of enum value "__SomeEnum.__SOME_ENUM_VALUE" must not begin with "__", which is reserved by GraphQL introspection.', + locations: [{ line: 31, column: 9 }], + }, + { + message: + 'Name of type "__SomeScalar" must not begin with "__", which is reserved by GraphQL introspection.', + locations: [{ line: 34, column: 7 }], + }, + ]); + }); +}); + describe('Type System: Objects must have fields', () => { it('accepts an Object type with fields object', () => { const schema = buildSchema(` @@ -586,65 +864,6 @@ describe('Type System: Objects must have fields', () => { }, ]); }); - - it('rejects an Object type with incorrectly named fields', () => { - const schema = schemaWithFieldType( - new GraphQLObjectType({ - name: 'SomeObject', - fields: { - __badName: { type: GraphQLString }, - }, - }), - ); - expectJSON(validateSchema(schema)).toDeepEqual([ - { - message: - 'Name "__badName" must not begin with "__", which is reserved by GraphQL introspection.', - }, - ]); - }); -}); - -describe('Type System: Fields args must be properly named', () => { - it('accepts field args with valid names', () => { - const schema = schemaWithFieldType( - new GraphQLObjectType({ - name: 'SomeObject', - fields: { - goodField: { - type: GraphQLString, - args: { - goodArg: { type: GraphQLString }, - }, - }, - }, - }), - ); - expectJSON(validateSchema(schema)).toDeepEqual([]); - }); - - it('rejects field arg with invalid names', () => { - const schema = schemaWithFieldType( - new GraphQLObjectType({ - name: 'SomeObject', - fields: { - badField: { - type: GraphQLString, - args: { - __badName: { type: GraphQLString }, - }, - }, - }, - }), - ); - - expectJSON(validateSchema(schema)).toDeepEqual([ - { - message: - 'Name "__badName" must not begin with "__", which is reserved by GraphQL introspection.', - }, - ]); - }); }); describe('Type System: Union types must be valid', () => { @@ -1376,24 +1595,6 @@ describe('Type System: Enum types must be well defined', () => { }, ]); }); - - it('rejects an Enum type with incorrectly named values', () => { - const schema = schemaWithFieldType( - new GraphQLEnumType({ - name: 'SomeEnum', - values: { - __badName: {}, - }, - }), - ); - - expectJSON(validateSchema(schema)).toDeepEqual([ - { - message: - 'Name "__badName" must not begin with "__", which is reserved by GraphQL introspection.', - }, - ]); - }); }); describe('Type System: Object fields must have output types', () => { diff --git a/src/type/assertHasValidName.ts b/src/type/assertHasValidName.ts new file mode 100644 index 0000000000..e42d876649 --- /dev/null +++ b/src/type/assertHasValidName.ts @@ -0,0 +1,86 @@ +import { inspect } from '../jsutils/inspect.js'; +import { invariant } from '../jsutils/invariant.js'; + +import { isNameContinue, isNameStart } from '../language/characterClasses.js'; + +import type { GraphQLSchemaElement } from './definition.js'; +import { + isArgument, + isEnumValue, + isField, + isInputField, + isNamedType, +} from './definition.js'; +import { isDirective } from './directives.js'; + +function getDescriptor( + schemaElement: GraphQLSchemaElement & { + readonly name: string; + }, +): string { + if (isNamedType(schemaElement)) { + return `of type "${schemaElement.name}"`; + } + if (isField(schemaElement)) { + return `"${schemaElement.name}" of field "${schemaElement}"`; + } + if (isArgument(schemaElement)) { + return `"${schemaElement.name}" of argument "${schemaElement}"`; + } + if (isInputField(schemaElement)) { + return `"${schemaElement.name}" of input field "${schemaElement}"`; + } + if (isEnumValue(schemaElement)) { + return `"${schemaElement.name}" of enum value "${schemaElement}"`; + } + if (isDirective(schemaElement)) { + return `of directive "${schemaElement}"`; + } + /* c8 ignore next 3 */ + // Not reachable, all possible inputs have been considered) + invariant(false, `Unexpected schema element: "${inspect(schemaElement)}".`); +} + +export function assertHasValidName( + schemaElement: GraphQLSchemaElement & { + readonly name: string; + }, + allowReservedNames = false, +): string { + const name = schemaElement.name; + if (name.length === 0) { + throw new Error( + `Expected name ${getDescriptor(schemaElement)} to be a non-empty string.`, + ); + } + + for (let i = 1; i < name.length; ++i) { + if (!isNameContinue(name.charCodeAt(i))) { + throw new Error( + `Name ${getDescriptor(schemaElement)} must only contain [_a-zA-Z0-9].`, + ); + } + } + + if (!isNameStart(name.charCodeAt(0))) { + throw new Error( + `Name ${getDescriptor(schemaElement)} must start with [_a-zA-Z].`, + ); + } + + if (!allowReservedNames && name.startsWith('__')) { + throw new Error( + `Name ${getDescriptor(schemaElement)} must not begin with "__", which is reserved by GraphQL introspection.`, + ); + } + + if (isEnumValue(schemaElement)) { + if (name === 'true' || name === 'false' || name === 'null') { + throw new Error( + `Name ${getDescriptor(schemaElement)} cannot be: ${name}.`, + ); + } + } + + return name; +} diff --git a/src/type/definition.ts b/src/type/definition.ts index ea96be5153..fac4302562 100644 --- a/src/type/definition.ts +++ b/src/type/definition.ts @@ -45,7 +45,6 @@ import type { VariableValues } from '../execution/values.js'; import { valueFromASTUntyped } from '../utilities/valueFromASTUntyped.js'; -import { assertEnumValueName, assertName } from './assertName.js'; import type { GraphQLDirective } from './directives.js'; import type { GraphQLSchema } from './schema.js'; @@ -669,7 +668,7 @@ export class GraphQLScalarType extensionASTNodes: ReadonlyArray; constructor(config: Readonly>) { - this.name = assertName(config.name); + this.name = config.name; this.description = config.description; this.specifiedByURL = config.specifiedByURL; this.serialize = @@ -880,7 +879,7 @@ export class GraphQLObjectType private _interfaces: ThunkReadonlyArray; constructor(config: Readonly>) { - this.name = assertName(config.name); + this.name = config.name; this.description = config.description; this.isTypeOf = config.isTypeOf; this.extensions = toObjMapWithSymbols(config.extensions); @@ -1116,7 +1115,7 @@ export class GraphQLField config: GraphQLFieldConfig, ) { this.parentType = parentType; - this.name = assertName(name); + this.name = name; this.description = config.description; this.type = config.type; @@ -1182,7 +1181,7 @@ export class GraphQLArgument implements GraphQLSchemaElement { config: GraphQLArgumentConfig, ) { this.parent = parent; - this.name = assertName(name); + this.name = name; this.description = config.description; this.type = config.type; this.defaultValue = config.defaultValue; @@ -1281,7 +1280,7 @@ export class GraphQLInterfaceType private _interfaces: ThunkReadonlyArray; constructor(config: Readonly>) { - this.name = assertName(config.name); + this.name = config.name; this.description = config.description; this.resolveType = config.resolveType; this.extensions = toObjMapWithSymbols(config.extensions); @@ -1407,7 +1406,7 @@ export class GraphQLUnionType implements GraphQLSchemaElement { private _types: ThunkReadonlyArray; constructor(config: Readonly>) { - this.name = assertName(config.name); + this.name = config.name; this.description = config.description; this.resolveType = config.resolveType; this.extensions = toObjMapWithSymbols(config.extensions); @@ -1528,7 +1527,7 @@ export class GraphQLEnumType /* */ implements GraphQLSchemaElement { private _nameLookup: ObjMap | null; constructor(config: Readonly */>) { - this.name = assertName(config.name); + this.name = config.name; this.description = config.description; this.extensions = toObjMapWithSymbols(config.extensions); this.astNode = config.astNode; @@ -1756,7 +1755,7 @@ export class GraphQLEnumValue implements GraphQLSchemaElement { config: GraphQLEnumValueConfig, ) { this.parentEnum = parentEnum; - this.name = assertEnumValueName(name); + this.name = name; this.description = config.description; this.value = config.value !== undefined ? config.value : name; this.deprecationReason = config.deprecationReason; @@ -1832,7 +1831,7 @@ export class GraphQLInputObjectType implements GraphQLSchemaElement { private _fields: ThunkObjMap; constructor(config: Readonly) { - this.name = assertName(config.name); + this.name = config.name; this.description = config.description; this.extensions = toObjMapWithSymbols(config.extensions); this.astNode = config.astNode; @@ -1960,7 +1959,7 @@ export class GraphQLInputField implements GraphQLSchemaElement { ); this.parentType = parentType; - this.name = assertName(name); + this.name = name; this.description = config.description; this.type = config.type; this.defaultValue = config.defaultValue; diff --git a/src/type/directives.ts b/src/type/directives.ts index 96f5b6b65a..365042f953 100644 --- a/src/type/directives.ts +++ b/src/type/directives.ts @@ -10,7 +10,6 @@ import { toObjMapWithSymbols } from '../jsutils/toObjMap.js'; import type { DirectiveDefinitionNode } from '../language/ast.js'; import { DirectiveLocation } from '../language/directiveLocation.js'; -import { assertName } from './assertName.js'; import type { GraphQLArgumentConfig, GraphQLFieldNormalizedConfigArgumentMap, @@ -62,7 +61,7 @@ export class GraphQLDirective implements GraphQLSchemaElement { astNode: Maybe; constructor(config: Readonly) { - this.name = assertName(config.name); + this.name = config.name; this.description = config.description; this.locations = config.locations; this.isRepeatable = config.isRepeatable ?? false; diff --git a/src/type/index.ts b/src/type/index.ts index dd9d103868..bbb75a9ef8 100644 --- a/src/type/index.ts +++ b/src/type/index.ts @@ -202,4 +202,5 @@ export { export { validateSchema, assertValidSchema } from './validate.js'; // Upholds the spec rules about naming. +export { assertHasValidName } from './assertHasValidName.js'; export { assertName, assertEnumValueName } from './assertName.js'; diff --git a/src/type/validate.ts b/src/type/validate.ts index ede99c8054..20ff83984f 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -34,6 +34,7 @@ import { validateInputValue, } from '../utilities/validateInputValue.js'; +import { assertHasValidName } from './assertHasValidName.js'; import type { GraphQLArgument, GraphQLEnumType, @@ -42,6 +43,7 @@ import type { GraphQLInputType, GraphQLInterfaceType, GraphQLObjectType, + GraphQLSchemaElement, GraphQLUnionType, } from './definition.js'; import { @@ -196,7 +198,7 @@ function validateDirectives(context: SchemaValidationContext): void { } // Ensure they are named correctly. - validateName(context, directive); + validateName(context, directive, false); if (directive.locations.length === 0) { context.reportError( @@ -208,7 +210,7 @@ function validateDirectives(context: SchemaValidationContext): void { // Ensure the arguments are valid. for (const arg of directive.args) { // Ensure they are named correctly. - validateName(context, arg); + validateName(context, arg, false); // Ensure the type is an input type. if (!isInputType(arg.type)) { @@ -348,14 +350,16 @@ function uncoerceDefaultValue(value: unknown, type: GraphQLInputType): unknown { function validateName( context: SchemaValidationContext, - node: { readonly name: string; readonly astNode: Maybe }, + schemaElement: GraphQLSchemaElement & { + readonly name: string; + readonly astNode: Maybe; + }, + allowReservedNames: boolean, ): void { - // Ensure names are valid, however introspection types opt out. - if (node.name.startsWith('__')) { - context.reportError( - `Name "${node.name}" must not begin with "__", which is reserved by GraphQL introspection.`, - node.astNode, - ); + try { + assertHasValidName(schemaElement, allowReservedNames); + } catch (error: unknown) { + context.reportError((error as Error).message, schemaElement.astNode); } } @@ -376,20 +380,18 @@ function validateTypes(context: SchemaValidationContext): void { continue; } - // Ensure it is named correctly (excluding introspection types). - if (!isIntrospectionType(type)) { - validateName(context, type); - } + const allowReservedNames = isIntrospectionType(type); + validateName(context, type, allowReservedNames); if (isObjectType(type)) { // Ensure fields are valid - validateFields(context, type); + validateFields(context, type, allowReservedNames); // Ensure objects implement the interfaces they claim to. validateInterfaces(context, type); } else if (isInterfaceType(type)) { // Ensure fields are valid. - validateFields(context, type); + validateFields(context, type, allowReservedNames); // Ensure interfaces implement the interfaces they claim to. validateInterfaces(context, type); @@ -398,10 +400,10 @@ function validateTypes(context: SchemaValidationContext): void { validateUnionMembers(context, type); } else if (isEnumType(type)) { // Ensure Enums have valid values. - validateEnumValues(context, type); + validateEnumValues(context, type, allowReservedNames); } else if (isInputObjectType(type)) { // Ensure Input Object fields are valid. - validateInputFields(context, type); + validateInputFields(context, type, allowReservedNames); // Ensure Input Objects do not contain invalid field circular references. // Ensure Input Objects do not contain non-nullable circular references. @@ -416,6 +418,7 @@ function validateTypes(context: SchemaValidationContext): void { function validateFields( context: SchemaValidationContext, type: GraphQLObjectType | GraphQLInterfaceType, + allowReservedNames: boolean, ): void { const fields = Object.values(type.getFields()); @@ -429,7 +432,7 @@ function validateFields( for (const field of fields) { // Ensure they are named correctly. - validateName(context, field); + validateName(context, field, allowReservedNames); // Ensure the type is an output type if (!isOutputType(field.type)) { @@ -443,7 +446,7 @@ function validateFields( // Ensure the arguments are valid for (const arg of field.args) { // Ensure they are named correctly. - validateName(context, arg); + validateName(context, arg, allowReservedNames); // Ensure the type is an input type if (!isInputType(arg.type)) { @@ -648,6 +651,7 @@ function validateUnionMembers( function validateEnumValues( context: SchemaValidationContext, enumType: GraphQLEnumType, + allowReservedNames: boolean, ): void { const enumValues = enumType.getValues(); @@ -660,13 +664,14 @@ function validateEnumValues( for (const enumValue of enumValues) { // Ensure valid name. - validateName(context, enumValue); + validateName(context, enumValue, allowReservedNames); } } function validateInputFields( context: SchemaValidationContext, inputObj: GraphQLInputObjectType, + allowReservedNames: boolean, ): void { const fields = Object.values(inputObj.getFields()); @@ -680,7 +685,7 @@ function validateInputFields( // Ensure the input fields are valid for (const field of fields) { // Ensure they are named correctly. - validateName(context, field); + validateName(context, field, allowReservedNames); // Ensure the type is an input type if (!isInputType(field.type)) { From 8e8d72aa8a1d9ab19fd9a458430997d276f93139 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 3 Jun 2025 17:10:52 +0300 Subject: [PATCH 2/3] f --- src/type/__tests__/validation-test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 939b2d3829..196f475a58 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -557,7 +557,7 @@ describe('Type System: Schema elements must be properly named', () => { } type SomeUnionMember { - field: String + field: String } union SomeUnion = SomeUnionMember @@ -727,7 +727,7 @@ describe('Type System: Schema elements must be properly named', () => { } type SomeUnionMember { - field: String + field: String } union __SomeUnion = SomeUnionMember From bcb8cea292053c931a4eff5e1dddf58fb84c7d85 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 3 Jun 2025 17:11:41 +0300 Subject: [PATCH 3/3] website fix --- website/pages/upgrade-guides/v16-v17.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/pages/upgrade-guides/v16-v17.mdx b/website/pages/upgrade-guides/v16-v17.mdx index 00b8a27343..00ab2b2662 100644 --- a/website/pages/upgrade-guides/v16-v17.mdx +++ b/website/pages/upgrade-guides/v16-v17.mdx @@ -145,7 +145,7 @@ Use the `validateInputValue` helper to retrieve the actual errors. - Removed deprecated `getOperationType` function, use `getRootType` on the `GraphQLSchema` instance instead - Removed deprecated `getVisitFn` function, use `getEnterLeaveForKind` instead - Removed deprecated `printError` and `formatError` utilities, you can use `toString` or `toJSON` on the error as a replacement -- Removed deprecated `assertValidName` and `isValidNameError` utilities, use `assertName` instead +- Removed deprecated `assertValidName` and `isValidNameError` utilities, use `assertHasValidName` instead - Removed deprecated `assertValidExecutionArguments` function, use `assertValidSchema` instead - Removed deprecated `getFieldDefFn` from `TypeInfo` - Removed deprecated `TypeInfo` from `validate` https://github.com/graphql/graphql-js/pull/4187