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

fix: Type.Optional() set nullable: true #118

Closed
wants to merge 2 commits into from
Closed

fix: Type.Optional() set nullable: true #118

wants to merge 2 commits into from

Conversation

theoludwig
Copy link

@theoludwig theoludwig commented Nov 12, 2021

Hello! 👋 @sinclairzx81

Current Behavior

Currently, a null value of type: "string" in the ajv schema is serialized as a double-quoted empty string ("")

Expected Behavior

A null string should be serialized as null, and not "".

Solution

This PR adds nullable: true to the schema object that returns Type.Optional.
Might be a BREAKING CHANGE, I don't really know to be honest but, maybe we could add an option to Type.Optional to not add this behavior by default.

References

@sinclairzx81
Copy link
Owner

sinclairzx81 commented Nov 13, 2021

@divlo Hi, thanks for the PR. Unfortunately I can't merge this one as nullable isn't a valid JSON schema keyword. The nullable keyword is only present in the Open API / Swagger specifications, and I believe they may be dropping support for it and moving to draft 2020-12 onwards. Refer to relevant issues below.

json-schema-org/json-schema-spec#1067
OAI/OpenAPI-Specification#2246

Optional Properties

The Type.Optional(T) modifier is used to make a object property T | undefined. It mirrors the TypeScript prop?: T syntax for specifying the potential absence of value.

Consider the following.

// TypeScript
interface Foo {
   name?: string // string | undefined
}
// TypeBox
const Foo = Type.Object({
   name: Type.Optional(Type.String()) // string | undefined
})
// JSON Schema
const Foo = { 
  type: 'object', 
  properties: { 
     name: { type: 'string' } 
  },
  required: []  // not required
}

Handling Null

TypeBox handles null in the same way as TypeScript, where null is treated as a value type similar to number or string. TypeBox uses union types to describe values that can be either null or some other type.

Consider the following

// TypeScript
interface Foo {
    name: string | null
}
// TypeBox
const Foo = Type.Object({
   name: Type.Union([Type.String(), Type.Null()]) // string | null
})
// JSON Schema
const Foo = { 
  type: 'object', 
  properties: { 
    name: { 
      anyOf: [
        { type: 'string' },
        { type: 'null' }
      ]
    } 
  },
  required: ['name']  // required
}

Implementing Nullable

TypeBox doesn't provide out of the box support for the nullable keyword, but does allow users to construct custom types that map back to TypeBox's Type System. The following implements Type.Nullable(T) using generics.

function Nullable<T extends TSchema>(schema: T): TUnion<[T, TNull]> {
    return { ...schema, nullable: true } as any
}

const T = Nullable(Type.String())        // const T = {
                                         //   type: 'string',
                                         //   nullable: true
                                         // }

type T = Static<typeof T>                // type T = string | null

For additional information, refer to the OpenAPI section of the TypeBox readme.

@theoludwig
Copy link
Author

Thanks for your detailed explanation! @sinclairzx81

I believe they may be dropping support for it and moving to draft 2020-12 onwards. Refer to relevant issues below.

Indeed, we should not implement things that are not supported anymore, I didn't know that nullable was only present in the Open API / Swagger specifications. 👍

This solved the problem:

const Foo = Type.Object({
   name: Type.Union([Type.String({ minLength: 1 }), Type.Null()]) // string | null
})

Set minLength: 1, allows to convert "" to null and thanks to Type.Union, null value stays null. 👍

Closing, as the issue is solved (I should have opened an issue before opening a PR my bad).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants