dtsgenerator icon indicating copy to clipboard operation
dtsgenerator copied to clipboard

AdditionalProperties in swagger definition creates broken TS interface

Open ph3b opened this issue 2 years ago • 9 comments

Hi!

First of all, great library! Have been using it for a long time and saves so much time aligning contracts between frontend and backends. Great work!

Today I stumbled upon a weird bug while upgrading rom v2 -> v3.

In my swagger definition I have the following object:

Microsoft.AspNetCore.Mvc.ProblemDetails": {
        "required": [
          "detail",
          "instance",
          "title",
          "type"
        ],
        "type": "object",
        "properties": {
          "type": {
            "type": "string"
          },
          "title": {
            "type": "string"
          },
          "status": {
            "type": "integer",
            "format": "int32",
            "nullable": true
          },
          "detail": {
            "type": "string"
          },
          "instance": {
            "type": "string"
          }
        },
        "additionalProperties": {
          "type": "object",
          "additionalProperties": false
        }
      }
}

This get's "translated" to the following TS interface:

export interface MicrosoftAspNetCoreMvcProblemDetails {
    [name: string]: {
      [key: string]: any;
    };
    type: string;
    title: string;
    status?: null | number; // int32
    detail: string;
    instance: string;
  }

Which TypeScript immediately complains about with the following message: Property 'title' of type 'string' is not assignable to 'string' index type '{ [key: string]: any; }'.ts(2411)

image

Using: dtsgenerator: 3.13.2 typescript: 4.4.4

ph3b avatar Nov 26 '21 09:11 ph3b

@ph3b Thank you for your report. This is certainly not a good result for the TypeScript specification. However, I do not currently have a good idea of what to do. What do you think would be the best outcome in this case?

horiuchi avatar Nov 27 '21 15:11 horiuchi

Hi, I'm using this from a c# side. I'm going to hack my instance to the following [name: string]: any;

My generated snippet is

export interface ProblemDetails {
            [name: string]: null;
            type?: string | null;
            title?: string | null;
            status?: null | number; // int32
            detail?: string | null;
            instance?: string | null;
            extensions?: {
                [name: string]: any;
            } | null;
        }

the doc is

"ProblemDetails": {
        "type": "object",
        "additionalProperties": {
          "nullable": true
        },
        "properties": {
          "type": {
            "type": "string",
            "nullable": true
          },
          "title": {
            "type": "string",
            "nullable": true
          },
          "status": {
            "type": "integer",
            "format": "int32",
            "nullable": true
          },
          "detail": {
            "type": "string",
            "nullable": true
          },
          "instance": {
            "type": "string",
            "nullable": true
          },
          "extensions": {
            "type": "object",
            "nullable": true,
            "additionalProperties": {}
          }
        }
      },

panzors avatar Dec 14 '21 00:12 panzors

@horiuchi looks like there is no solution, only add unknown or any type to repair compilation:

export interface MicrosoftAspNetCoreMvcProblemDetails {
    [name: string]: {
      [key: string]: any;
    } | unknown;
    ...

npdev453 avatar Jan 07 '22 06:01 npdev453

@npdev453 It is true that there seems to be no solution for the current TypeScript type system, but adding unknown or any would overwrite the type, making the typeset practically meaningless.

horiuchi avatar Jan 07 '22 16:01 horiuchi

Hello, I'm stumbling against something similar and found this issue,

I believe in my case what would work best in case of having a schema:

type: object
additionalProperties: false

would be to generate the following Typescript:

Record<any, never> 
// or alternatively: 
{ [key: string]: never }

In this case, since the pattern already appears within additionalProperties of MicrosoftAspNetCoreMvcProblemDetails, I believe we should end up with

export interface MicrosoftAspNetCoreMvcProblemDetails {
    [name: string]: { [key: string]: never };
    type: string;
    title: string;
    status?: null | number; // int32
    detail: string;
    instance: string;
  }

tdeo avatar Jul 12 '23 09:07 tdeo

@tdeo Thank you for your contribution.

However, that change does not solve this issue. The reported error(ts(2411)) is still there. I appreciate that you sent me a PR, but I cannot merge with #555.

horiuchi avatar Jul 18 '23 05:07 horiuchi

@tdeo Thank you for your contribution.

However, that change does not solve this issue. The reported error(ts(2411)) is still there. I appreciate that you sent me a PR, but I cannot merge with #555.

Hey @horiuchi,

I indeed got a bit mixed up and #555 doesn't quite fix this issue, which apparently has more to do with TS way of having generic + specific keys in an object.

I do still believe that the PR still adresses a real issue of having incorrect TS generated for the schema:

type: object
additionalProperties: false

and that it should get fixed. Should I first open an issue for this? Would you like to see anything else to get it merged?

tdeo avatar Jul 26 '23 22:07 tdeo

Hi @tdeo,

Since TypeScript interfaces are essentially open-ended, we cannot express that They do not have additional properties. https://basarat.gitbook.io/typescript/type-system/interfaces#interfaces

In the case of { [key: string]: never }, we cannot express that there are no properties, but only that all properties are of the type never. This means that the following properties can be added.

function f(): never { throw new Error() }
interface A {
  [name: string]: never;
}
const a: A = {
  test: f(),
};

So, I think it is better to ignore additionalProperties: false until TypeScript's type expressiveness improves.

horiuchi avatar Jul 27 '23 02:07 horiuchi

@npdev453 I have just tried your idea and found it to be the ideal solution. The correct type was inferred for the properties that existed as follows, and for the other properties, the type was unknown.

When I tried this in the past, all the properties were unknown, but I wonder if the result has changed due to TypeScript updates. At any rate, this seems to work fine.

export interface MicrosoftAspNetCoreMvcProblemDetails {
  [name: string]: { [key: string]: any; } | unknown;
  type: string;
  title: string;
  status?: null | number; // int32
  detail: string;
  instance: string;
}

const x: MicrosoftAspNetCoreMvcProblemDetails = {} as any;
let a = x['type'];
//  ^? string
let b = x.status;
//  ^? number | null | undefined
let c = x['foo'];
//  ^? unknown

horiuchi avatar Jul 27 '23 02:07 horiuchi