[client-fetch] Wrong type prop generation

[blouflashdb]

Description

Wrong generation of type that has allOf but no additionalProps.

Generates:

export type BikeModel = ModelBase & {
  type: "Bike";
} & {
  [key: string]: never;
};

But I would expect:

export type BikeModel = ModelBase & {
  type: "Bike";
} & {};

Reproducible example or configuration

Config:

import { defineConfig } from "@hey-api/openapi-ts";

export default defineConfig({
  input: "./swagger.json",
  output: {
    lint: "eslint",
    path: "src/api-clients-new"
  },
  plugins: ["@hey-api/client-fetch"]
});

OpenAPI specification (optional)

"ModelBase": {
        "required": [
          "type"
        ],
        "type": "object",
        "properties": {
          "type": {
            "$ref": "#/components/schemas/VehicleType"
          }
        },
        "additionalProperties": false,
        "discriminator": {
          "propertyName": "type",
          "mapping": {
            "Car": "#/components/schemas/CarModel",
            "Bike": "#/components/schemas/BikeModel",
            "Truck": "#/components/schemas/TruckModel"
          }
        }
      }, 
      "BikeModel": {
        "allOf": [
          {
            "$ref": "#/components/schemas/ModelBase"
          },
          {
            "type": "object",
            "additionalProperties": false
          }
        ]
      },

System information (optional)

No response

[mrlubos]

How does this affect your experience @blouflashdb? I remember there was a reason I made it like this, and it might've been due to compiler or certain cases like if the empty object is the only element in type, it actually allows everything

[danieltott]

The current version is absolutely more correct. Your schema implies that there are no additional properties allowed, which is what the outputted type is saying.

Empty objects should never be used as a type, and although this doesn't specifically apply when it's combined inline, the current behavior (using [key: string]: never;) of the library is correct.

Imagine if you had that last object as a ref:

{
  "ModelBase": {
    "required": [
      "type"
    ],
    "type": "object",
    "properties": {
      "type": {
        "$ref": "#/components/schemas/VehicleType"
      }
    },
    "additionalProperties": false,
    "discriminator": {
      "propertyName": "type",
      "mapping": {
        "Car": "#/components/schemas/CarModel",
        "Bike": "#/components/schemas/BikeModel",
        "Truck": "#/components/schemas/TruckModel"
      }
    }
  }, 
  "NoAdditionalProperties": {
    "type": "object",
    "additionalProperties": false
  },
  "BikeModel": {
    "allOf": [
      {
        "$ref": "#/components/schemas/ModelBase"
      },
      {
        "$ref": "#/components/schemas/NoAdditionalProperties"
      }
    ]
  }
}

Right now the lib outputs NoAdditionalProperties as

export type NoAdditionalProperties = {
  [key: string]: never;
};

With your suggested change, it would output as

export type NoAdditionalProperties = {};

Which would result in this being valid code:

type EmptyObject = {}

type NoKeysObject = {
  [key: string]: never;
}

// These are all valid as far as ts is concerned
const ThisIsTechnicallyAnObjectBecauseJavascript: EmptyObject = "abc";
const SoIsThis: EmptyObject = "";
const AndThis: EmptyObject = 0;

// these will throw errors
const TsSavesUsHere: NoKeysObject = "abc";
const TsSavesUsHereToo: NoKeysObject = 0;

That's not what we want. Here's a ts playground with the difference.

[mrlubos]

Wow you went the extra mile @danieltott, that's correct 🙏

[danieltott]

Yeah I saw this when looking for another issue and really didn't want it to get accepted 😅