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

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

Closed
5 of 6 tasks
rhuanbarreto opened this issue Oct 20, 2021 · 9 comments
Closed
5 of 6 tasks

Comments

@rhuanbarreto
Copy link
Contributor

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issue still exists?
  • Have you searched for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

If an API definition has a schema with another parameter format than camelCase, the parameter format in the model/interface should also follow the paramNaming config and be camelCase.

openapi-generator version

5.3.0-SNAPSHOT

OpenAPI declaration file content or url
openapi: "3.0.1"

info:
  title: Test API
  version: "1"

paths:
  /projects/{project_name}/test:
    post:
      operationId: create_test
      summary: Create a test
      parameters:
        - $ref: "#/components/parameters/project_name"
      requestBody:
        content:
          application/json:
            schema:
              $ref: "#/components/schemas/Test"
      responses:
        201:
          description: Test created
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/TestStatus"
components:
  parameters:
    project_name:
      description: Name of the project
      name: project_name
      in: path
      required: true
      schema:
        type: string
        example: test-project
  schemas:
    Test:
      type: object
      additionalProperties: false
      properties:
        start_time:
          type: integer
          title: start_time
          description: The number of milliseconds since 00:00:00 Thursday, 1 January 1970, Coordinated Universal Time (UTC), minus leap seconds.
          minimum: 0
        end_time:
          type: integer
          title: end_time
          description: The number of milliseconds since 00:00:00 Thursday, 1 January 1970, Coordinated Universal Time (UTC), minus leap seconds.
          minimum: 0
      required:
        - start_time
        - end_time
    TestStatus:
      type: object
      properties:
        id:
          description: An UUID v4 identifier for the test
          type: string
          format: uuid
          example: 5a7b8f0c-e8c8-4b5b-b8e2-e8e9f8e9e9e9
        status:
          description: The status of the test
          type: string
          enum:
            - Running
            - Pending
            - Success
            - Failed
            - Error
            - Cancelled
            - Unknown
          example: Running
      required:
        - id
        - status
Generation Details

config:

{
  "projectName": "test",
  "moduleName": "Sdk",
  "supportsES6": true
}
Steps to reproduce

Generate the API with this config.

Then model interfaces will be generated without translating parameters to camelCase.

This behaviour is done in the javascript generator correctly.

@nickjs
Copy link

nickjs commented Nov 22, 2021

I am also experiencing this issue, while it worked fine in the previous version. Please let me know if there is any additional test case or debugging information I can provide.

@ty-v1
Copy link
Contributor

ty-v1 commented Jan 16, 2022

@rhuanbarreto @nickjs
You should set modelPropertyNaming option as camelCase to camelize interface pararameters.
But this option was disabled by #10447. So, you cannot change interface property naming.

@nickjs
Copy link

nickjs commented Jan 18, 2022

Hi Yuya, thanks for the follow up! Upon digging further into this I did find that PR that disabled the option (sorry I didn't update my original comment!), however I believe there may be further use cases to consider. I totally agree with the spirit behind the change: the generated axios requests/responses should match what the server is going to send or expect! If the server won't send camelCased properties, we shouldn't even allow camelCased properties to be an option.

However. Axios is, for better or worse, highly configurable. For example, I currently find myself working in a codebase where axios is configured to transform all incoming property names from snake_case to camelCase, which breaks with these latest typings. And that expectation is, of course, already used across hundreds of call-sites, so changing them all would be difficult :)

To be clear though: I am not trying to make an argument purely for backwards-compatibility -- more just that since this is an axios-specific adapter, if there's an option or transform that axios supports, we should allow generating typings to match it. Probably not go out of our way to do so if something is difficult, but at least make it possible.

@aiven-hh @macjohnny Sorry to tag you in here, but I would love to get your feedback. Would you be open to me submitting a PR that adds just modelPropertyNaming back (but keeps all the other great changes of #10447)?

@aiven-hh
Copy link
Contributor

aiven-hh commented Jan 18, 2022

It was probably wrong to disable it. I only now noticed reading that point through that it is documented as "Only change it if you provide your own run-time code for (de-)serialization of models". I probably assumed it transformed the properties in every case which was the problem overall (and was fixed with other changes and I assume works with the "original" setting).

@macjohnny
Copy link
Member

@nickjs your contribution would be welcome

@vmstarchenko
Copy link

Hello!

Do we have any updates on this issue? In this issue parameter modelPropertyNaming was removed because of internal problems. Can it be added back in this issue. Option modelPropertyNaming looks like a required feature.

@toriumi0118
Copy link

toriumi0118 commented Aug 10, 2022

#10643 (comment)
is looks right. No longer support modelPropertyName option. It does not take effect to genereated code with code generator version 5.3+.

However, in my case, I need the behavior for my running product in development. Then, I addressed it with follows. It might make your project complex and bit hard to maintain.

  1. Export default template with yarn openapi-generator-cli author template -g typescript-axios -o templates. (I don't know how you call the cli. The openapi-generator-cli have a author option, then use it. ref: https://openapi-generator.tech/docs/usage#author )
  2. You can achieve templates in templates directory which you pass the option by -o at step1.
  3. You can find a modelGeneric.mustache file in templates dir.
  4. Surround the {{baseName}} by lambda.camelcase functiion. like this. {{#lambda.camelcase}}{{baseName}}{{/lambda.camelcase}} ref: https://openapi-generator.tech/docs/templating/#mustache-lambdas
  5. Call generate command with templateDir option. ref: https://openapi-generator.tech/docs/usage#generate

The process is totally ignore your option which might change model property name if there is. Because it converts the property name by compulsion.

@Eiryyy
Copy link

Eiryyy commented Aug 16, 2022

I use axios to convert snake_case responses by Rails to camelCase. modelPropertyName is required.

@mourad1081
Copy link

Any update on this? This is insane that camelCase, which is such a trivial and must-to-have feature has been disabled!

@rhuanbarreto rhuanbarreto closed this as not planned Won't fix, can't repro, duplicate, stale Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants