typescript-node: Fixed models.mustache with for...in on an empty array #8575

glblaser · 2021-01-29T07:12:32Z

When an empty array comes in, the current for...in syntax results in creating an empty object being created and added to the array. This is the output from receiving an empty array that gets deserialized, resulting in an object with undefined values.

Switching to a for i < array.length loop fixes this issue.

body: [
    GroupCustomResource {
      apiVersion: undefined,
      kind: undefined,
      spec: undefined,
      metadata: undefined,
      status: undefined,
      '': undefined
    }
  ]

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:
```
./mvnw clean package 
./bin/generate-samples.sh
./bin/utils/export_docs_generators.sh
```
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
For Windows users, please run the script in Git BASH.
File the PR against the correct branch: master, 5.1.x, 6.0.x
If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

TypeScript 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

When an empty array comes in, the current for...in syntax results in creating an empty object being created and added to the array. This is the output from receiving an empty array that gets deserialized, resulting in an object with undefined values. Switching to a for i < array.length loop fixes this issue. ``` body: [ GroupCustomResource { apiVersion: undefined, kind: undefined, spec: undefined, metadata: undefined, status: undefined, '': undefined } ] ```

macjohnny

LGTM

…ries to objects during deserialization. When an object comes in, the current for...in syntax results in adding an undefined key/value pair to each nested object, with key being empty string and value being undefined. This is the output from receiving an object that gets deserialized. Adding a check to see if attributeType.name exists before adding it to the deserialized object fixes this. ``` res.body of listGroup for saved group is [ GroupCustomResource { apiVersion: 'v1', kind: 'Group', spec: GroupSpec { name: 'TestDeploymentGroup67264', tenancyId: 'tenancy', description: 'TestGroup description', '': undefined }, metadata: { additionalProp1: {}, spectra: [Object] }, status: undefined, '': undefined } ] ```

macjohnny · 2021-01-29T14:19:48Z

@glblaser please merge the most recent master and re-generate the samples according to the PR checklist

amakhrov · 2021-01-29T17:46:44Z

@glblaser Looks like you have two distinct changes:

replace for .. in with for when serializing an array
add truthiness check when serializing an object

Could you please clarify the impact of each change? Showing the api-spec example, and generated code "before" and "after" would be very helpful also.

glblaser · 2021-01-29T20:34:25Z

@glblaser Looks like you have two distinct changes:

replace for .. in with for when serializing an array

add truthiness check when serializing an object

Could you please clarify the impact of each change? Showing the api-spec example, and generated code "before" and "after" would be very helpful also.

@amakhrov Sure thing. Let me know if anything's unclear.

For the for...in on an empty array change to a regular for loop.
Before the change my result when getting an empty array:

[
  GroupCustomResource {
    apiVersion: undefined,
    kind: undefined,
    spec: undefined,
    metadata: undefined,
    status: undefined,
    '': undefined
  }
]

After the change my result is:

[]

Before the change my when getting an array of size 1:

[
  GroupCustomResource {
    apiVersion: 'v1',
    kind: 'Group',
    spec: GroupSpec {
      name: 'TestDeploymentGroup67264',
      tenancyId: 'tenancy',
      description: 'TestGroup description',
      '': undefined
    },
    metadata: { additionalProp1: {}, spectra: [Object] },
    status: undefined,
    '': undefined
  },
  GroupCustomResource {
    apiVersion: undefined,
    kind: undefined,
    spec: undefined,
    metadata: undefined,
    status: undefined,
    '': undefined
  }
]

After the change my result is:

[
  GroupCustomResource {
    apiVersion: 'v1',
    kind: 'Group',
    spec: GroupSpec {
      name: 'TestDeploymentGroup67264',
      tenancyId: 'tenancy',
      description: 'TestGroup description',
      '': undefined
    },
    metadata: { additionalProp1: {}, spectra: [Object] },
    status: undefined,
    '': undefined
  }
]

It seems to add this "ghost" object to my returned result with all values being undefined.

For the truthiness check change (note: this is with the above array change implemented).
Before the change my result when getting an array of size 1:

[
  GroupCustomResource {
    apiVersion: 'v1',
    kind: 'Group',
    spec: GroupSpec {
      name: 'TestDeploymentGroup67264',
      tenancyId: 'tenancy',
      description: 'TestGroup description',
      '': undefined
    },
    metadata: { additionalProp1: {}, spectra: [Object] },
    status: undefined,
    '': undefined
  }
]

After the change my result is:

[
  GroupCustomResource {
    apiVersion: 'v1',
    kind: 'Group',
    spec: GroupSpec {
      name: 'TestDeploymentGroup67264',
      tenancyId: 'tenancy',
      description: 'TestGroup description'
    },
    metadata: { additionalProp1: {}, spectra: [Object] },
    status: undefined
  }
]

The truthiness check removes the '': undefined key/value pair from each nested object.

amakhrov · 2021-01-29T21:15:19Z

Thanks for the clarification @glblaser !

The changes look good to me.

As a side note, it looks like you might have something funky in your codebase (perhaps extending Array.prototype with custom properties).

amakhrov · 2021-01-29T21:17:43Z

modules/openapi-generator/src/main/resources/typescript-node/models.mustache

@@ -172,7 +172,9 @@ export class ObjectSerializer {
            let attributeTypes = typeMap[type].getAttributeTypeMap();
            for (let index in attributeTypes) {
                let attributeType = attributeTypes[index];
-                instance[attributeType.name] = ObjectSerializer.deserialize(data[attributeType.baseName], attributeType.type);
+                if (attributeType.name) {


perhaps a more robust fix could be to switch from for .. in to for here.
since you get some random property from attributeTypes, it's totally possible that this property also has a nested name key - so this check would be true, which is undesired.

attributeTypes is just an array of the key's name and the value's type. I modified my fix for this to changing the for...in loop to a regular for loop, as that does not pick up the "ghost" property on the array like the for...in loop. This also negates the need for a truth check.

attributeTypes is [ { name: 'group', baseName: 'group', type: 'string' }, { name: 'name', baseName: 'name', type: 'string' }, { name: 'description', baseName: 'description', type: 'string' } ]

wing328 · 2021-02-04T06:08:43Z

Please merge the latest master to your branch to address some of the CI issues.

…ialize

glblaser · 2021-02-12T01:28:47Z

Please merge the latest master to your branch to address some of the CI issues.

Messed up earlier by rebasing to the latest master, but corrected that with a fetch/merge. I think it's good to go now.

Not sure about the the ci/circleci failure/errors having to do with my changes.

wing328 · 2021-03-22T06:23:54Z

@glblaser thanks for the PR, which has been included in the v5.1.0 release: https://twitter.com/oas_generator/status/1373636654024380423

macjohnny approved these changes Jan 29, 2021

View reviewed changes

amakhrov reviewed Jan 29, 2021

View reviewed changes

jimschubert added the Client: TypeScript label Feb 3, 2021

wing328 added the Issue: Bug label Feb 4, 2021

Gary Blaser added 2 commits February 11, 2021 16:20

Fixed for array for loop in serialize and added truth check for deser…

98938b5

…ialize

Made for...in loop a for loop; gets rid of the need for the truth check

fd30330

glblaser requested a review from jimschubert as a code owner February 11, 2021 23:58

Merge remote-tracking branch 'upstream/master'

4d21be1

glblaser force-pushed the master branch from 61dfa81 to 4d21be1 Compare February 12, 2021 00:17

Fixed serialze/deserialize for loops for arrays

88593b8

macjohnny approved these changes Feb 12, 2021

View reviewed changes

macjohnny merged commit 0e16eba into OpenAPITools:master Feb 12, 2021

macjohnny added this to the 5.1.0 milestone Feb 12, 2021

macjohnny changed the title ~~Fixed typescript bug in models.mustache with for...in on an empty array.~~ typescript-node: Fixed typescript bug in models.mustache with for...in on an empty array. Feb 12, 2021

wing328 changed the title ~~typescript-node: Fixed typescript bug in models.mustache with for...in on an empty array.~~ typescript-node: Fixed models.mustache with for...in on an empty array Mar 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

typescript-node: Fixed models.mustache with for...in on an empty array #8575

typescript-node: Fixed models.mustache with for...in on an empty array #8575

glblaser commented Jan 29, 2021 •

edited

Loading

macjohnny left a comment

macjohnny commented Jan 29, 2021 •

edited

Loading

amakhrov commented Jan 29, 2021

glblaser commented Jan 29, 2021 •

edited

Loading

amakhrov commented Jan 29, 2021

amakhrov Jan 29, 2021

glblaser Feb 11, 2021

wing328 commented Feb 4, 2021

glblaser commented Feb 12, 2021

wing328 commented Mar 22, 2021

typescript-node: Fixed models.mustache with for...in on an empty array #8575

typescript-node: Fixed models.mustache with for...in on an empty array #8575

Conversation

glblaser commented Jan 29, 2021 • edited Loading

PR checklist

macjohnny left a comment

Choose a reason for hiding this comment

macjohnny commented Jan 29, 2021 • edited Loading

amakhrov commented Jan 29, 2021

glblaser commented Jan 29, 2021 • edited Loading

amakhrov commented Jan 29, 2021

amakhrov Jan 29, 2021

Choose a reason for hiding this comment

glblaser Feb 11, 2021

Choose a reason for hiding this comment

wing328 commented Feb 4, 2021

glblaser commented Feb 12, 2021

wing328 commented Mar 22, 2021

glblaser commented Jan 29, 2021 •

edited

Loading

macjohnny commented Jan 29, 2021 •

edited

Loading

glblaser commented Jan 29, 2021 •

edited

Loading