Skip to content

[BUG][typescript-fetch] Generating incorrect types #3869

Open

Description

@keean

When using openapi-generator to generate a typescript-fetch API from this specification:

openapi-generator generate -i https://api.demo.kaizenep.com/swagger.json -g typescript-fetch -o kaizen-api

The generated code has incorrect types where "List" is referenced. For example line 182 and 219 of output apis/EventsApi.ts

async postEventSearchRaw(requestParameters: PostEventSearchRequest): Promise<runtime.ApiResponse<Array>> {
async postEventSearch(requestParameters: PostEventSearchRequest): Promise<Array> {

This should be:

async postEventSearchRaw(requestParameters: PostEventSearchRequest): Promise<runtime.ApiResponse<List>> {
async postEventSearch(requestParameters: PostEventSearchRequest): Promise<List> {

this is using openapi-generator 4.0.3

Activity

  1. auto-labeler commented on Sep 10, 2019

    @auto-labeler

    👍 Thanks for opening this issue!
    🏷 I have applied any labels matching special text in your issue.

    The team will review the labels and make any necessary changes.

  2. macjohnny commented on Sep 10, 2019

    @macjohnny
    Member

    can you explain why it should be List instead of Array?

  3. keean commented on Sep 10, 2019

    @keean
    Author

    First, Array is not a good typescript type (at least not in TypeScript 3.1.3). This will generate the compile time warning:

    [ WARN  ]  TypeScript: src/kaizen-api/apis/EventsApi.ts:182:102
               Generic type 'Array<T>' requires 1 type argument(s).
    
        L181:   */
        L182:  async postEventSearchRaw(requestParameters: PostEventSearchRequest): Promise<runtime.ApiResponse<Array>> {
        L183:      if (requestParameters.payload === null || requestParameters.payload === undefined) {
    

    It should be Array<T> and what is T? I guess the closest allowable typescript type would be Array<any>.

    The easiest way to see what type it should be is to look at the code. If we look at the return statement from the function starting at line 182:

    return new runtime.JSONApiResponse(response, (jsonValue) => ListFromJSON(jsonValue));

    We can see that the parameter to runtime.ApiResponse must be the same as the type returned from ListFromJSON which returns List.

  4. macjohnny commented on Sep 10, 2019

    @macjohnny
    Member

    @keean can you try with the latest version 4.1.1?

  5. keean commented on Sep 10, 2019

    @keean
    Author

    4.1.1 seems to have the same problem, but less serious because it at least compiles (with warnings).

    However the client code has to treat the value as a "List" because it has this interface:

    export interface List {
        /**
         * 
         * @type {number}
         * @memberof List
         */
        start?: number;
        /**
         * 
         * @type {Array<object>}
         * @memberof List
         */
        hits?: Array<object>;
        /**
         * 
         * @type {number}
         * @memberof List
         */
        total?: number;
        /**
         * 
         * @type {number}
         * @memberof List
         */
        size?: number;
    }

    It is clearly not an Array, it does not even have a length property. ListFromJSON returns a List because that is what the JSON structure passed from the server its, not an array.

    I don't really understand why TypeScript allows this, it seems to be a limitation in the type checking, effectively it allows:

    Promise<List> to be silently cast to Promise<Array> and then Promise<Array> to be cast back to Promise<List> in the client code.

    The type really is not an Array at all. My code works now without hand editing, so I am not worried about this, but you should be worried about this because a type is completely wrong and all it would take would be one dereference to crash at runtime. Is there a deeper problem here?

  6. keean commented on Sep 10, 2019

    @keean
    Author

    Have just discovered this is an Error in production mode, it's a warning only for development compilation, so it is important to me to get this resolved.

    [ ERROR ]  TypeScript: src/kaizen-api/src/apis/EventsApi.ts:182:102
               Generic type 'Array<T>' requires 1 type argument(s).
    
        L181:   */
        L182:  async postEventSearchRaw(requestParameters: PostEventSearchRequest): Promise<runtime.ApiResponse<Array>> {
        L183:      if (requestParameters.payload === null || requestParameters.payload === undefined) {
    
    [ ERROR ]  TypeScript: src/kaizen-api/src/apis/UsersApi.ts:390:100
               Generic type 'Array<T>' requires 1 type argument(s).
    
        L389:   */
        L390:  async postUserSearchRaw(requestParameters: PostUserSearchRequest): Promise<runtime.ApiResponse<Array>> {
        L391:      if (requestParameters.payload === null || requestParameters.payload === undefined) {
    
    [ ERROR ]  TypeScript: src/kaizen-api/src/runtime.ts:127:24
               Cannot find name 'GlobalFetch'.
    
        L127:  export type FetchAPI = GlobalFetch['fetch'];
    
    [22:56.4]  build failed in 14.45 s
    

    So I am back to hand editing the Array to List to get this to work.

  7. macjohnny commented on Sep 10, 2019

    @macjohnny
    Member

    Would you like to implement a fix?
    I think the type should be Array, and hence fixed in the ...FromJSON
    @TiFu what is your opinion?

  8. keean commented on Sep 10, 2019

    @keean
    Author

    If we add a console.log, we can see the actual type there is a List not an Array:

    await postEventSearch = {hits: Array(1), size: 10, start: 0, total: 1}
    

    So we can clearly see the Promise resolves to a List and not an Array, hence the return type of postEventSearch must be Promise<List>

    The ListFromJSON is returning the correct type. You can clearly see what it is meant to be looking at the swagger.json file:

    "200": {
         "description": "Success",
         "schema": {
             "$ref": "#/definitions/list"
         }
    },

    and the reference to List is clearly an object not an array:

    "list": {
        "properties": {
            "hits": {
                "items": {
                    "type": "object"                
                },
                "type": "array"
            },
            "size": {
                "type": "integer"
            },
            "start": {
                "type": "integer"
            },
            "total": {
                "type": "integer"
            }
        },
       "type": "object"
    },

    So postEventSearch returns a single List object not an Array.

    I am happy to look at implementing a fix, but I have not looked at the OpenAPI generator code before. Where is the code that derives what the return type should be? In the template it gets referred to as returnType so it is perhaps not a template problem? How is the returnType derived?

  9. TiFu commented on Sep 11, 2019

    @TiFu
    Contributor

    @macjohnny I agree with @keean that it should be of type List.

    I think the bug is in AbstractTypeScriptClientCodegen:

    https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractTypeScriptClientCodegen.java#L110

    It defines that the type list maps to a (TypeScript) array which in my opinion doesn't make a whole lot of sense to include - The OAI Spec doesn't define that list is equivalent to array and this also doesn't make a whole lot of sense in the context of TypeScript. Removing that line will likely fix this bug.
    Note that removing this will be a breaking change. However, I'd hope no one relies on this behavior.

    @keean would you be willing to test this and submit a PR if this fixes the bug?

  10. macjohnny commented on Sep 11, 2019

    @macjohnny
    Member

    @tifi @keean thanks for the analysis. Now I understand, List is a custom type, and I agree, the mapping in


    does not make sense. @wing328 can you confirm there is no generic type List in the parsed spec?

  11. keean commented on Sep 11, 2019

    @keean
    Author

    I am happy to test. As far as I am aware there is no generic List type in TypeScript, and it is not a reserved word in OpenAPI. List of TypeScript Basic types:

    https://www.typescriptlang.org/docs/handbook/basic-types.html

    And TypeScript advanced types:

    https://www.typescriptlang.org/docs/handbook/advanced-types.html

    But whatever language is used, unless OpenAPI defines it as a reserved word, I would expect the schema custom definition to be used, which means in a language where "List" is reserved, then the OpenAPI type would need to be mapped to "ApiList" or some other identifier.

    I would assume that an OpenAPI sphema referencing an undefined "List" object would fail to validate?

  12. wing328 commented on Sep 11, 2019

    @wing328
    Member

    does not make sense. @wing328 can you confirm there is no generic type List in the parsed spec?

    "List" is not an OpenAPI type from what I can tell.

  13. keean commented on Sep 11, 2019

    @keean
    Author

    So if I comment out that line, it generates the correct types.

    However I still have to make a manual change to get it to compile without error, that I might as well mention here, I have to replace GlobalFetch with WindowOrWorkerGlobalScope in src/runtime.ts

  14. macjohnny commented on Sep 11, 2019

    @macjohnny
    Member

    An option to generate code for typescript 3+ was introduced, which should fix that

  15. evstinik commented on Nov 19, 2019

    @evstinik

    @macjohnny oh finally, you saved my life. An option for ts3+.. how long I've been searching for that just to switch to newest ts version 3.7. Thanks!

  16. dngconsulting commented on Dec 5, 2019

    @dngconsulting
    Contributor

    It it possible to fix this compilation error also ?

     /app/src/sdk/runtime.ts
    webapp       | TypeScript error in /app/src/sdk/runtime.ts(297,11):
    webapp       | 'value', which lacks return-type annotation, implicitly has an 'any' return type.  TS7010
    webapp       |
    webapp       |     295 |     constructor(public raw: Response) {}
    webapp       |     296 |
    webapp       |   > 297 |     async value() {
    webapp       |         |           ^
    webapp       |     298 |         return undefined;
    webapp       |     299 |     }
    webapp       |     300 | }
    
    

    Thanx

    Sami

  17. macjohnny commented on Dec 5, 2019

    @macjohnny
    Member

    @dngconsulting yes, I would be happy to review your pull-request.

  18. dngconsulting commented on Dec 5, 2019

    @dngconsulting
    Contributor

    @macjohnny here we go => #4711 hoping that the merge will be as quick as your answer 😉

  19. Bakkiyaraj commented on Feb 6, 2020

    @Bakkiyaraj

    Cannot find name 'GlobalFetch'. TS2304
    export type FetchAPI = GlobalFetch['fetch'];

    We are using typescript-fetch to generate models. Can you please confirm which release has the fix?
    or suggest the workaround for this

  20. janniclas commented on Feb 10, 2020

    @janniclas

    @Bakkiyaraj A general workaround is to replace GlobalFetch['fetch'] with WindowOrWorkerGlobalScope['fetch'].
    The correct code is generated if you use the --additional-properties=typescriptThreePlus=true flag during generation

  21. trumbitta commented on Oct 30, 2020

    @trumbitta
    Contributor

    @janniclas I'm confused: shouldn't TS3+ be the default?

  22. DanielRuf commented on Nov 18, 2021

    @DanielRuf
    Contributor

    @janniclas I'm confused: shouldn't TS3+ be the default?

    Related: #9973

  23. rvansa commented on Jun 1, 2022

    @rvansa

    I am experiencing the typescript error due to Array without generics with typescript-fetch using version 6.0.0, too - I don't have the items in OpenApi as it is a free-form input; I'd expect Array<any>. Can I do something about this in this version?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions