Use exists() to check for properties

[noordawod]

Use exists() function to test for property existence on dictionaries/Maps rather than sprinkling the code with json.key === null || json.key === undefined.

In addition, the code removes the implicit null type of optional and required properties because in the case of TS/JS, restricting the property to only null means that it won't be able to receive the undefined value too, which in the case of JSON, ultimately resolves to null in transport.

export interface SomeInterface {
  requiredNullableProperty: string | null,
  requiredProperty: string,
  nullableProperty?: string,
}

The last line in the example covers the situation when you may pass in a value that is either null, undefined, or indeed a string. If you restrict it only to string | null, then undefined won't be a plausible value, which it should.

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package 
  ./bin/generate-samples.sh ./bin/configs/*.yaml
  ./bin/utils/export_docs_generators.sh
  

  (For Windows users, please run the script in Git BASH)
  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
• File the PR against the correct branch: master (upcoming 7.1.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) @topce (2018/10) @akehir (2019/07) @petejohansonxo (2019/11) @amakhrov (2020/02) @davidgamero (2022/03) @mkusaka (2022/04)

[noordawod]

Calling all codobots 🤖

[tryforceful]

It seems to me that maybe something in this PR has introduced an unexpected behavior?

I have an OpenAPI schema definition as such:

{
  "components": {
    "schemas": {
	  "MyObject": {
		  "properties": {
		    "backtoken": {
		      "title": "Backtoken",
		      "maxLength": 500,
		      "type": "string",
		      "nullable": true
		    },
		    "skiptoken": {
		      "title": "Skiptoken",
		      "maxLength": 500,
		      "type": "string",
		      "nullable": true
		    }
		  },
		  "type": "object"
	  }
	}
  }
}

This is being generated with the typescript-fetch generator as:

export interface MyObject { 
    backtoken?: string; 
    skiptoken?: string;
}

Even though the openapi spec has an explicit mention of nullable: true.

Shouldn't it still honor that nullable: true property?

[noordawod]

It seems to me that maybe something in this PR has introduced an unexpected behavior?

I have an OpenAPI schema definition as such:

{
  "components": {
    "schemas": {
	  "MyObject": {
		  "properties": {
		    "backtoken": {
		      "title": "Backtoken",
		      "maxLength": 500,
		      "type": "string",
		      "nullable": true
		    },
		    "skiptoken": {
		      "title": "Skiptoken",
		      "maxLength": 500,
		      "type": "string",
		      "nullable": true
		    }
		  },
		  "type": "object"
	  }
	}
  }
}

This is being generated with the typescript-fetch generator as:

export interface MyObject { 
    backtoken?: string; 
    skiptoken?: string;
}

Even though the openapi spec has an explicit mention of nullable: true.

Shouldn't it still honor that nullable: true property?

This was mentioned and clarified before in this comment: #17798 (comment)

[nielsvanvelzen]

This change breaks the OpenAPI spec. There is a difference between a nullable and optional property. With the release of 7.4.0 all properties that can be null (a value) are just made optional (omitted from request/response).

[hendrikpeilke]

@noordawod still getting back on this, reading carefully through the comments and reviews.

Your sentencte

The last line in the example covers the situation when you may pass in a value that is either null, undefined, or indeed a string. If you restrict it only to string | null, then undefined won't be a plausible value, which it should.

regarding this line

nullableProperty?: string,

I guess you're seeing the "pass in" from an API perspective, right? null, undefined or a string coming in from outside and is beeing parsed.

Seeing it the other way around, wanting to be able to make the resulting json in a request differentiate between null, undefined and a string is not possible with this solution, since typescript won't let you write nullableProperty = null;. Is that intended? I think it should be possible to assign null to a property defined as nullable in the openapi spec.

Other generators like sping, support differentiating between undefined, null and a string value. How would I achieve this by using the typescript-fetch-operator after this fix?

[l0gicgate]

It appears that this PR has broken nullable types. null and undefined are two different things in TS.

Take the instance where we do a PATCH request and I want to partially update an entity. That means I would be able to pass in a partial object with subset of the fields.

Here's a definition of what a patch could look like, you can optionally pass any of the parameters. In this case, the subTitle field is also nullable, which would make it null in the database if I wanted to erase it:

export interface ArticlePatch {
    title?: string;
    subTitle?: string | null;
}

Scenario 1: Patching title only, I don't pass in subTitle

PATCH /articles/1
{
  "title": "My title",
}

Scenario 2: Patching subTitle only, making it null. Now I can't do this, because the types are broken:

PATCH /articles/1
{
  "subTitle": null,
}

subTitle is nullable: true in swagger.json:

{
  "subTitle": {
    "type": "string",
    "nullable": true
  }
}

In previous versions 7.3.0 and down the output was this:

export interface ArticlePatch {
    title?: string;
    subTitle?: string | null;
}

Since this PR has been merged:

export interface ArticlePatch {
    title?: string;
    subTitle?: string; // No longer nullable
}