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

The nullable option is not reflected in TypeScript types #38

Closed
sunaurus opened this issue Oct 18, 2020 · 15 comments
Closed

The nullable option is not reflected in TypeScript types #38

sunaurus opened this issue Oct 18, 2020 · 15 comments

Comments

@sunaurus
Copy link
Contributor

const NullableSchema = Type.Number({nullable: true})
type NullableSchemaType = Static<typeof NullableSchema>;
const test: NullableSchemaType = null;

results in:

error TS2322: Type 'null' is not assignable to type 'number'.

Is it even possible to fix this?

I realize that the recommended workaround is to use Type.Union() with Type.Null(), but this does not play well with some other libraries (among other reasons due to #37) and is actually considered an anti-pattern by some people: ajv-validator/ajv#1107 (comment)

@sinclairzx81
Copy link
Owner

@sunaurus Hi

Yeah, TypeBox doesn't currently try and infer types from the additional options passed on the types (i.e. { nullable: true}). I would be happy to investigate adding something to interpret nullable, however nullable would need to be represented in the current JSON schema draft specification.

Do you have some documentation / specifications available for nullable ?

@sunaurus
Copy link
Contributor Author

You're right, I assumed that it was JSON schema but it's actually just an OpenAPI thing. My mistake!

@Nepoxx
Copy link

Nepoxx commented Dec 4, 2020

@sinclairzx81 while this is indeed an extension supported by OpenAPI, it would be very useful to me. I don't think it should be in the base library but maybe you can comment on how I could add this functionality locally. I was initially going for something like this:

class NullableTypeBuilder extends TypeBuilder {
  NullableString<TCustomFormatOption extends string>(
    options?: StringOptions<StringFormatOption | TCustomFormatOption>,
  ): TOptional<TString> {
    return {
      ...options,
      kind: exports.StringKind,
      modifier: exports.OptionalModifier,
      nullable: true,
      type: 'string',
    };
  }
}

const MyType = new NullableTypeBuilder();

export const testSchema = MyType.Object({
  display_name: MyType.NullableString(),
});

export type TestType = Static<typeof testSchema>;

however TOptional is not what I would like here, since it gives:

type TestType = {} & {} & {
    display_name?: string;
} & {}

instead of

type TestType = {} & {} & {
    display_name: string | null;
} & {}

I'm not sure how to do the latter without forking Typebox, any ideas?

@sinclairzx81
Copy link
Owner

sinclairzx81 commented Dec 4, 2020

@Nepoxx Hi,

You can achieve this with the following examples, but you will need to run the typescript compiler in strict: true mode for null | string inference to work as expected. See examples below.

import { Type, Static } from '@sinclair/typebox'

const T = Type.Object({
    display_name: Type.Union([Type.String({ nullable: true }), Type.Null()])
})

type T = Static<typeof T>
//
// type T = {} & {} & {} & {
//     display_name: string | null;
// }

Or, if you want to construct a reusable type for nullable strings, you can do something like.

import { Type, Static, StringFormatOption, StringOptions } from '@sinclair/typebox'

// The following is the full signature for a kind of `nullable` string. The generic `CustomFormat`
// allows passing additional custom formats for string validation above an beyond the built-in JSON schema
// string format validation types.
const NullableString = <CustomFormat extends string>(options?: StringOptions<StringFormatOption | CustomFormat>) => {
    return Type.Union([Type.String({ nullable: true, ...options }), Type.Null()])
}

const T = Type.Object({
    display_name:  NullableString(),
    display_email: NullableString({ format: 'email' })
})

type T = Static<typeof T>
// type T = {} & {} & {} & {
//     display_name: string | null;
//     display_email: string | null;
// }

Also, it's possible to extend (sub class) the TypeBuilder to add the methods there if you wish (as in your example). I think for something like this, composition of the NullableString type is probably the easiest tho.

@Nepoxx
Copy link

Nepoxx commented Dec 7, 2020

Thanks for the quick reply :)

The issues is that Type.Union generates anyOf which is not exactly the same as an array of types.

i.e.

{
  type: 'object'
  properties: {
    foo: {
      anyOf: [
        {type: 'string'},
        {type: 'null'}
      ]
  }
  }
}
// vs
{
  type: 'object'
  properties: {
    foo: { type: ['string', 'null'] }
  }
}

the main difference comes from how the spec says one should be validated and how types are coerced. My ideal is having

{
  type: 'object'
  properties: {
    foo: { type: 'string', nullable: true }
  }
}

and having the corresponding TS type:

type Foo = {
    foo: string | null;
}

@sinclairzx81
Copy link
Owner

@Nepoxx Thanks for the response.

I am open to amending the output JSON schema for better (or more terse) alignment with the TS type checker, however it would need to be in tune with the JSON schema specification. Unfortunately, the nullable property only relates to the Open API / Swagger specification, so I can't really add it to TypeBox without it diverging from the spec.

I'm not sure why the Swagger team decided to diverge from the JSON schema specification, but its a little frustrating they did. I'm guessing the Open API nullable property was pre-emptively added before JSON schema got anyOf, allOf and oneOf composite schemas. Looking at the swagger docs, it does look like Swagger does at least support those composite types here, but doesn't have a null type.

I could recommend you perhaps forking TypeBox and having a go at adding this in. My advice might be to create a new Nullable(T) type. This is similar to what the Readonly, Optional and ReadonlyOptional modifiers do currently, so they might make for a good starting place to start adding this in.

const T = Type.Object({
     foo: Type.Nullable(Type.String())
})

const T = {
     "type": "object",
     "properties": {
          "foo": { "type" : "string", "nullable": true }
     },
     "required": ["foo"]
}

Where Type.Nullable(T) statically resolves to T | null. You will also need to add a StaticNullable<T> type to do the inference. Something like...

export type StaticNullable<T extends TSchema> = Static<T> | null

Hope this helps
S

@mindplay-dk
Copy link

Is there still no way to add nullables to JSON schema today?

I'm refactoring a codebase from hand-written TS types, and now I'm forced to use undefined instead of null in a bunch of places, which is a breaking change, and causes JSON.stringify to emit no property at all, as opposed to e.g. value: null.

null is native to JSON, while undefined isn't supported by JSON at all, so this is quite frustrating.

No workaround besides Open API? (which breaks JSON schema...)

@mindplay-dk
Copy link

According to this answer, it seems we want type: ["string", "null"].

Since the nullable option is supported for Open API, probably best to just make that work? (rather than adding a Nullable helper or namespace.)

Could we add support for the nullable option inside Store, like this maybe?

class TypeBuilder {
    // ...
    Store(schema) {
        if (schema.nullable === true && typeof schema.type === "string") {
            schema = { ...schema, type: [schema.type, "null"] };
        }
        // ...
        return $schema;
    }
    // ...
}

@sinclairzx81
Copy link
Owner

@mindplay-dk Hi. Unfortunately OpenAPI's nullable can't be implemented as it's not part of the JSON Schema specification. I believe OpenAPI are deprecating nullable as of OpenAPI 3.1 where they have fully aligned JSON Schema drafts 2019/2020 specifications.

https://www.openapis.org/blog/2021/02/16/migrating-from-openapi-3-0-to-3-1-0

Note that Nullable types are possible with TypeBox using JSON schema representations. TypeBox constructs these from unions the same way TypeScript does. So for example, the follow three definitions are equivalent.

// typebox
const T = Type.Union([Type.String(), Type.Null()])

// typescript
type T = string | null

// schema
const T = {
    anyOf: [
       { type: 'string' },
       { type: 'null' }
    ]
}

Currently, TypeBox provides a fallback mechanism for OpenAPI prior to 3.1 where you can construct a custom schema representation for nullable. Example below and link here

import {  Type, Static, TSchema, TUnion, TNull } from '@sinclair/typebox'

/** Augments the given schema with the 'nullable' and returns a (T | null) */
function Nullable<T extends TSchema>(schema: T): TUnion<[TNull, T]> {
  return { ...schema, nullable: true } as any // facade
}

const T = Type.Object({
  email: Nullable(Type.String())
})

type T = Static<typeof T>

As for the type: ['string', 'null'] representation as per the stackoverflow link. TypeBox opts for anyOf as these generalize across all types (not just primitives) and closely align to TypeScript semantics for union.

I expect things will be a bit easier from Open API 3.1 and above.

Hope this helps
S

@mindplay-dk
Copy link

Note that Nullable types are possible with TypeBox using JSON schema representations.

Well, I saw that elsewhere, but as someone pointed out, these are not exactly equivalent to T | null.

For now, I went with this helper function:

function Nullable<T extends TSchema>(schema: T): TUnion<[T, TNull]>
function Nullable<T extends TSchema>(schema: any): any {
  if (typeof schema.type !== "string") {
    throw new Error(`This type cannot be made nullable`);
  }
  
  schema.type = [schema.type, "null"];

  return schema;
}

It's not what I proposed above, but it seems to work.

We might be able to merge these ideas?

So instead of the throw, it could use anyOf for more complex types? The arguments and return-type are the same.

It would be nice to have something that built-in that "just works".

@mindplay-dk
Copy link

(for the record, I'm not using Open API - this needs to be standard JSON schema.)

@sinclairzx81
Copy link
Owner

@mindplay-dk

Well, I saw that elsewhere, but as someone pointed out, these are not exactly equivalent to T | null.

Is there a functional difference between the following schema representations?

const T = { type: ['string', 'null'] }

and

const T = { anyOf: [{ type: 'string'}, { type: 'null' }] }

For composition sake, TypeBox opts for the latter (as it permits the union of two objects), resulting in the following.

const T = {
    anyOf: [
       { type: 'object', properties: { a: { type: 'string' } } },
       { type: 'object', properties: { b: { type: 'string' } } },
    ]
}

The above is not trivially expressible using type: [...].

If there is a functional disparity between anyOf and type: [...] I'd be happy to review and try and reconcile that in some way. But it wouldn't be a trivial change to make. I would expect some ambiguity and complexity appropriately resolving the correct TS type (i.e. should it resolve from anyOf or should it resolve from type: [...]?).

Open to your thoughts.
S

@mindplay-dk
Copy link

Is there a functional difference...

Good question.

My main concern was that quicktype.io might interpret these differently - but it doesn't appear to:

image

Ran it for a bunch of languages, and don't see any difference.

I guess the only practical reason to prefer type: ["string", "null"] would be human-friendliness. Even that is arguably questionable though - there's something to be said for just having a consistent output, and you will have to use anyOf for more complex types either way.

So yeah, I agree with your conclusion, I think. 🙂

Well, shouldn't we have a Nullable helper in there then? I realize it's a completely trivial thing - basically:

(t) => Type.Union([t, Type.Null()])

But it takes quite a bit of thinking to arrive at this conclusion. Might be better to just have it in there, so no one has to learn from documentation how to do it correctly? Must be a fairly common requirement.

@sinclairzx81
Copy link
Owner

@mindplay-dk

But it takes quite a bit of thinking to arrive at this conclusion. Might be better to just have it in there, so no one has to learn from documentation how to do it correctly? Must be a fairly common requirement.

Yeah I understand, and I do actually get asked this quite a lot; usually from Open API users seeking out ways to express nullable types. My reluctance to include something as default mostly stems from ensuring that TypeBox functions always produce valid JSON schema output (which rules out the nullable: true representation), and that TypeScript is able to adequately and efficiently resolve a corresponding type (leading to generalization via the anyOf representation). Getting the JSON schema representation and TS inference logic in tune has been a careful balancing act between ease of use and efficiency. It continues to be a work in progress.

With respect to (t) => Type.Union([t, Type.Null()]), it is helpful to think in terms of how the type would be expressed in TypeScript (so T | null in this case). A nullable T type is a composite union, so like TypeScript, TypeBox provides baseline primitives to compose such higher order types.

For example, TypeScript provides no type for Nullable<T> but can be comprised from a union.

type Nullable<T> = T | null

const Nullable = <T extends TSchema>(t: T) => Type.Union([t, Type.Null()])

I do actually intend to provide better documentation in the future around some of the more sophisticated things you can do with TypeBox with the TS analogs. I'm hoping to get something up before the end of the year.

Hope this helps
S

@dvv
Copy link

dvv commented Mar 19, 2024

Hi!

I wonder whether there's a way to make nullable (union-ish) schema checks more consistent?
E.g.

Value.Decode(Type.String(), 123) // ERR: Expected string -- GOOD
Value.Decode(Nullable(Type.String()), 123) // ERR: Expected union value -- IMO BAD

TIA

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

No branches or pull requests

5 participants