Skip to content

Commit 869ca46

Browse files
authored
Propose single wrapping type (#4339)
This reduces the new AST-nodes to only be for the newly introduced type, this does make it so that when we invoke `print` we have to rely on the user to either specify that we're in semantic nullability mode _or_ we could do a pre-traverse and when we enter a node with semantic-non-null we toggle it on ourselves. The main reasoning behind removing the new name for our existing null type is that I would prefer to be backwards compatible in terms of schema structure. This because it might become complex for people to reason about composed schemas, i.e. a lot of individually parsed schemas that later on compose into a larger one. I know that _technically_ this is covered because in the classic ones we'll have the non wrapped null type and in the modern ones we'll have the semantic nullable wrapped type. For schema-builders like pothos and others I think this is rather complex to reason about _and_ to supply us with. I would instead choose to absorb this complexity in the feature and stay backwards compatible. This also sets us up for the SDL not being a breaking change, we only add one AST-type, what's left now is to settle on a semantic non-null syntax and making everything backwards compatible.
1 parent 0f13010 commit 869ca46

13 files changed

+332
-439
lines changed

src/execution/__tests__/semantic-nullability-test.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import {
1010
GraphQLNonNull,
1111
GraphQLObjectType,
1212
GraphQLSemanticNonNull,
13-
GraphQLSemanticNullable,
1413
} from '../../type/definition';
1514
import { GraphQLString } from '../../type/scalars';
1615
import { GraphQLSchema } from '../../type/schema';
@@ -28,7 +27,7 @@ describe('Execute: Handles Semantic Nullability', () => {
2827
const DataType: GraphQLObjectType = new GraphQLObjectType({
2928
name: 'DataType',
3029
fields: () => ({
31-
a: { type: new GraphQLSemanticNullable(GraphQLString) },
30+
a: { type: GraphQLString },
3231
b: { type: new GraphQLSemanticNonNull(GraphQLString) },
3332
c: { type: new GraphQLNonNull(GraphQLString) },
3433
d: { type: new GraphQLSemanticNonNull(DeepDataType) },

src/execution/execute.ts

-13
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ import {
4444
isNonNullType,
4545
isObjectType,
4646
isSemanticNonNullType,
47-
isSemanticNullableType,
4847
} from '../type/definition';
4948
import {
5049
SchemaMetaFieldDef,
@@ -690,18 +689,6 @@ function completeValue(
690689
return completed;
691690
}
692691

693-
// If field type is SemanticNullable, complete for inner type
694-
if (isSemanticNullableType(returnType)) {
695-
return completeValue(
696-
exeContext,
697-
returnType.ofType,
698-
fieldNodes,
699-
info,
700-
path,
701-
result,
702-
);
703-
}
704-
705692
// If result value is null or undefined then return null.
706693
if (result == null) {
707694
return null;

src/language/__tests__/parser-test.ts

+6-10
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,7 @@ describe('Parser', () => {
659659
});
660660

661661
describe('parseDocumentDirective', () => {
662-
it('doesn\'t throw on document-level directive', () => {
662+
it("doesn't throw on document-level directive", () => {
663663
parse(dedent`
664664
@SemanticNullability
665665
type Query {
@@ -690,16 +690,12 @@ describe('Parser', () => {
690690
it('parses nullable types', () => {
691691
const result = parseType('MyType?', { allowSemanticNullability: true });
692692
expectJSON(result).toDeepEqual({
693-
kind: Kind.SEMANTIC_NULLABLE_TYPE,
694-
loc: { start: 0, end: 7 },
695-
type: {
696-
kind: Kind.NAMED_TYPE,
693+
kind: Kind.NAMED_TYPE,
694+
loc: { start: 0, end: 6 },
695+
name: {
696+
kind: Kind.NAME,
697697
loc: { start: 0, end: 6 },
698-
name: {
699-
kind: Kind.NAME,
700-
loc: { start: 0, end: 6 },
701-
value: 'MyType',
702-
},
698+
value: 'MyType',
703699
},
704700
});
705701
});

src/language/__tests__/predicates-test.ts

-1
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ describe('AST node predicates', () => {
9393
'ListType',
9494
'NonNullType',
9595
'SemanticNonNullType',
96-
'SemanticNullableType',
9796
]);
9897
});
9998

src/language/__tests__/schema-printer-test.ts

+15-5
Original file line numberDiff line numberDiff line change
@@ -183,28 +183,38 @@ describe('Printer: SDL document', () => {
183183

184184
it('prints NamedType', () => {
185185
expect(
186-
print(parseType('MyType', { allowSemanticNullability: false })),
186+
print(parseType('MyType', { allowSemanticNullability: false }), {
187+
useSemanticNullability: false,
188+
}),
187189
).to.equal(dedent`MyType`);
188190
});
189191

190192
it('prints SemanticNullableType', () => {
191193
expect(
192-
print(parseType('MyType?', { allowSemanticNullability: true })),
194+
print(parseType('MyType?', { allowSemanticNullability: true }), {
195+
useSemanticNullability: true,
196+
}),
193197
).to.equal(dedent`MyType?`);
194198
});
195199

196200
it('prints SemanticNonNullType', () => {
197201
expect(
198-
print(parseType('MyType', { allowSemanticNullability: true })),
202+
print(parseType('MyType', { allowSemanticNullability: true }), {
203+
useSemanticNullability: true,
204+
}),
199205
).to.equal(dedent`MyType`);
200206
});
201207

202208
it('prints NonNullType', () => {
203209
expect(
204-
print(parseType('MyType!', { allowSemanticNullability: true })),
210+
print(parseType('MyType!', { allowSemanticNullability: true }), {
211+
useSemanticNullability: true,
212+
}),
205213
).to.equal(dedent`MyType!`);
206214
expect(
207-
print(parseType('MyType!', { allowSemanticNullability: false })),
215+
print(parseType('MyType!', { allowSemanticNullability: false }), {
216+
useSemanticNullability: true,
217+
}),
208218
).to.equal(dedent`MyType!`);
209219
});
210220
});

src/language/ast.ts

+1-10
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,6 @@ export type ASTNode =
162162
| ListTypeNode
163163
| NonNullTypeNode
164164
| SemanticNonNullTypeNode
165-
| SemanticNullableTypeNode
166165
| SchemaDefinitionNode
167166
| OperationTypeDefinitionNode
168167
| ScalarTypeDefinitionNode
@@ -238,7 +237,6 @@ export const QueryDocumentKeys: {
238237
ListType: ['type'],
239238
NonNullType: ['type'],
240239
SemanticNonNullType: ['type'],
241-
SemanticNullableType: ['type'],
242240

243241
SchemaDefinition: ['description', 'directives', 'operationTypes'],
244242
OperationTypeDefinition: ['type'],
@@ -529,20 +527,13 @@ export interface SemanticNonNullTypeNode {
529527
readonly type: NamedTypeNode | ListTypeNode;
530528
}
531529

532-
export interface SemanticNullableTypeNode {
533-
readonly kind: Kind.SEMANTIC_NULLABLE_TYPE;
534-
readonly loc?: Location;
535-
readonly type: NamedTypeNode | ListTypeNode;
536-
}
537-
538530
/** Type Reference */
539531

540532
export type TypeNode =
541533
| NamedTypeNode
542534
| ListTypeNode
543535
| NonNullTypeNode
544-
| SemanticNonNullTypeNode
545-
| SemanticNullableTypeNode;
536+
| SemanticNonNullTypeNode;
546537

547538
export interface NamedTypeNode {
548539
readonly kind: Kind.NAMED_TYPE;

src/language/kinds.ts

-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ enum Kind {
3838
LIST_TYPE = 'ListType',
3939
NON_NULL_TYPE = 'NonNullType',
4040
SEMANTIC_NON_NULL_TYPE = 'SemanticNonNullType',
41-
SEMANTIC_NULLABLE_TYPE = 'SemanticNullableType',
4241

4342
/** Type System Definitions */
4443
SCHEMA_DEFINITION = 'SchemaDefinition',

src/language/parser.ts

+1-5
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ import type {
5151
SelectionNode,
5252
SelectionSetNode,
5353
SemanticNonNullTypeNode,
54-
SemanticNullableTypeNode,
5554
StringValueNode,
5655
Token,
5756
TypeNode,
@@ -795,10 +794,7 @@ export class Parser {
795794
type,
796795
});
797796
} else if (this.expectOptionalToken(TokenKind.QUESTION_MARK)) {
798-
return this.node<SemanticNullableTypeNode>(start, {
799-
kind: Kind.SEMANTIC_NULLABLE_TYPE,
800-
type,
801-
});
797+
return type;
802798
}
803799

804800
return this.node<SemanticNonNullTypeNode>(start, {

src/language/predicates.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,7 @@ export function isTypeNode(node: ASTNode): node is TypeNode {
6868
node.kind === Kind.NAMED_TYPE ||
6969
node.kind === Kind.LIST_TYPE ||
7070
node.kind === Kind.NON_NULL_TYPE ||
71-
node.kind === Kind.SEMANTIC_NON_NULL_TYPE ||
72-
node.kind === Kind.SEMANTIC_NULLABLE_TYPE
71+
node.kind === Kind.SEMANTIC_NON_NULL_TYPE
7372
);
7473
}
7574

0 commit comments

Comments
 (0)