openapi-generator icon indicating copy to clipboard operation
openapi-generator copied to clipboard

[BUG] typescript-fetch since 7.4.0 ignores nullable: true

Open tfilo opened this issue 1 year ago • 6 comments

Bug Report Checklist

  • [x] Have you provided a full/minimal spec to reproduce the issue?
  • [x] Have you validated the input using an OpenAPI validator (example)?
  • [ ] Have you tested with the latest master to confirm the issue still exists?
  • [x] Have you searched for related issues/PRs?
  • [x] What's the actual output vs expected output?
  • [ ] [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

Hello since update to 7.4.0, nullable: true anotation is ignored and typescript generated interface doesn´t include "| null"

openapi-generator version

7.4.0

OpenAPI declaration file content or url
openapi: 3.0.1
info:
  title: TEST
  description: TEST
  version: 1.0.0
servers:
  - url: /
    description: Default Server URL
paths:
  /api/test:
    post:
      tags:
        - Test
      operationId: test
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/ResponseObject'
      responses:
        '200':
          description: OK
          content:
            '*/*':
              schema:
                $ref: '#/components/schemas/ResponseObject'

components:
  schemas:
    ResponseObject:
      type: object
      properties:
        code:
          type: integer
          nullable: true
Generation Details

I generate it using npm package: "@openapitools/openapi-generator-cli": "^2.13.1". With command npx openapi-generator-cli generate with following configuration

{
    "$schema": "./node_modules/@openapitools/openapi-generator-cli/config.schema.json",
    "spaces": 4,
    "generator-cli": {
        "version": "7.4.0",
        "useDocker": true,
        "generators": {
            "v3.0": {
                "generatorName": "typescript-fetch",
                "glob": "openapi/*.{json,yaml}",
                "output": "src/openapi/#{name}",
                "typeMappings": {
                    "Date": "string"
                },
                "additionalProperties": {
                    "prefixParameterInterfaces": true,
                    "useSingleRequestParameter": false,
                    "supportsES6": true
                }
            }
        }
    }
}
Steps to reproduce

If you generate source code from this provided yaml with 7.4.0 you will get generated output where ResponseObject.ts is defined like this:

export interface ResponseObject {
    /**
     * 
     * @type {number}
     * @memberof ResponseObject
     */
    code?: number;
}

but with all previouse versions it was like this

export interface ResponseObject {
    /**
     * 
     * @type {number}
     * @memberof ResponseObject
     */
    code?: number | null;
}

It looks like version 7.4.0 started to ignore nullable: true anotation.

Related issues/PRs

Maybe related to this issue ? #18005

Suggest a fix

Output should look like this:

export interface ResponseObject {
    /**
     * 
     * @type {number}
     * @memberof ResponseObject
     */
    code?: number | null;
}

tfilo avatar Apr 04 '24 08:04 tfilo

i wonder if you can do a git bisect to identify the commit causing the regression?

wing328 avatar Apr 05 '24 02:04 wing328

I will try my best to find some time for that but not sure when.

tfilo avatar Apr 16 '24 07:04 tfilo

Heya, just to save you a bit of time: this issue is not related to #18005.

#18005 was created by the changes in #17934 which only changed a mustache file used for the aspnetcore generator.

ghost avatar Apr 25 '24 02:04 ghost

I believe this PR broke it: https://github.com/OpenAPITools/openapi-generator/pull/17798

l0gicgate avatar May 03 '24 15:05 l0gicgate

Was this change intended? This breaks our PATCH requests as we distinguish between undefined and null in the api.

matthiasschwarz avatar May 21 '24 17:05 matthiasschwarz

@l0gicgate is correct that #17798 introduced this bug, specifically this change (and the others like it throughout the same PR).

To JavaScript and TypeScript, null and undefined are not equivalent. In a TypeScript object type, a ? marks a property as optional, meaning an object of that shape may not have the property and, consequently, accessing the property may return undefined.

If you'd like to denote a property as also being able to hold a null value, you have to explicitly annotate a type as such:

interface Foo {
  nullableField: string | null;
}

// Fine: can be `null`
const foo1: Foo = { nullableField: null };
// Error: cannot be `undefined`
const foo2: Foo = { nullableField: undefined };
// Error: field has to be present
const foo3: Foo = {};

Technically if a field is expected to be present but could hold an undefined value it would look like:

interface Foo {
  presentButMaybeUndefined: string | undefined;
}

// Fine: can be `undefined`
const foo1: Foo = { presentButMaybeUndefined: undefined };
// Error: field wasn't optional (`?`) so it has to be present
const foo2: Foo = {};

I think the most correct way to capture all possible use cases if a field is optional (in the OpenAPI sense) is to annotate it field with both ? and an explicit undefined:

interface Foo {
  couldBeUndefined?: string | undefined;
}

// Fine: field is optional
const foo1: Foo = {}
// Fine: field can explicitly hold `undefined`
const foo2: Foo = { couldBeUndefined: undefined };

However, if the types prior to #17798 were working then it's probably fine to just revert the change and avoid possibly causing new bugs by explicitly adding undefined to optional fields.


The correct behavior should be:

interface Foo {
  requiredNonNullable: string;
  requiredNullable: string | null;
  optionalNonNullable?: string;
  optionalNullable?: string | null;
}

bthall16 avatar May 24 '24 15:05 bthall16

this issue is addressed here: https://github.com/OpenAPITools/openapi-generator/pull/18887

hendrikpeilke avatar Jun 11 '24 10:06 hendrikpeilke