7.x nullable enum bug

[drwpow]

          Thank you for the great project @drwpow !

I am trying to migrate to the v7, I only noticed an issue with nullable enums.

When I run npx -y openapi-typescript@next test.json > test.ts on this spec:

{
    "openapi": "3.1.0",
    "info": {
      "title": "Minimal API with Nullable Enum",
      "version": "1.0.0"
    },
    "paths": {
      "/example": {
        "get": {
          "summary": "Get Example Value",
          "responses": {
            "200": {
              "description": "Successful response",
              "content": {
                "application/json": {
                  "schema": {
                    "type": "object",
                    "properties": {
                      "value": {
                        "type": "string",
                        "enum": ["option1", "option2", "option3"],
                        "nullable": true
                      }
                    }
                  }
                }
              }
            }
          }
        }
      }
    }
  }

I get:

For me value should be nullable: value?: "option1" | "option2" | "option3" | null;

Is it a bug ?

Originally posted by @gduliscouet-ubitransport in #1368 (comment)

[michaeljaltamirano]

@drwpow Isn't it the case that OpenAPI 3.1 no longer supports the nullable property, instead preferring "null" to be added to the type? https://www.openapis.org/blog/2021/02/16/migrating-from-openapi-3-0-to-3-1-0.

redocly in particular lints nullable: true as an invalid spec. https://redocly.com/docs/openapi-visual-reference/null/.

If I add null to the enum, it builds correctly, but redocly fails the spec validation. If I update type to be [string, "null"], represented in YAML as:

type: 
  - string
  - "null"

I do not get a spec error, but the output builds the same as the repro shared in this issue. Is there a design doc that explains how this project should treat null/nullable in OpenAPI 3.0 vs. 3.1?

[gduliscouet-ubitransport]

In fact it is difficult to find the right information. From my researsh I concluded that, in 3.1:

• using nullable is not recommended (but I don't think it is forbidden)
• adding "null" to the type is in fact the recommended approach
• for enum however you should have null listed in the enum and no type key

I got this mostly from this comment: OpenApi is based on JSON Schema, so looking at the enum doc in JSON Schema can help

{
  "enum": ["red", "amber", "green", null, 42]
}

[drwpow]

nullable: true and type: [null] should work the same way in both versions; any deviation is a bug. The latter is the “preferred” way according to the spec, but I frequently find a lot of 3.0 specs that still have nullable: true, and supporting it is fairly trivial so we allow it. If you have both in your schema and they disagree, we probably don’t handle that well, but that also gets into “ambiguous schema” territory and in some of those scenarios output may not be guaranteed.

[mattoni]

Is it incorrect to have
{ "type": ["string", "null], "enum": ["a", "b"] } ? Because I'm getting this in 6.7.5, where anything with a type of string/null and enum is just being set as string | undefined.

This:

generates this:

which doesn't seem correct to me.

[gduliscouet-ubitransport]

Again I think this is not clearly forbiden, but the recommanded approach in openapi 3.1 is to:

• not have a type key
• add null to the enum

So:

"role": {
  "enum": ["orchestrator", null]
}

[drwpow]

Fixed in #1644. Now that I think of it, I do recall a case where someone wanted:

type:
  - string
  - "null"
enum:
  - blue
  - green
  - yellow

to generate

"blue" | "green" | "yellow"

rather than

"blue" | "green" | "yellow" | null

based on the reasoning that “null wasn’t in the enum”. But I somewhat disagree with that, because with polymorphic types, the enum only applies to type: string; the additional type: null is a separate property and IMHO should be part of the final union (in other words, the enum probably isn’t intended to capture all possible values across all types, is it?). Since 7.x isn’t released yet, if the latter output is a breaking change from 6.x (I don’t recall off the top of my head), I think it’s an improvement.

Worse-case scenario, you have to handle the null case, and defensive programming isn’t a bad stance to take.