Skip to content

move validation of names into validateSchema #4423

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: next
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ export {
validateSchema,
assertValidSchema,
// Upholds the spec rules about naming.
assertHasValidName,
assertName,
assertEnumValueName,
} from './type/index.js';
Expand Down
22 changes: 22 additions & 0 deletions src/language/__tests__/schema-parser-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
82 changes: 82 additions & 0 deletions src/type/__tests__/assertHasValidName-test.ts
Original file line number Diff line number Diff line change
@@ -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.',
);
});
});
96 changes: 0 additions & 96 deletions src/type/__tests__/definition-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down
23 changes: 0 additions & 23 deletions src/type/__tests__/directive-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
});
});
Loading
Loading