Skip to content

Commit

Permalink
fix(ast): correctly mark fields as nullable when not required
Browse files Browse the repository at this point in the history
Creates a new type alias `Nullable<T>` which will make a given field nullable
if that field is not required by the schema.

This more accurately models the allowed types for GraphQL since GraphQL will
accept `null` as a valid value when a field is marked as such. Prior to this change,
`null` was not allowed as a value in TypeScript since the generated type definition did not allow it.

Closes #1128
  • Loading branch information
Will authored and willsoto committed Jan 22, 2021
1 parent 5e52a17 commit 06d001a
Show file tree
Hide file tree
Showing 14 changed files with 71 additions and 29 deletions.
44 changes: 32 additions & 12 deletions lib/graphql-ast.explorer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,17 @@ export class GraphQLAstExplorer {
? `${DEFINITIONS_FILE_HEADER}\n${options.additionalHeader}\n\n`
: DEFINITIONS_FILE_HEADER;
tsFile.insertText(0, header);
tsFile.addTypeAlias({
name: 'Nullable',
isExported: false,
type: 'T | null',
typeParameters: [
{
name: 'T',
},
],
});

return tsFile;
}

Expand Down Expand Up @@ -263,6 +274,7 @@ export class GraphQLAstExplorer {
}

const { name: type, required } = this.getFieldTypeDefinition(item.type);

if (!this.isRoot(parentRef.getName())) {
(parentRef as InterfaceDeclaration).addProperty({
name: propertyName,
Expand All @@ -271,6 +283,7 @@ export class GraphQLAstExplorer {
});
return;
}

if (options.skipResolverArgs) {
(parentRef as ClassDeclaration).addProperty({
name: propertyName,
Expand All @@ -281,9 +294,7 @@ export class GraphQLAstExplorer {
(parentRef as ClassDeclaration).addMethod({
isAbstract: mode === 'class',
name: propertyName,
returnType: `${this.addSymbolIfRoot(
type,
)} | Promise<${this.addSymbolIfRoot(type)}>`,
returnType: `${type} | Promise<${type}>`,
parameters: this.getFunctionParameters(
(item as FieldDefinitionNode).arguments,
),
Expand All @@ -292,28 +303,37 @@ export class GraphQLAstExplorer {
}

getFieldTypeDefinition(
type: TypeNode,
typeNode: TypeNode,
): {
name: string;
required: boolean;
} {
const { required, type: nestedType } = this.getNestedType(type);
type = nestedType;
const { required, type } = this.getNestedType(typeNode);

const isArray = type.kind === 'ListType';
if (isArray) {
const { type: nestedType } = this.getNestedType(get(type, 'type'));
type = nestedType;
const {
type: arrayType,
required: arrayTypeRequired,
} = this.getNestedType(get(type, 'type'));

const typeName = this.addSymbolIfRoot(get(arrayType, 'name.value'));
const name = arrayTypeRequired
? this.getType(typeName)
: `Nullable<${this.getType(typeName)}>`;

const typeName = get(type, 'name.value');
return {
name: this.getType(typeName) + '[]',
name: required ? name + '[]' : `Nullable<${name}[]>`,
required,
};
}
const typeName = get(type, 'name.value');

const typeName = this.addSymbolIfRoot(get(type, 'name.value'));

return {
name: this.getType(typeName),
name: required
? this.getType(typeName)
: `Nullable<${this.getType(typeName)}>`,
required,
};
}
Expand Down
8 changes: 5 additions & 3 deletions tests/generated-definitions/array-property.fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
/* eslint-disable */
export interface Foo {
a: string[];
b?: string[];
c: string[];
d?: string[];
b?: Nullable<string[]>;
c: Nullable<string>[];
d?: Nullable<Nullable<string>[]>;
}

type Nullable<T> = T | null;
1 change: 1 addition & 0 deletions tests/generated-definitions/custom-header.fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@
/* Put anything here you like */

export type CustomDate = any;
type Nullable<T> = T | null;
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@
/* tslint:disable */
/* eslint-disable */
export type CustomDate = unknown;
type Nullable<T> = T | null;
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@
export type List = string[];
export type Point = [number, number];
export type DateTime = Date;
type Nullable<T> = T | null;
1 change: 1 addition & 0 deletions tests/generated-definitions/custom-scalar.fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@
/* tslint:disable */
/* eslint-disable */
export type CustomDate = any;
type Nullable<T> = T | null;
2 changes: 2 additions & 0 deletions tests/generated-definitions/enum.fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@ export enum Foobar {
Bar = "Bar",
Baz = "Baz"
}

type Nullable<T> = T | null;
6 changes: 4 additions & 2 deletions tests/generated-definitions/federation.fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@ export class Post {
}

export abstract class IQuery {
abstract getPosts(): Post[] | Promise<Post[]>;
abstract getPosts(): Nullable<Nullable<Post>[]> | Promise<Nullable<Nullable<Post>[]>>;
}

export class User {
id: string;
posts?: Post[];
posts?: Nullable<Nullable<Post>[]>;
}

type Nullable<T> = T | null;
4 changes: 3 additions & 1 deletion tests/generated-definitions/interface-property.fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,7 @@ export interface Bar {

export interface Foo {
a: Bar;
b?: Bar;
b?: Nullable<Bar>;
}

type Nullable<T> = T | null;
8 changes: 5 additions & 3 deletions tests/generated-definitions/mutation.fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@ export interface Cat {
}

export interface IMutation {
createCat(name?: string): Cat | Promise<Cat>;
returnsQuery(): IQuery | Promise<IQuery>;
createCat(name?: Nullable<string>): Nullable<Cat> | Promise<Nullable<Cat>>;
returnsQuery(): Nullable<IQuery> | Promise<Nullable<IQuery>>;
}

export interface IQuery {
query(): number | Promise<number>;
query(): Nullable<number> | Promise<Nullable<number>>;
}

type Nullable<T> = T | null;
4 changes: 3 additions & 1 deletion tests/generated-definitions/query-skip-args.fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,7 @@ export interface Cat {
}

export interface IQuery {
cat?: Cat;
cat?: Nullable<Cat>;
}

type Nullable<T> = T | null;
4 changes: 3 additions & 1 deletion tests/generated-definitions/query.fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,7 @@ export interface Cat {
}

export interface IQuery {
cat(id: string): Cat | Promise<Cat>;
cat(id: string): Nullable<Cat> | Promise<Nullable<Cat>>;
}

type Nullable<T> = T | null;
8 changes: 5 additions & 3 deletions tests/generated-definitions/simple-type.fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
export interface Cat {
id: number;
name: string;
age?: number;
color?: string;
weight?: number;
age?: Nullable<number>;
color?: Nullable<string>;
weight?: Nullable<number>;
}

type Nullable<T> = T | null;
8 changes: 5 additions & 3 deletions tests/generated-definitions/typename.fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ export interface Cat {
__typename?: 'Cat';
id: number;
name: string;
age?: number;
color?: string;
weight?: number;
age?: Nullable<number>;
color?: Nullable<string>;
weight?: Nullable<number>;
}

type Nullable<T> = T | null;

0 comments on commit 06d001a

Please sign in to comment.