Skip to content

suggestions for executable definitions PR #1

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.

Sign up for GitHub

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 2 commits into
base: executableDescriptions2025
Choose a base branch
from
Open

suggestions for executable definitions PR #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e12042e
reduce diff
yaacovCR Jul 17, 2025
5c7cb18
tweak parser changes
yaacovCR Jul 22, 2025
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
122 changes: 52 additions & 70 deletions src/language/__tests__/parser-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { expect } from 'chai';
import { assert, expect } from 'chai';
import { describe, it } from 'mocha';

import { dedent } from '../../__testUtils__/dedent';
Expand Down Expand Up @@ -57,6 +57,15 @@ describe('Parser', () => {
locations: [{ line: 1, column: 1 }],
});

// Throws on first error, the unexpected description.
expectSyntaxError(`
"Unexpected description"
notAnOperation Foo { field }
`).to.deep.include({
message: 'Syntax Error: Unexpected description, only GraphQL definitions support descriptions.',
locations: [{ line: 2, column: 7 }],
});

expectSyntaxError('...').to.deep.include({
message: 'Syntax Error: Unexpected "...".',
locations: [{ line: 1, column: 1 }],
Expand Down Expand Up @@ -163,25 +172,11 @@ describe('Parser', () => {
# This comment has a \u0A0A multi-byte character.
{ field(arg: "Has a \u0A0A multi-byte character.") }
`);
const opDef = ast.definitions.find(
(d) => d.kind === Kind.OPERATION_DEFINITION,

expect(ast).to.have.nested.property(
'definitions[0].selectionSet.selections[0].arguments[0].value.value',
'Has a \u0A0A multi-byte character.',
);
if (!opDef || opDef.kind !== Kind.OPERATION_DEFINITION) {
throw new Error('No operation definition found');
}
const fieldSel = opDef.selectionSet.selections[0];
if (fieldSel.kind !== Kind.FIELD) {
throw new Error('Expected a field selection');
}
const args = fieldSel.arguments;
if (!args || args.length === 0) {
throw new Error('No arguments found');
}
const argValueNode = args[0].value;
if (argValueNode.kind !== Kind.STRING) {
throw new Error('Expected a string value');
}
expect(argValueNode.value).to.equal('Has a \u0A0A multi-byte character.');
});

it('parses kitchen sink', () => {
Expand Down Expand Up @@ -350,7 +345,6 @@ describe('Parser', () => {

it('creates ast from nameless query without variables', () => {
const result = parse(dedent`
"Query description"
query {
node {
id
Expand All @@ -360,47 +354,42 @@ describe('Parser', () => {

expectJSON(result).toDeepEqual({
kind: Kind.DOCUMENT,
loc: { start: 0, end: 49 },
loc: { start: 0, end: 29 },
definitions: [
{
kind: Kind.OPERATION_DEFINITION,
loc: { start: 0, end: 49 },
description: {
kind: Kind.STRING,
loc: { start: 0, end: 19 },
block: false,
value: 'Query description',
},
loc: { start: 0, end: 29 },
description: undefined,
operation: 'query',
name: undefined,
variableDefinitions: [],
directives: [],
selectionSet: {
kind: Kind.SELECTION_SET,
loc: { start: 26, end: 49 },
loc: { start: 6, end: 29 },
selections: [
{
kind: Kind.FIELD,
loc: { start: 30, end: 47 },
loc: { start: 10, end: 27 },
alias: undefined,
name: {
kind: Kind.NAME,
loc: { start: 30, end: 34 },
loc: { start: 10, end: 14 },
value: 'node',
},
arguments: [],
directives: [],
selectionSet: {
kind: Kind.SELECTION_SET,
loc: { start: 35, end: 47 },
loc: { start: 15, end: 27 },
selections: [
{
kind: Kind.FIELD,
loc: { start: 41, end: 43 },
loc: { start: 21, end: 23 },
alias: undefined,
name: {
kind: Kind.NAME,
loc: { start: 41, end: 43 },
loc: { start: 21, end: 23 },
value: 'id',
},
arguments: [],
Expand Down Expand Up @@ -693,13 +682,10 @@ describe('Parser', () => {
field(a: $a, b: $b)
}
`);
// Find the operation definition
const opDef = result.definitions.find(
(d) => d.kind === Kind.OPERATION_DEFINITION,
);
if (!opDef || opDef.kind !== Kind.OPERATION_DEFINITION) {
throw new Error('No operation definition found');
}

const opDef = result.definitions[0];
assert(opDef.kind === Kind.OPERATION_DEFINITION);

expect(opDef.description?.value).to.equal('Operation description');
expect(opDef.name?.value).to.equal('myQuery');
expect(opDef.variableDefinitions?.[0].description?.value).to.equal(
Expand All @@ -712,15 +698,16 @@ describe('Parser', () => {
expect(opDef.variableDefinitions?.[1].description?.block).to.equal(true);
expect(opDef.variableDefinitions?.[0].variable.name.value).to.equal('a');
expect(opDef.variableDefinitions?.[1].variable.name.value).to.equal('b');
// Check type names safely

const typeA = opDef.variableDefinitions?.[0].type;
if (typeA && typeA.kind === Kind.NAMED_TYPE) {
expect(typeA.name.value).to.equal('Int');
}
assert(typeA?.kind === Kind.NAMED_TYPE);

expect(typeA.name.value).to.equal('Int');

const typeB = opDef.variableDefinitions?.[1].type;
if (typeB && typeB.kind === Kind.NAMED_TYPE) {
expect(typeB.name.value).to.equal('String');
}
assert(typeB?.kind === Kind.NAMED_TYPE);

expect(typeB.name.value).to.equal('String');
});

it('parses variable definition with description, default value, and directives', () => {
Expand All @@ -732,40 +719,35 @@ describe('Parser', () => {
field(foo: $foo)
}
`);
const opDef = result.definitions.find(
(d) => d.kind === Kind.OPERATION_DEFINITION,
);
if (!opDef || opDef.kind !== Kind.OPERATION_DEFINITION) {
throw new Error('No operation definition found');
}

const opDef = result.definitions[0];
assert(opDef.kind === Kind.OPERATION_DEFINITION);
const varDef = opDef.variableDefinitions?.[0];
expect(varDef?.description?.value).to.equal('desc');
expect(varDef?.variable.name.value).to.equal('foo');
if (varDef?.type.kind === Kind.NAMED_TYPE) {
expect(varDef.type.name.value).to.equal('Int');
}
if (varDef?.defaultValue && 'value' in varDef.defaultValue) {
expect(varDef.defaultValue.value).to.equal('42');
}

assert(varDef?.type.kind === Kind.NAMED_TYPE);
expect(varDef.type.name.value).to.equal('Int');

assert(varDef?.defaultValue?.kind === Kind.INT);
expect(varDef.defaultValue.value).to.equal('42');

expect(varDef?.directives?.[0].name.value).to.equal('dir');
});

it('parses fragment with variable description (legacy)', () => {
const result = parse('fragment Foo("desc" $foo: Int) on Bar { baz }', {
allowLegacyFragmentVariables: true,
});
const fragDef = result.definitions.find(
(d) => d.kind === Kind.FRAGMENT_DEFINITION,
);
if (!fragDef || fragDef.kind !== Kind.FRAGMENT_DEFINITION) {
throw new Error('No fragment definition found');
}
const fragDef = result.definitions[0]
assert(fragDef.kind === Kind.FRAGMENT_DEFINITION);

const varDef = fragDef.variableDefinitions?.[0];
expect(varDef?.description?.value).to.equal('desc');
expect(varDef?.variable.name.value).to.equal('foo');
if (varDef?.type.kind === Kind.NAMED_TYPE) {
expect(varDef.type.name.value).to.equal('Int');
}

assert(varDef?.type.kind === Kind.NAMED_TYPE);
expect(varDef.type.name.value).to.equal('Int');
});

it('produces sensible error for description on shorthand query', () => {
Expand All @@ -777,7 +759,7 @@ describe('Parser', () => {
}
`),
).to.throw(
'Syntax Error: Unexpected description, descriptions are not supported on shorthand queries.',
'Syntax Error: Unexpected description, shorthand queries do not support descriptions.',
);
});
});
Expand Down
4 changes: 2 additions & 2 deletions src/language/__tests__/schema-parser-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ describe('Schema Parser', () => {
}
`).to.deep.equal({
message:
'Syntax Error: Unexpected description, descriptions are not supported on type extensions.',
'Syntax Error: Unexpected description, only GraphQL definitions support descriptions.',
locations: [{ line: 2, column: 7 }],
});

Expand All @@ -353,7 +353,7 @@ describe('Schema Parser', () => {
}
`).to.deep.equal({
message:
'Syntax Error: Unexpected description, descriptions are not supported on type extensions.',
'Syntax Error: Unexpected description, only GraphQL definitions support descriptions.',
locations: [{ line: 2, column: 7 }],
});

Expand Down
45 changes: 22 additions & 23 deletions src/language/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,16 +258,25 @@ export class Parser {
* - InputObjectTypeDefinition
*/
parseDefinition(): DefinitionNode {
if (this.peek(TokenKind.BRACE_L)) {
return this.parseOperationDefinition();
}

// Many definitions begin with a description and require a lookahead.
const hasDescription = this.peekDescription();
const keywordToken = hasDescription
? this._lexer.lookahead()
: this._lexer.token;

if (keywordToken.kind === TokenKind.BRACE_L) {
// Check for shorthand query with description
if (hasDescription) {
throw syntaxError(
this._lexer.source,
this._lexer.token.start,
'Unexpected description, shorthand queries do not support descriptions.',
);
}
return this.parseOperationDefinition();
}

if (keywordToken.kind === TokenKind.NAME) {
switch (keywordToken.value) {
case 'schema':
Expand All @@ -286,37 +295,27 @@ export class Parser {
return this.parseInputObjectTypeDefinition();
case 'directive':
return this.parseDirectiveDefinition();
case 'query':
case 'mutation':
case 'subscription':
return this.parseOperationDefinition();
case 'fragment':
return this.parseFragmentDefinition();
}

if (hasDescription && keywordToken.value === 'extend') {
if (hasDescription) {
throw syntaxError(
this._lexer.source,
this._lexer.token.start,
'Unexpected description, descriptions are not supported on type extensions.',
'Unexpected description, only GraphQL definitions support descriptions.',
);
}

switch (keywordToken.value) {
case 'query':
case 'mutation':
case 'subscription':
return this.parseOperationDefinition();
case 'fragment':
return this.parseFragmentDefinition();
case 'extend':
return this.parseTypeSystemExtension();
if (keywordToken.value === 'extend') {
return this.parseTypeSystemExtension();
}
}

// Check for shorthand query with description
if (hasDescription && keywordToken.kind === TokenKind.BRACE_L) {
throw syntaxError(
this._lexer.source,
this._lexer.token.start,
'Unexpected description, descriptions are not supported on shorthand queries.',
);
}

throw this.unexpected(keywordToken);
}

Expand Down