Skip to content
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

Add support for setting default nullability for fields and return types #302

Merged
merged 4 commits into from
Apr 6, 2019
Merged
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 CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<!-- here goes all the unreleased changes descriptions -->
### Features
- add postinstall script for printing info on console about supporting the project
- add support for setting default nullability for fields and return types (#297)

## v0.17.1
### Features
Expand Down
6 changes: 4 additions & 2 deletions docs/types-and-fields.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,11 @@ For simple types (like `string` or `boolean`) this is all that's needed but due

Why use function syntax and not a simple `{ type: Rate }` config object? Because, by using function syntax we solve the problem of circular dependencies (e.g. Post <--> User), so it was adopted as a convention. You can use the shorthand syntax `@Field(() => Rate)` if you want to save some keystrokes but it might be less readable for others.

For nullable properties like `averageRating` which might not be defined when a recipe has no ratings yet, we mark the class property as optional with a `?:` operator and also have to pass the `{ nullable: true }` decorator parameter. We should be aware that when we declare our type as a nullable union (e.g. `string | null`), we need to explicitly provide the type to the `@Field` decorator.
By default, all fields are non nullable, just like properties in TypeScript. However, you can change that behavior by providing `nullableByDefault: true` option in `buildSchema` settings, described in [bootstrap guide](./bootstrap.md).

In the case of lists, we may also need to define their nullability in a more detailed form. The basic `{ nullable: true | false }` setting only applies to the whole list (`[Item!]` or `[Item!]!`), so if we need a sparse array, we can control the list items' nullability via `nullable: items` (for `[Item]!`) or `nullable: itemsAndList` (for the `[Item]`) option.
So for nullable properties like `averageRating` which might not be defined when a recipe has no ratings yet, we mark the class property as optional with a `?:` operator and also have to pass the `{ nullable: true }` decorator parameter. We should be aware that when we declare our type as a nullable union (e.g. `string | null`), we need to explicitly provide the type to the `@Field` decorator.

In the case of lists, we may also need to define their nullability in a more detailed form. The basic `{ nullable: true | false }` setting only applies to the whole list (`[Item!]` or `[Item!]!`), so if we need a sparse array, we can control the list items' nullability via `nullable: items` (for `[Item]!`) or `nullable: itemsAndList` (for the `[Item]`) option. Be aware that setting `nullableByDefault: true` option will also apply to lists, so it will produce `[Item]` type, just like with `nullable: itemsAndList`.

In the config object we can also provide the `description` and `deprecationReason` properties for GraphQL schema purposes.

Expand Down
13 changes: 10 additions & 3 deletions src/helpers/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ export function convertTypeIfScalar(type: any): GraphQLScalarType | undefined {
export function wrapWithTypeOptions<T extends GraphQLType>(
typeOwnerName: string,
type: T,
typeOptions: TypeOptions = {},
typeOptions: TypeOptions,
nullableByDefault: boolean,
): T {
if (
!typeOptions.array &&
Expand All @@ -61,15 +62,21 @@ export function wrapWithTypeOptions<T extends GraphQLType>(

let gqlType: GraphQLType = type;
if (typeOptions.array) {
if (typeOptions.nullable === "items" || typeOptions.nullable === "itemsAndList") {
if (
typeOptions.nullable === "items" ||
typeOptions.nullable === "itemsAndList" ||
(typeOptions.nullable === undefined && nullableByDefault === true)
) {
gqlType = new GraphQLList(gqlType);
} else {
gqlType = new GraphQLList(new GraphQLNonNull(gqlType));
}
}
if (
typeOptions.defaultValue === undefined &&
(!typeOptions.nullable || typeOptions.nullable === "items")
(typeOptions.nullable === false ||
(typeOptions.nullable === undefined && nullableByDefault === false) ||
typeOptions.nullable === "items")
) {
gqlType = new GraphQLNonNull(gqlType);
}
Expand Down
10 changes: 10 additions & 0 deletions src/schema/build-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ export interface BuildContextOptions {
pubSub?: PubSubEngine | PubSubOptions;
globalMiddlewares?: Array<Middleware<any>>;
container?: ContainerType | ContainerGetter<any>;
/**
* Default value for type decorators, like `@Field({ nullable: true })`
*/
nullableByDefault?: boolean;
}

export abstract class BuildContext {
Expand All @@ -37,6 +41,7 @@ export abstract class BuildContext {
static pubSub: PubSubEngine;
static globalMiddlewares: Array<Middleware<any>>;
static container: IOCContainer;
static nullableByDefault: boolean;

/**
* Set static fields with current building context data
Expand Down Expand Up @@ -75,6 +80,10 @@ export abstract class BuildContext {
}

this.container = new IOCContainer(options.container);

if (options.nullableByDefault !== undefined) {
this.nullableByDefault = options.nullableByDefault;
}
}

/**
Expand All @@ -89,6 +98,7 @@ export abstract class BuildContext {
this.pubSub = new PubSub();
this.globalMiddlewares = [];
this.container = new IOCContainer();
this.nullableByDefault = false;
}
}

Expand Down
6 changes: 4 additions & 2 deletions src/schema/schema-generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,8 @@ export abstract class SchemaGenerator {
throw new Error(`Cannot determine GraphQL output type for ${typeOwnerName}`!);
}

return wrapWithTypeOptions(typeOwnerName, gqlType, typeOptions);
const { nullableByDefault } = BuildContext;
return wrapWithTypeOptions(typeOwnerName, gqlType, typeOptions, nullableByDefault);
}

private static getGraphQLInputType(
Expand All @@ -569,6 +570,7 @@ export abstract class SchemaGenerator {
throw new Error(`Cannot determine GraphQL input type for ${typeOwnerName}`!);
}

return wrapWithTypeOptions(typeOwnerName, gqlType, typeOptions);
const { nullableByDefault } = BuildContext;
return wrapWithTypeOptions(typeOwnerName, gqlType, typeOptions, nullableByDefault);
}
}
179 changes: 179 additions & 0 deletions tests/functional/default-nullable.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
import "reflect-metadata";
import {
IntrospectionSchema,
IntrospectionObjectType,
IntrospectionNonNullTypeRef,
IntrospectionNamedTypeRef,
TypeKind,
IntrospectionListTypeRef,
} from "graphql";

import { Field, ObjectType, Resolver, Query } from "../../src";
import { getMetadataStorage } from "../../src/metadata/getMetadataStorage";
import { getSchemaInfo } from "../helpers/getSchemaInfo";

describe("buildSchema -> nullableByDefault", () => {
let SampleObjectClass: any;
let SampleResolverClass: any;

beforeEach(async () => {
getMetadataStorage().clear();

@ObjectType()
class SampleObject {
@Field()
normalField: string;

@Field(type => [String])
normalArrayField: string[];

@Field({ nullable: true })
nullableField: string;

@Field({ nullable: false })
nonNullableField: string;
}
SampleObjectClass = SampleObject;

@Resolver(of => SampleObject)
class SampleResolver {
@Query()
normalQuery(): string {
return "normalQuery";
}

@Query(returns => [String])
normalArrayQuery(): string[] {
return ["normalArrayQuery"];
}

@Query(type => String, { nullable: true })
nullableQuery() {
return null;
}

@Query({ nullable: false })
nonNullableQuery(): string {
return "nonNullableQuery";
}
}
SampleResolverClass = SampleResolver;
});

describe("default behavior", () => {
let schemaIntrospection: IntrospectionSchema;
let queryType: IntrospectionObjectType;
let sampleObjectType: IntrospectionObjectType;

beforeEach(async () => {
({ schemaIntrospection, queryType } = await getSchemaInfo({
resolvers: [SampleResolverClass],
}));
sampleObjectType = schemaIntrospection.types.find(
type => type.name === "SampleObject",
) as IntrospectionObjectType;
});

it("should emit non nullable fields by default", async () => {
const normalField = sampleObjectType.fields.find(it => it.name === "normalField");
const normalFieldType = normalField!.type as IntrospectionNonNullTypeRef;
const normalFieldInnerType = normalFieldType.ofType as IntrospectionNamedTypeRef;
expect(normalFieldType.kind).toBe(TypeKind.NON_NULL);
expect(normalFieldInnerType.name).toBe("String");

const normalArrayField = sampleObjectType.fields.find(it => it.name === "normalArrayField");
const normalArrayFieldType = normalArrayField!.type as IntrospectionNonNullTypeRef;
const normalArrayFieldListType = normalArrayFieldType.ofType as IntrospectionListTypeRef;
const normalArrayFieldListElementType = normalArrayFieldListType.ofType as IntrospectionNonNullTypeRef;
const normalArrayFieldListElementInnerType = normalArrayFieldListElementType.ofType as IntrospectionNamedTypeRef;
expect(normalArrayFieldType.kind).toBe(TypeKind.NON_NULL);
expect(normalArrayFieldListType.kind).toBe(TypeKind.LIST);
expect(normalArrayFieldListElementType.kind).toBe(TypeKind.NON_NULL);
expect(normalArrayFieldListElementInnerType.kind).toBe(TypeKind.SCALAR);
expect(normalArrayFieldListElementInnerType.name).toBe("String");
});

it("should emit non nullable queries by default", async () => {
const normalQuery = queryType.fields.find(it => it.name === "normalQuery");
const normalQueryType = normalQuery!.type as IntrospectionNonNullTypeRef;
const normalQueryInnerType = normalQueryType.ofType as IntrospectionNamedTypeRef;
expect(normalQueryType.kind).toBe(TypeKind.NON_NULL);
expect(normalQueryInnerType.name).toBe("String");

const normalArrayQuery = queryType.fields.find(it => it.name === "normalArrayQuery");
const normalArrayQueryType = normalArrayQuery!.type as IntrospectionNonNullTypeRef;
const normalArrayQueryListType = normalArrayQueryType.ofType as IntrospectionListTypeRef;
const normalArrayQueryListElementType = normalArrayQueryListType.ofType as IntrospectionNonNullTypeRef;
const normalArrayQueryListElementInnerType = normalArrayQueryListElementType.ofType as IntrospectionNamedTypeRef;
expect(normalArrayQueryType.kind).toBe(TypeKind.NON_NULL);
expect(normalArrayQueryListType.kind).toBe(TypeKind.LIST);
expect(normalArrayQueryListElementType.kind).toBe(TypeKind.NON_NULL);
expect(normalArrayQueryListElementInnerType.kind).toBe(TypeKind.SCALAR);
expect(normalArrayQueryListElementInnerType.name).toBe("String");
});
});

describe("nullableByDefault = true", () => {
let schemaIntrospection: IntrospectionSchema;
let queryType: IntrospectionObjectType;
let sampleObjectType: IntrospectionObjectType;

beforeEach(async () => {
({ schemaIntrospection, queryType } = await getSchemaInfo({
resolvers: [SampleResolverClass],
nullableByDefault: true,
}));
sampleObjectType = schemaIntrospection.types.find(
type => type.name === "SampleObject",
) as IntrospectionObjectType;
});

it("should emit nullable fields by default", async () => {
const normalField = sampleObjectType.fields.find(it => it.name === "normalField")!;
const normalFieldType = normalField.type as IntrospectionNamedTypeRef;
expect(normalFieldType.name).toBe("String");

const normalArrayField = sampleObjectType.fields.find(it => it.name === "normalArrayField");
const normalArrayFieldType = normalArrayField!.type as IntrospectionListTypeRef;
const normalArrayFieldListElementInnerType = normalArrayFieldType.ofType as IntrospectionNamedTypeRef;
expect(normalArrayFieldType.kind).toBe(TypeKind.LIST);
expect(normalArrayFieldListElementInnerType.kind).toBe(TypeKind.SCALAR);
expect(normalArrayFieldListElementInnerType.name).toBe("String");
});

it("should emit nullable queries by default", async () => {
const normalQuery = queryType.fields.find(it => it.name === "normalQuery")!;
const normalQueryType = normalQuery.type as IntrospectionNamedTypeRef;
expect(normalQueryType.name).toBe("String");

const normalArrayQuery = queryType.fields.find(it => it.name === "normalArrayQuery");
const normalArrayQueryType = normalArrayQuery!.type as IntrospectionListTypeRef;
const normalArrayQueryListElementInnerType = normalArrayQueryType.ofType as IntrospectionNamedTypeRef;
expect(normalArrayQueryType.kind).toBe(TypeKind.LIST);
expect(normalArrayQueryListElementInnerType.kind).toBe(TypeKind.SCALAR);
expect(normalArrayQueryListElementInnerType.name).toBe("String");
});

it("shouldn't affect explicit nullability options from decorators", async () => {
const nullableField = sampleObjectType.fields.find(it => it.name === "nullableField")!;
const nullableFieldType = nullableField.type as IntrospectionNamedTypeRef;
expect(nullableFieldType.name).toBe("String");

const nonNullableField = sampleObjectType.fields.find(it => it.name === "nonNullableField")!;
const nonNullableFieldType = nonNullableField.type as IntrospectionNonNullTypeRef;
const nonNullableFieldInnerType = nonNullableFieldType.ofType as IntrospectionNamedTypeRef;
expect(nonNullableFieldType.kind).toBe(TypeKind.NON_NULL);
expect(nonNullableFieldInnerType.name).toBe("String");

const nullableQuery = queryType.fields.find(it => it.name === "nullableQuery")!;
const nullableQueryType = nullableQuery.type as IntrospectionNamedTypeRef;
expect(nullableQueryType.name).toBe("String");

const nonNullableQuery = queryType.fields.find(it => it.name === "nonNullableQuery")!;
const nonNullableQueryType = nonNullableQuery.type as IntrospectionNonNullTypeRef;
const nonNullableQueryInnerType = nonNullableQueryType.ofType as IntrospectionNamedTypeRef;
expect(nonNullableQueryType.kind).toBe(TypeKind.NON_NULL);
expect(nonNullableQueryInnerType.name).toBe("String");
});
});
});