Skip to content

Commit

Permalink
Remove SemanticNonNull from TypeNode
Browse files Browse the repository at this point in the history
This type is reused for variables, inputs and list-types which
can't be smantically non-null. list-types can be but only if they
are used in an SDL context.

This is kind of a short-coming of our types, we conflate SDL
and execution language.
  • Loading branch information
JoviDeCroock committed Feb 12, 2025
1 parent 2321f93 commit 4c8a02b
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 49 deletions.
31 changes: 7 additions & 24 deletions src/execution/__tests__/semantic-nullability-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,7 @@ describe('Execute: Handles Semantic Nullability', () => {

it('SemanticNonNull throws error on null without error', async () => {
const data = {
a: () => 'Apple',
b: () => null,
c: () => 'Cookie',
};

const document = parse(`
Expand All @@ -53,11 +51,8 @@ describe('Execute: Handles Semantic Nullability', () => {
rootValue: data,
});

const executable = document.definitions?.values().next()
.value as ExecutableDefinitionNode;
const selectionSet = executable.selectionSet.selections
.values()
.next().value;
const executable = document.definitions[0] as ExecutableDefinitionNode;
const selectionSet = executable.selectionSet.selections[0];

expect(result).to.deep.equal({
data: {
Expand All @@ -77,11 +72,9 @@ describe('Execute: Handles Semantic Nullability', () => {

it('SemanticNonNull succeeds on null with error', async () => {
const data = {
a: () => 'Apple',
b: () => {
throw new Error('Something went wrong');
},
c: () => 'Cookie',
};

const document = parse(`
Expand All @@ -90,11 +83,8 @@ describe('Execute: Handles Semantic Nullability', () => {
}
`);

const executable = document.definitions?.values().next()
.value as ExecutableDefinitionNode;
const selectionSet = executable.selectionSet.selections
.values()
.next().value;
const executable = document.definitions[0] as ExecutableDefinitionNode;
const selectionSet = executable.selectionSet.selections[0];

const result = await execute({
schema: new GraphQLSchema({ query: DataType }),
Expand All @@ -121,9 +111,6 @@ describe('Execute: Handles Semantic Nullability', () => {
};

const data = {
a: () => 'Apple',
b: () => null,
c: () => 'Cookie',
d: () => deepData,
};

Expand All @@ -141,13 +128,9 @@ describe('Execute: Handles Semantic Nullability', () => {
rootValue: data,
});

const executable = document.definitions?.values().next()
.value as ExecutableDefinitionNode;
const dSelectionSet = executable.selectionSet.selections.values().next()
.value as FieldNode;
const fSelectionSet = dSelectionSet.selectionSet?.selections
.values()
.next().value;
const executable = document.definitions[0] as ExecutableDefinitionNode;
const dSelectionSet = executable.selectionSet.selections[0] as FieldNode;
const fSelectionSet = dSelectionSet.selectionSet?.selections[0];

expect(result).to.deep.equal({
data: {
Expand Down
10 changes: 3 additions & 7 deletions src/language/ast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -529,11 +529,7 @@ export interface SemanticNonNullTypeNode {

/** Type Reference */

export type TypeNode =
| NamedTypeNode
| ListTypeNode
| NonNullTypeNode
| SemanticNonNullTypeNode;
export type TypeNode = NamedTypeNode | ListTypeNode | NonNullTypeNode;

export interface NamedTypeNode {
readonly kind: Kind.NAMED_TYPE;
Expand All @@ -544,7 +540,7 @@ export interface NamedTypeNode {
export interface ListTypeNode {
readonly kind: Kind.LIST_TYPE;
readonly loc?: Location;
readonly type: TypeNode;
readonly type: TypeNode | SemanticNonNullTypeNode;
}

export interface NonNullTypeNode {
Expand Down Expand Up @@ -609,7 +605,7 @@ export interface FieldDefinitionNode {
readonly description?: StringValueNode;
readonly name: NameNode;
readonly arguments?: ReadonlyArray<InputValueDefinitionNode>;
readonly type: TypeNode;
readonly type: TypeNode | SemanticNonNullTypeNode;
readonly directives?: ReadonlyArray<ConstDirectiveNode>;
}

Expand Down
11 changes: 6 additions & 5 deletions src/language/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ export function parseConstValue(
export function parseType(
source: string | Source,
options?: ParseOptions | undefined,
): TypeNode {
): TypeNode | SemanticNonNullTypeNode {
const parser = new Parser(source, options);
parser.expectToken(TokenKind.SOF);
const type = parser.parseTypeReference();
Expand Down Expand Up @@ -403,7 +403,8 @@ export class Parser {
return this.node<VariableDefinitionNode>(this._lexer.token, {
kind: Kind.VARIABLE_DEFINITION,
variable: this.parseVariable(),
type: (this.expectToken(TokenKind.COLON), this.parseTypeReference()),
type: (this.expectToken(TokenKind.COLON),
this.parseTypeReference()) as TypeNode,
defaultValue: this.expectOptionalToken(TokenKind.EQUALS)
? this.parseConstValueLiteral()
: undefined,
Expand Down Expand Up @@ -773,15 +774,15 @@ export class Parser {
* - ListType
* - NonNullType
*/
parseTypeReference(): TypeNode {
parseTypeReference(): TypeNode | SemanticNonNullTypeNode {
const start = this._lexer.token;
let type;
if (this.expectOptionalToken(TokenKind.BRACKET_L)) {
const innerType = this.parseTypeReference();
this.expectToken(TokenKind.BRACKET_R);
type = this.node<ListTypeNode>(start, {
kind: Kind.LIST_TYPE,
type: innerType,
type: innerType as TypeNode,
});
} else {
type = this.parseNamedType();
Expand Down Expand Up @@ -992,7 +993,7 @@ export class Parser {
kind: Kind.INPUT_VALUE_DEFINITION,
description,
name,
type,
type: type as TypeNode,
defaultValue,
directives,
});
Expand Down
5 changes: 4 additions & 1 deletion src/utilities/extendSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import type {
ScalarTypeExtensionNode,
SchemaDefinitionNode,
SchemaExtensionNode,
SemanticNonNullTypeNode,
TypeDefinitionNode,
TypeNode,
UnionTypeDefinitionNode,
Expand Down Expand Up @@ -431,7 +432,9 @@ export function extendSchemaImpl(
return type;
}

function getWrappedType(node: TypeNode): GraphQLType {
function getWrappedType(
node: TypeNode | SemanticNonNullTypeNode,
): GraphQLType {
if (node.kind === Kind.LIST_TYPE) {
return new GraphQLList(getWrappedType(node.type));
}
Expand Down
14 changes: 2 additions & 12 deletions src/utilities/typeFromAST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,7 @@ import type {
import { Kind } from '../language/kinds';

import type { GraphQLNamedType, GraphQLType } from '../type/definition';
import {
GraphQLList,
GraphQLNonNull,
GraphQLSemanticNonNull,
} from '../type/definition';
import { GraphQLList, GraphQLNonNull } from '../type/definition';
import type { GraphQLSchema } from '../type/schema';

/**
Expand Down Expand Up @@ -43,19 +39,13 @@ export function typeFromAST(
): GraphQLType | undefined {
switch (typeNode.kind) {
case Kind.LIST_TYPE: {
const innerType = typeFromAST(schema, typeNode.type);
const innerType = typeFromAST(schema, typeNode.type as TypeNode);
return innerType && new GraphQLList(innerType);
}
case Kind.NON_NULL_TYPE: {
const innerType = typeFromAST(schema, typeNode.type);
return innerType && new GraphQLNonNull(innerType);
}
// We only use typeFromAST for fragment/variable type inference
// which should not be affected by semantic non-null types
// case Kind.SEMANTIC_NON_NULL_TYPE: {
// const innerType = typeFromAST(schema, typeNode.type);
// return innerType && new GraphQLSemanticNonNull(innerType);
// }
case Kind.NAMED_TYPE:
return schema.getType(typeNode.name.value);
}
Expand Down

0 comments on commit 4c8a02b

Please sign in to comment.