[typescript] fixture: broken `Array<Array>` #19548

joscha · 2024-09-08T21:25:01Z

Full disclosure: I haven't been able to fix this issue, yet. I can reproduce it and I created a fixture that shows the problem, but I need some help figuring out where exactly the issue occurs.

This introduces a fixture that shows an issue with the typescript generator.

Currently, for a definition like this (reduced for brevity, see array_list.yaml for full version):

      ListPaged:
        type: 'object'
        properties:
          data:
            type: 'array'
            items:
              $ref: '#/components/schemas/List'
      List:
        type: 'object'
        properties:
          id:
            type: 'integer'

(an array of objects with the name List).

as Typescript type like:

export class ListPaged {
    'data'?: Array<Array>;
    // ...
}

is produced. This is both incorrect as well as syntactically invalid (missing generic for the inner Array type).
It should be:

export class ListPaged {
    'data'?: Array<List>;
    // ...
}

Interestingly, if the name of List is changed to something that is not close to a reserved word, like L for example, it generates correctly. E.g.:

export class ListPaged {
    'data'?: Array<L>;
    // ...
}

Thus I am assuming there is somewhere in the model generation code where the word List is interpreted as being an array type.
Running the fixture with typescript-fetch shows that the ListPaged type gets generated correctly for this generator.

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:
File the PR against the correct branch: master (upcoming 7.6.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: @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)

joscha · 2024-09-08T21:26:37Z

samples/client/others/typescript/builds/array-of-lists/models/ListPaged.ts

+import { HttpFile } from '../http/http';
+
+export class ListPaged {
+    'data'?: Array<Array>;


This should be:

Suggested change

'data'?: Array<Array>;

'data'?: Array<List>;

…L.ts

macjohnny · 2024-09-09T06:09:52Z

the issue occurs because of a type mapping

openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptClientCodegen.java

Line 114 in 0026e15

typeMapping.put("List", "Array");

this happens with any model named like a native type. the typemapping can be overridden by parameter, so i would suggest to close this PR

joscha · 2024-09-09T07:38:20Z

the issue occurs because of a type mapping

I was wondering if that was the case, but couldn't find it. Thanks for the code pointer.

openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptClientCodegen.java

Line 114 in 0026e15

typeMapping.put("List", "Array");

this happens with any model named like a native type. the typemapping can be overridden by parameter, so i would suggest to close this PR

When you say "native type" you mean a native Java type that symbol in swagger matches to?

Given that:

List is an object in my example above, which is incompatible with Array
There is no native List type in neither Typescript nor Javascript
The generated code doesn't transpile because of a type error (the missing generic for Array)
The type information is incorrect
Other typescript generators don't have that issue
Often we don't control the openapi definition, we just get it from a vendor and whatever code is produced should just work out of the box
We can automatically determine that a symbol with the name List that is of type: "object" is not a an array
It is super hard to debug when you use the generator CLI as an end user, as an implementation detail (the type mapping) leaks through into the generated code

my thoughts are that we should probably fix this in the generator? WDYT?

macjohnny · 2024-09-09T08:47:32Z

I dont know why the List type mapping was needed/added, maybe @TiFu knows?

joscha · 2024-09-17T10:51:17Z

I dont know why the List type mapping was needed/added, maybe @TiFu knows?

@TiFu any idea? Would love to fix this one for good.

macjohnny · 2024-09-17T11:41:41Z

I guess this could be a leftover, IIRC there was such a List mapping in

openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractTypeScriptClientCodegen.java

Lines 331 to 358 in fd85359

    
           typeMapping = new HashMap<String, String>(); 
        
           typeMapping.put("Set", "Set"); 
        
           typeMapping.put("set", "Set"); 
        
           typeMapping.put("Array", "Array"); 
        
           typeMapping.put("array", "Array"); 
        
           typeMapping.put("boolean", "boolean"); 
        
           typeMapping.put("string", "string"); 
        
           typeMapping.put("int", "number"); 
        
           typeMapping.put("float", "number"); 
        
           typeMapping.put("number", "number"); 
        
           typeMapping.put("long", "number"); 
        
           typeMapping.put("short", "number"); 
        
           typeMapping.put("char", "string"); 
        
           typeMapping.put("double", "number"); 
        
           typeMapping.put("object", "object"); 
        
           typeMapping.put("integer", "number"); 
        
           typeMapping.put("Map", "any"); 
        
           typeMapping.put("map", "any"); 
        
           typeMapping.put("date", "string"); 
        
           typeMapping.put("DateTime", "string"); 
        
           typeMapping.put("binary", "any"); 
        
           typeMapping.put("File", "any"); 
        
           typeMapping.put("file", "any"); 
        
           typeMapping.put("ByteArray", "string"); 
        
           typeMapping.put("UUID", "string"); 
        
           typeMapping.put("URI", "string"); 
        
           typeMapping.put("Error", "Error"); 
        
           typeMapping.put("AnyType", "any");

as well, but now there isn't, so I guess we can remove it. @joscha do you want to do that in your PR and update the samples?

joscha · 2024-09-17T11:44:34Z

I guess this could be a leftover, IIRC there was such a List mapping in

openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractTypeScriptClientCodegen.java

Lines 331 to 358 in fd85359

typeMapping = new HashMap<String, String>();

typeMapping.put("Set", "Set");

typeMapping.put("set", "Set");

typeMapping.put("Array", "Array");

typeMapping.put("array", "Array");

typeMapping.put("boolean", "boolean");

typeMapping.put("string", "string");

typeMapping.put("int", "number");

typeMapping.put("float", "number");

typeMapping.put("number", "number");

typeMapping.put("long", "number");

typeMapping.put("short", "number");

typeMapping.put("char", "string");

typeMapping.put("double", "number");

typeMapping.put("object", "object");

typeMapping.put("integer", "number");

typeMapping.put("Map", "any");

typeMapping.put("map", "any");

typeMapping.put("date", "string");

typeMapping.put("DateTime", "string");

typeMapping.put("binary", "any");

typeMapping.put("File", "any");

typeMapping.put("file", "any");

typeMapping.put("ByteArray", "string");

typeMapping.put("UUID", "string");

typeMapping.put("URI", "string");

typeMapping.put("Error", "Error");

typeMapping.put("AnyType", "any");

as well, but now there isn't, so I guess we can remove it. @joscha do you want to do that in your PR and update the samples?

Great! Yes, give me an hour or so and I'll put it up!

…y-of-lists

joscha · 2024-09-17T12:51:08Z

@macjohnny changes are in, PTAL.

joscha · 2024-09-17T13:08:42Z

Thank you for 🚤 @macjohnny!

macjohnny · 2024-09-17T13:19:06Z

@joscha thanks for your contribution!

[typescript] fixture: broken Array<Array>

d37a5b1

joscha commented Sep 8, 2024

View reviewed changes

Delete samples/client/others/typescript/builds/array-of-lists/models/…

b4db689

…L.ts

joscha added 3 commits September 17, 2024 13:44

fix: remove incorrect mapping

cdbfcd6

chore: update generated code samples

dbffb8c

Merge remote-tracking branch 'upstream/master' into joscha/issue-arra…

2bbdb9c

…y-of-lists

macjohnny approved these changes Sep 17, 2024

View reviewed changes

macjohnny merged commit 0b084cd into OpenAPITools:master Sep 17, 2024
15 checks passed

joscha deleted the joscha/issue-array-of-lists branch September 17, 2024 13:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[typescript] fixture: broken `Array<Array>` #19548

[typescript] fixture: broken `Array<Array>` #19548

joscha commented Sep 8, 2024

joscha Sep 8, 2024

macjohnny commented Sep 9, 2024

joscha commented Sep 9, 2024

macjohnny commented Sep 9, 2024

joscha commented Sep 17, 2024

macjohnny commented Sep 17, 2024

joscha commented Sep 17, 2024

joscha commented Sep 17, 2024

joscha commented Sep 17, 2024

macjohnny commented Sep 17, 2024

[typescript] fixture: broken Array<Array> #19548

[typescript] fixture: broken Array<Array> #19548

Conversation

joscha commented Sep 8, 2024

PR checklist

joscha Sep 8, 2024

Choose a reason for hiding this comment

macjohnny commented Sep 9, 2024

joscha commented Sep 9, 2024

macjohnny commented Sep 9, 2024

joscha commented Sep 17, 2024

macjohnny commented Sep 17, 2024

joscha commented Sep 17, 2024

joscha commented Sep 17, 2024

joscha commented Sep 17, 2024

macjohnny commented Sep 17, 2024

[typescript] fixture: broken `Array<Array>` #19548

[typescript] fixture: broken `Array<Array>` #19548