feat: document polymorphic [null, string] type non spec-compliant output

[toomuchdesign]

Description

I've recently run into a probably known issue related to polymorphic object definitions returning an unexpected empty string value in the resulting union type:

{
  "properties": {
    "petType": {
      "type": [
        "null",
        "string"
      ],
      "enum": [
        "option1",
        "option2",
        null
      ]
    }
  }
}

...resulting into:

petType?: "option1" | "option2" | "" | null; // "" should not be there

I've found this behaviour documented as expected in the tests:
https://github.com/drwpow/openapi-typescript/blob/c732542df136eee2f41d242321758a0901b45248/packages/openapi-typescript/test/schema-object.test.ts#L283

Proposal

I'm writing to ask:

• Is this behaviour intended?
• Why tests mention a "no fix"? is it so because of a technical limitation or any other reason?
• How do you suggest to deal with this unexpected output (patching nested structures of objects/arrays in TS is quite hard)
• In case fixing this is not possible, I'd be happy to document this somewhere.

Happy to open a PR in case maintainers decided to consider fixing this, instead.

Thanks for the outstanding work here!

Checklist

• I’m willing to open a PR for this (see CONTRIBUTING.md)

[drwpow]

Hm at this point, I can’t remember the exact reason for that. OpenAPI 3.x prefers nullable: true over the legacy ["string", "null"] typing. And while I think ["string", "null"] is technically invalid now and only worked for 2.x, I don’t see why we couldn’t provide a targeted fix for that as a QoL improvement (off the top of my head, I can’t see how that would generate undesirable types, or conflict with anything else, but will update my answer if that changes).

As a temporary workaround, again, nullable: true is probably preferred in general for 3.x schemas.

[tyronen9]

As a temporary workaround, again, nullable: true is probably preferred in general for 3.x schemas.

It's not:

In line with JSON Schema, the type keyword can now define multiple types for a schema with an array. This is useful new functionality, but has also made nullable redundant. In order to achieve the goal of letting JSON Schema tools understand OpenAPI, it was decided to remove nullable entirely instead of deprecate it.

# OpenAPI v3.0
type: string
nullable: true

# OpenAPI v3.1
type:
- "string"
- "null" 

However, if using openapi-typescript, this changes generated code from:

fieldname: string | null

to

fieldname: OneOf<[string, Record<string, never>]>

We may want a separate issue for this, but why does null map to Record<XX, never> instead of simply null? It means I have to change type definitions all over my codebase to migrate to 3.1. And some of these fields are written to databases, and they really need a value of null.

[toomuchdesign]

Beside nullable turning out not to be future proof (I'm learning this now), I believe that the other main issue about it is that most of openapi-typescript might not control the OpenApi definition source (unless of manually fixing it at each update).

@tyronen9 The example you mention is about prop with type prop as [...] which are NOT enum, right?

If so it might worth opening another issue/PR extending the tests I've added in this PR which tries to address the initial issue.

About ["string", "null"] being invalid, I'm definitely not an expert but this JSON online validator seems to be able to perfectly understand it.

@drwpow If you decided to consider this PR I'd happy to extend its test to make sure we don't introduce regression bug. Feel free to comment it and propose extra test cases.

[drwpow]

This looks good to me. I believe if this does change behavior for some people, it’s a desirable change. I’m not aware off the top of my head of any regressions this would likely cause. Overall this seems like an improvement.