Union with null is not supported

[atomicus]

TSOA fails on generating routes when interface includes 'null' type.

• I'm submitting a ...

  • bug report
  • feature request
  • support request

• I confirm that I

  • used the search to make sure that a similar issue hasn't already been submit

Expected Behavior

Generate propper swagger for empty-valued patams

Current Behavior

Error is presented:

Generate routes error.
 Error: Unknown type: NullKeyword
    at new GenerateMetadataError (/Users/atom/Projekty/atom/107/dcs-api/node_modules/tsoa/dist/metadataGeneration/exceptions.js:20:28)
    at TypeResolver.resolve (/Users/atom/Projekty/atom/107/dcs-api/node_modules/tsoa/dist/metadataGeneration/typeResolver.js:153:19)
    at /Users/atom/Projekty/atom/107/dcs-api/node_modules/tsoa/dist/metadataGeneration/typeResolver.js:567:152
    at Array.map (<anonymous>)
    at TypeResolver.getModelProperties (/Users/atom/Projekty/atom/107/dcs-api/node_modules/tsoa/dist/metadataGeneration/typeResolver.js:555:18)
    at TypeResolver.getReferenceType (/Users/atom/Projekty/atom/107/dcs-api/node_modules/tsoa/dist/metadataGeneration/typeResolver.js:352:35)
    at TypeResolver.resolve (/Users/atom/Projekty/atom/107/dcs-api/node_modules/tsoa/dist/metadataGeneration/typeResolver.js:202:30)
    at /Users/atom/Projekty/atom/107/dcs-api/node_modules/tsoa/dist/metadataGeneration/typeResolver.js:567:152
    at Array.map (<anonymous>)
    at TypeResolver.getModelProperties (/Users/atom/Projekty/atom/107/dcs-api/node_modules/tsoa/dist/metadataGeneration/typeResolver.js:555:18)

Steps to Reproduce

1. Create interface in eg:

interface IServerSessionDetails {
   id: number,
   name: string,
   emptyVal: string | null
}

2.run routes generation

Context (Environment)

Version of the library: 2.5.2
Version of NodeJS: 8.15 & 10.15

• Confirm you were using yarn not npm: [x]

[dgreene1]

Hi @atomicus. Out of curiosity, why have you chosen to represent the type as EmptyVal: string | null when emptyVal?: string is more typical? Btw, I’ll mark this as “help wanted” so that contributors know that this is free game.

cc’ing @WoH since he and I have discussed this use case before, but we were both interested in learning more about the use case from users. So any context you can provide about the usefulness would be helpful @atomicus.

[WoH]

This also applies to unions with undefined.

To add to @dgreene1's points, while there is the concept of (x-)nullable in swagger, it won't always work as intended. While you could mark a property nullable, this won't work for more complex types, i.e.:

---
UnionModel:
  description: Contains a nullable union (ideally)
  properties:
    and:
      allOf:
      - "$ref": "#/components/schemas/TypeAliasModel1"
      - "$ref": "#/components/schemas/TypeAliasModel2"
      nullable: true

will not actually work. Even

---
UnionModel:
  description: Contains a nullable union (ideally)
  properties:
    and:
      allOf:
      - "$ref": "#/components/schemas/TypeAliasModel1"
        nullable: true
      - "$ref": "#/components/schemas/TypeAliasModel2"
        nullable: true

will not work for now. As far as I'm aware, this should work in a future version of json schema that will eventually be adopted by OpenAPI. Once that's there I'll take another look.

This is why I decided to not work on this for now, however, I think it's great to have an issue to refer to.

[dgreene1]

@WoH why won’t the second case work?

[WoH]

https://tools.ietf.org/html/draft-wright-json-schema-00#section-7

[scttcper]

When working with relational db i don't think last_name?: string is more common. I think many people would have last_name: string | null because most databases and orm's use null as a default instead of an empty string. We've got some services stuck using v2.4.10 because they now error on NullKeyword and whatever the error for using "unknown" is.

I'm going to try working on fixing NullKeyword if only for simple use cases like string | null. Honestly, I'd take this field reverting to "any" instead of erroring.

[dgreene1]

When working with relational db i don't think last_name?: string is more common

I’d encourage you and anyone reading this keep your tsoa types separate from your database types. It’s incredibly valuable for software to use abstractions. This allows your database to change and evolve while keeping your contract the same for consumers.

I realize that I’ve just given unsolicited architectural advice, so please forgive me.

But it’s still unclear to me how we can support string | null when Swagger/OpenAPI doesn’t support it.

I think at best we can log a warning to console that says, “please use string | undefined instead.” Because that’s the only “nullable” concept that Swagger/OpenAPI supports. And it’s part of our design goals to only validate types that can be communicated with OpenAPI. When/if OpenAPI supports string | null I will be the first to push for support with tsoa.

[dgreene1]

So what do you think of us having a more helpful error message that says, unions with null are not yet supported. Please use a union of undefined instead.

[scttcper]

keep your tsoa types separate from your database types

that's definitely good advice, unfortunately i think these other teams haven't done that and now they are keen on using the older tsoa version forever until it explodes in classic tech debt style.

I don't think a different error would be that helpful in this case as the existing error already links to the type and the line which it was found and that's really helpful. Really, they'd probably say they don't care as the swagger docs that they currently have via older tsoa, while wrong, are still better than most other services.

Your comment had me go and look a bit more at what the spec actually is and now i'm probably in favor of changing the public interfaces. gah...

[mgatorr]

We find a "solution" for this OpenApi problem:

interface SomeModel {
  nullableField: NullString
}

type NullString = string | null

This not the best or more clean solution but work for us, but swagger don't represent well the model

[WoH]

This will not work in tsoa v3.x since we are properly able to resolve type aliases.
My best alternative at this point is to type null as:

nullableProperty:
  type: boolean
  enum:
    - null
  nullable: true

While I think this is not ideal, it allows typing null as an isolated type, which is pretty important to avoid a lot of corner cases during type resolution.
Also, it would allow differentiating properly betweenundefined (or optional properties) and null on a required property.
Wdyt @dgreene1?

[dgreene1]

I’m really not sure. I guess I don’t enough to have a strong opinion. I think we might need outside input. Maybe we can tag someone from the OpenAPI spec?

[WoH]

I can add some more context for now, feel free to tag someone:

OpenAPI v3.0.3. adds language that clarifies that #479 (comment) won't work (OAI/OpenAPI-Specification#2046)

Since the behavior around null is not clearly specified, there is a proposal to fix: https://github.com/OAI/OpenAPI-Specification/blob/master/proposals/003_Clarify-Nullable.md

Enum with null is supported by the underlying json-schema draft 00:

5.20.  enum
   The value of this keyword MUST be an array.  This array SHOULD have
   at least one element.  Elements in the array SHOULD be unique.

   Elements in the array MAY be of any type, including null.

   An instance validates successfully against this keyword if its value
   is equal to one of the elements in this keyword's array value.

OpenAPI says JSON-Schema behavior should be adopted unless explicitly stated otherwise:

The Schema Object allows the definition of input and output data types. These types can be objects, but also primitives and arrays. This object is an extended subset of the JSON Schema Specification Wright Draft 00.
For more information about the properties, see JSON Schema Core and JSON Schema Validation. Unless stated otherwise, the property definitions follow the JSON Schema.

In the list of properties taken from the JSON Schema definition with adjusted definitions, enum is not mentioned.

[dgreene1]

if you think that would work then I guess we could give it a go.

My only hesitation is that it looks strange to me that the enum only has one type in it.

Ultimately the best way to gain confidence in this is to try to compile that schema back into TypeScript using some popular converter. If it becomes string | null then we know we’re doing it right.

[WoH]

Swagger Core works: swagger-api/swagger-core#3100 / swagger-api/swagger-core#3098

Supposedly it's fixed in OpenAPI generator: OpenAPITools/openapi-generator#1997, however the bugfix only addresses python as far as I can tell and that one crashed on me when I did null on any enum and tried to generate code for any language.

[eggysplat]

We find a "solution" for this OpenApi problem:

interface SomeModel {
  nullableField: NullString
}

type NullString = string | null

This not the best or more clean solution but work for us, but swagger don't represent well the model

I know this is a bit of an older issue. However, as a workaround I created a wrapper generic type for a nullable, which works fine, and sets nullable: true, or x-nullable: true in v2 spec.

The code I used:

type Nullable<T> = T | null;

interface SomeModel {
    nullableField: Nullable<string>
}

You can use that with any type / interfaces as well where a union with null is required.

[WoH]

See: #479 (comment)
and: #479 (comment)

I think this is a fine hack for now, I'll make sure 3.0 properly documents null so it doesn't cause too many issues. However, please be aware that the documentation and validation of Nullable<string> would be wrong in 2.5

[fantapop]

I'm having issues with this in our build. I'm unable to create an interface using | null in my types and then generate a remote client using swagger codegen for the typescript-fetch generator. I think I probably need to bug it in openapi-generator but i wanted to raise it here as well. What I'm seeing is that in order to represent the nullable enum type, the openapi-generator is creating code that looks like this:

'inspiration': string | numberToJSON(value.inspiration),

But numberToJSON is not included in the file and creating this error:

error TS2304: Cannot find name 'numberToJSON'.

The same thing is showing up for numberFromJSON.

Is the enum representation of null supposed to provide a way to represent undefined in addition to null? I'm wondering what the benefit of it is over using the nullable: true on the string in this case?

[WoH]

The reason is that nullable:true can only exist on a schema with a type.
TS allows null to exist on it's own:

interface A {
  prop: null;
} 

What do you set there?

Another example:
Referenced types can't be nullable: true, but that's what we'd need to do if you want to ref to a Model | null. Issue is even worse with Unions:

type A = B | C | D | null;

Where do we set nullable: true?

[fantapop]

I understand, do you agree that this is likely an issue in open api generator? I can open an issue there and see If I can get any traction. Until then we're unfortunately going to be stuck on 3.0.2.

[WoH]

I'd say so, yes.

[fantapop]

I had a chance to poke around this a bit more. I looked over the swagger-codegen repo. My assumption is that a fix from them will be daunting to achieve. There are a few reasonable seeming PRs against the typescript code generation that have been sitting there unattended for a long time.

The bug on swagger-codegen side that causes this to fail is that using oneOf appears to generate invalid typescript.

I think there is also a bug on the tsoa side. the enum which is joined with the value as a oneOf, contains the string "null" instead of the null value. In the section on Nullable Enums here, it states: without quotes, i.e. null not "null". Doing it in this way however is considered typescript error when strict null checks are enabled (which seems like it might be its own bug).

I will file an issue in swagger-codegen about supporting oneOf correctly. As mentioned, even If I can figure out how to fix it and make a PR, it's possible it may not be accepted.

How difficult it would be to support the more straightforward nullable: true syntax for cases which support it? i.e { "type": "string", "nullable": true }
It generates the correct types and works correctly with required as well. I think this would cover 95% of cases users need.

[WoH]

I think there is also a bug on the tsoa side. the enum which is joined with the value as a oneOf, contains the string "null" instead of the null value. In the section on Nullable Enums here, it states: without quotes, i.e. null not "null". Doing it in this way however is considered typescript error when strict null checks are enabled (which seems like it might be its own bug).

If you could open a separate issue for this, I'd add it to the 3.0 backlog.

How difficult it would be to support the more straightforward nullable: true syntax for cases which support it? i.e { "type": "string", "nullable": true }
It generates the correct types and works correctly with required as well. I think this would cover 95% of cases users need.

CBA. If someone opens a PR that handles this in a reasonable way (only affecting the SpecGenerator), I'd be potentially open for that.

[fantapop]

@WoH

If you could open a separate issue for this, I'd add it to the 3.0 backlog.
#668

[fantapop]

The bug on swagger-codegen side that causes this to fail is that using oneOf appears to generate invalid typescript.

It turns out I'm not even using swagger-codegen. I was having a really hard time tracking down what was going wrong, it all make sense now. I'm actually using the openapi-generator to generate clients in each of my projects. It looks like they already have an issue tracking this there:

OpenAPITools/openapi-generator#5202

It doesn't look like its been acknowledged yet.

[fantapop]

A couple items of note. IMO a more intuitive representation of null in openapi v3 is described here:
OAI/OpenAPI-Specification#1368 (comment)

It may be a moot point however because I see that nullable is getting deprecated in v3.1.0 and a type = "null" is being added. At the point this comes out, this seems to be the obvious choice to use. See here: https://github.com/OAI/OpenAPI-Specification/blob/v3.1.0-dev/versions/3.1.0.md#fixed-fields-20

[fantapop]

@WoH I found a way to add support for nullable refs without oneOf. Using anyOf: [ TheRef ], nullable: true works with openapi generator. The anyOf construct is more universally support across most open api templates so I think this is a better option. I submitted a PR to add support for this. I just wanted to mention it here for more context. #734

[xianny]

Bumping this thread now that the "null" type has been added to OpenAPI.

An example model of:

SomeModel {
    nullableField: string | null;
}

Should generate a spec of:

{
"type": "object"
"properties": {
    "nullableField": {
        "type": ["string", "null"]
    }
}

[alexellison]

Seconding the above bump now that the null type exists in OpenAPI 3.1.

[atomicus]

@dgreene1

Sorry for not checking this for such a long time.

I'm not using the ? operator because I want my response to be strict and full even if value is null.

So, lets say we have interface:

interface IServerSessionDetails {
   id: number,
   name: string,
   emptyVal: string | null
}

According to this description, this will result in REST response of format:

{
 id: 1
 name: "NameValue"
 emptyVal: null 
}

While

interface IServerSessionDetails {
   id: number,
   name: string,
   emptyVal?: string 
}

Will result in:

{
 id: 1
 name: "NameValue"
}

So, those responses will not be the same. At least this is how I and most of the devs I know will react to that kind of typing.
? indicates the property is optional and might be not present in the serialized response. This means the de-serializer has to be prepared that $.emptyVall might be undefined and not exists.
null|string ensures that prop is in the response, and the de-serializer does not have to check for the existence of prop, just for its nullability or not.

This is how it works for example with https://github.com/typestack/class-transformer.

Property marked as ? can be undefined, thus will not exist in serialized object
Propertyt typed as |null will be serialized as =null.

There's just a difference between null, which is some kind of information, and no information.