Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AdditionalProperties in swagger definition creates broken TS interface #525

Open
ph3b opened this issue Nov 26, 2021 · 9 comments
Open

AdditionalProperties in swagger definition creates broken TS interface #525

ph3b opened this issue Nov 26, 2021 · 9 comments

Comments

@ph3b
Copy link

ph3b commented Nov 26, 2021

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

@horiuchi
Copy link
Owner

@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?

@panzors
Copy link

panzors commented Dec 14, 2021

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": {}
          }
        }
      },

@npdev453
Copy link

npdev453 commented Jan 7, 2022

@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;
    ...

@horiuchi
Copy link
Owner

horiuchi commented Jan 7, 2022

@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.

@tdeo
Copy link

tdeo commented Jul 12, 2023

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;
  }

@horiuchi
Copy link
Owner

horiuchi commented Jul 18, 2023

@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.

@tdeo
Copy link

tdeo commented Jul 26, 2023

@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?

@horiuchi
Copy link
Owner

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
Copy link
Owner

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants