[typescript-axios] Fix model property naming by aiven-hh · Pull Request #10447 · OpenAPITools/openapi-generator

aiven-hh · 2021-09-22T10:57:59Z

The property names in typescript-axios models were using formatted names instead of baseName and the template has no mapping to original baseName so this PR introduces a fix by:

removing MODEL_PROPERTY_NAMING option from typescript-axios generator as this was never supported
using baseName in model properties wrapped with '' for now to support at least all characters except '

This should fix #9524.

Also adds fake petstore example to be able verify these changes at least by eye, see samples/client/petstore/typescript-axios/builds/test-petstore/api.ts and SpecialModelName interface as an example.

In addition adds typing for options that was typed as any before.

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.3.0), 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.

aiven-hh · 2021-09-22T10:59:20Z

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

macjohnny

LGtM

macjohnny · 2021-09-28T17:07:52Z

@aiven-hh can you please resolve merge the latest master and re-generate the samples?

Disables MODEL_PROPERTY_NAMING for typescript-axios generator as this was never supported (templates have no mapping between original property names and formatted names). Wraps all properties in model interfaces with '' to support most characters except ' for now. Also fixes missing options typing that was any before.

aiven-hh · 2021-09-29T08:18:11Z

Rebased, all major changes still in the first commit. New sample and ./mvnw clean package && ./bin/generate-samples.sh changes in the second commit and ./mvnw clean package && ./bin/utils/export_docs_generators.sh in the third commit.

macjohnny

LGTM

macjohnny · 2021-10-02T06:10:37Z

@aiven-hh thanks for your contribution. One minir thing: if the model property name contains a dot, it is still converted to underscore

wing328 · 2021-10-02T09:50:35Z

Looks like this breaks the build:

Run `npm audit` for details.

> [email protected] build
> tsc

../../builds/with-npm-version/api.ts(287,77): error TS2339: Property 'query' does not exist on type 'AxiosRequestConfig'.
../../builds/with-npm-version/api.ts(331,77): error TS2339: Property 'query' does not exist on type 'AxiosRequestConfig'.
../../builds/with-npm-version/api.ts(372,77): error TS2339: Property 'query' does not exist on type 'AxiosRequestConfig'.
../../builds/with-npm-version/api.ts(414,77): error TS2339: Property 'query' does not exist on type 'AxiosRequestConfig'.
../../builds/with-npm-version/api.ts(451,77): error TS2339: Property 'query' does not exist on type 'AxiosRequestConfig'.
../../builds/with-npm-version/api.ts(490,77): error TS2339: Property 'query' does not exist on type 'AxiosRequestConfig'.
../../builds/with-npm-version/api.ts(542,77): error TS2339: Property 'query' does not exist on type 'AxiosRequestConfig'.
../../builds/with-npm-version/api.ts(594,77): error TS2339: Property 'query' does not exist on type 'AxiosRequestConfig'.
../../builds/with-npm-version/api.ts(949,77): error TS2339: Property 'query' does not exist on type 'AxiosRequestConfig'.
../../builds/with-npm-version/api.ts(982,77): error TS2339: Property 'query' does not exist on type 'AxiosRequestConfig'.
../../builds/with-npm-version/api.ts(1016,77): error TS2339: Property 'query' does not exist on type 'AxiosRequestConfig'.
../../builds/with-npm-version/api.ts(1051,77): error TS2339: Property 'query' does not exist on type 'AxiosRequestConfig'.
../../builds/with-npm-version/api.ts(1254,77): error TS2339: Property 'query' does not exist on type 'AxiosRequestConfig'.
../../builds/with-npm-version/api.ts(1290,77): error TS2339: Property 'query' does not exist on type 'AxiosRequestConfig'.
../../builds/with-npm-version/api.ts(1326,77): error TS2339: Property 'query' does not exist on type 'AxiosRequestConfig'.
../../builds/with-npm-version/api.ts(1361,77): error TS2339: Property 'query' does not exist on type 'AxiosRequestConfig'.
../../builds/with-npm-version/api.ts(1395,77): error TS2339: Property 'query' does not exist on type 'AxiosRequestConfig'.
../../builds/with-npm-version/api.ts(1439,77): error TS2339: Property 'query' does not exist on type 'AxiosRequestConfig'.
../../builds/with-npm-version/api.ts(1469,77): error TS2339: Property 'query' does not exist on type 'AxiosRequestConfig'.
../../builds/with-npm-version/api.ts(1508,77): error TS2339: Property 'query' does not exist on type 'AxiosRequestConfig'.
npm ERR! code 2
npm ERR! path /home/travis/build/OpenAPITools/openapi-generator/samples/client/petstore/typescript-axios/tests/default
npm ERR! command failed
npm ERR! command sh -c npm install ../../builds/with-npm-version && npm run build

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/travis/.npm/_logs/2021-10-02T06_31_56_428Z-debug.log
[ERROR] Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.2.1:exec (npm-install) on project TypeScriptAxiosPestoreClientTests: Command execution failed. Process exited with an error: 2 (Exit value: 2) -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.2.1:exec (npm-install) on project TypeScriptAxiosPestoreClientTests: Command execution failed.
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:215)

Ref: https://app.travis-ci.com/github/OpenAPITools/openapi-generator/builds/238968898

Can you please take a look when you've time?

aiven-hh · 2021-10-02T13:30:01Z

@wing328 Hmm, the change exposed another bug in the original templates, the generated code uses options.query when axios does not actually use query for query params but params.

aiven-hh · 2021-10-02T13:37:45Z

@macjohnny are you sure? If you check the original example here:

openapi-generator/modules/openapi-generator/src/test/resources/3_0/java/petstore-with-fake-endpoints-models-for-testing-with-http-signature.yaml

Lines 1797 to 1803 in 026dd8d

    
           _special_model.name_: 
        
             properties: 
        
               '$special[property.name]': 
        
                 type: integer 
        
                 format: int64 
        
               '_special_model.name_': 
        
                 type: string

and the generated sample

openapi-generator/samples/client/petstore/typescript-axios/builds/test-petstore/api.ts

Lines 1556 to 1569 in 026dd8d

    
           export interface SpecialModelName { 
        
               /** 
        
                *  
        
                * @type {number} 
        
                * @memberof SpecialModelName 
        
                */ 
        
               '$special[property.name]'?: number; 
        
               /** 
        
                *  
        
                * @type {string} 
        
                * @memberof SpecialModelName 
        
                */ 
        
               '_special_model.name_'?: string; 
        
           }

it looks fine to me.

macjohnny · 2021-10-02T13:48:14Z

@aiven-hh would you like to provide a fix? Or should we first revert the PR?

aiven-hh · 2021-10-02T14:27:15Z

Whatever you prefer, I can send the fix pr in a few minutes after I verify the fix.

aiven-hh · 2021-10-02T14:44:55Z

@macjohnny the fix is now in #10512

macjohnny approved these changes Sep 23, 2021

View reviewed changes

aiven-hh added 3 commits September 29, 2021 11:04

chore: add new sample, generate samples

48318b0

chore: update generator docs

ef3c20b

aiven-hh force-pushed the fix-typescript-axios-properties branch from 34a8637 to ef3c20b Compare September 29, 2021 08:15

aiven-hh requested a review from macjohnny September 29, 2021 08:19

macjohnny approved these changes Oct 2, 2021

View reviewed changes

macjohnny merged commit 755359c into OpenAPITools:master Oct 2, 2021

macjohnny added the Client: TypeScript label Oct 2, 2021

macjohnny added this to the 5.3.0 milestone Oct 2, 2021

aiven-hh mentioned this pull request Oct 2, 2021

[typescript-axios] Fix invalid query params usage #10512

Merged

5 tasks

ty-v1 mentioned this pull request Jan 16, 2022

[BUG] [typescript-axios] Interface parameters don't follow paramNaming #10643

Closed

6 tasks

jyotishp mentioned this pull request Jan 17, 2022

[REQ][typescript-axios] Add case conversion for model property names #11338

Open

stropho mentioned this pull request Jan 2, 2023

[BUG][TYPESCRIPT] typescript-axios: all attributes are optional and parameter names are enquoted #13246

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[typescript-axios] Fix model property naming#10447

[typescript-axios] Fix model property naming#10447
macjohnny merged 3 commits intoOpenAPITools:masterfrom
aiven-hh:fix-typescript-axios-properties

aiven-hh commented Sep 22, 2021 •

edited

Loading

Uh oh!

aiven-hh commented Sep 22, 2021

Uh oh!

macjohnny left a comment

Uh oh!

macjohnny commented Sep 28, 2021

Uh oh!

aiven-hh commented Sep 29, 2021

Uh oh!

macjohnny left a comment

Uh oh!

macjohnny commented Oct 2, 2021

Uh oh!

wing328 commented Oct 2, 2021

Uh oh!

aiven-hh commented Oct 2, 2021

Uh oh!

aiven-hh commented Oct 2, 2021

Uh oh!

macjohnny commented Oct 2, 2021

Uh oh!

aiven-hh commented Oct 2, 2021

Uh oh!

aiven-hh commented Oct 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

aiven-hh commented Sep 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR checklist

Uh oh!

aiven-hh commented Sep 22, 2021

Uh oh!

macjohnny left a comment

Choose a reason for hiding this comment

Uh oh!

macjohnny commented Sep 28, 2021

Uh oh!

aiven-hh commented Sep 29, 2021

Uh oh!

macjohnny left a comment

Choose a reason for hiding this comment

Uh oh!

macjohnny commented Oct 2, 2021

Uh oh!

wing328 commented Oct 2, 2021

Uh oh!

aiven-hh commented Oct 2, 2021

Uh oh!

aiven-hh commented Oct 2, 2021

Uh oh!

macjohnny commented Oct 2, 2021

Uh oh!

aiven-hh commented Oct 2, 2021

Uh oh!

aiven-hh commented Oct 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

aiven-hh commented Sep 22, 2021 •

edited

Loading