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

Feature: [TypeScript] "CollectParameters" to accept params as option-bag #865

Open
cspotcode opened this issue Aug 21, 2018 · 15 comments
Open

Comments

@cspotcode
Copy link

Description

I would like the TypeScript code generator to support "CollectParameters" extension, which tells generated methods to accept all parameters as a single object / option-bag instead of multiple, positional arguments.

This extension is described here:
https://blog.apimatic.io/swagger-2-0-extension-for-code-generation-settings-abf602c3869d

openapi-generator version

3.2.1

OpenAPI declaration file content or url
paths:
  /favorites:
    get:
      operationId: GetFavorites
      x-operation-settings:
        CollectParameters: true
      parameters:
        - in: query
          name: account_id
          type: string
        - in: query
          name: user_id
          type: string
      responses:
        '200':
          description: '200'
          schema:
            $ref: '#/definitions/Favorites'

CollectParameters: true should generate client methods that accept an option-bag of params.

await client.getFavorites({account_id: '123', user_id: '456'});
Related issues/PRs

Rewrite of TypeScript generators

@macjohnny
Copy link
Member

@TiFu maybe this can also be considered in the rewrite of the TS generators?

@TiFu
Copy link
Contributor

TiFu commented Aug 22, 2018

Will keep this in mind.

@cspotcode
Copy link
Author

@TiFu the code-gen changes should be really minimal. Given a function signature like this:

fooOperation(fooId: FooId, otherParam: OtherParamType) {

The CollectParameters version looks almost identical:

fooOperation({fooId, otherParam}: {fooId: FooId, otherParam: OtherParamType}) {

When none of the params are required, then append a default value so that the option-bag can be omitted entirely:

fooOperation({fooId, otherParam}: {fooId?: FooId, otherParam?: OtherParamType} = {}) {

@TiFu
Copy link
Contributor

TiFu commented Aug 22, 2018

@cspotcode Another option would be to create interfaces for these parameter bags

e.g.

interface FooOperationParam {
      fooId: FooId,
      otherParam: OtherParamType
}

fooOperation(param: FooOperationParam) { }

// if all params are optional:
fooOperation(param: FooOperationParam = {})

What would you prefer?

@cspotcode
Copy link
Author

cspotcode commented Aug 22, 2018 via email

@wing328
Copy link
Member

wing328 commented Aug 29, 2018

There were similar suggestions before but nothing has been implemented yet. The use case was that the method contains many optional parameters, which will make the method signature very long in languages such as Java.

Related: #641

@cspotcode
Copy link
Author

@TiFu If I want to take a stab at implementing this issue, which branch should I base my code on?

@wing328
Copy link
Member

wing328 commented Nov 4, 2018

FYI. I've added group parameters support to PHP API client: #1337

@wing328
Copy link
Member

wing328 commented Nov 7, 2018

Hi all,

I've tried to add group parameters support to TS Fetch client with some fake parameters to illustrate the change in #1394.

https://github.com/OpenAPITools/openapi-generator/pull/1394/files#diff-157014844e879c93896c6ee0805f91ddR1541 is a good starting point to review the change.

Please review and let me know if you've any feedback.

Disclaimer: I'm no expert in TypeScript

Thanks,
William

cc @TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) @nicokoenig (2018/09) @topce (2018/10)

@Kiran-Sivakumar
Copy link
Contributor

Kiran-Sivakumar commented Nov 15, 2018

Hi all, I have made a PR for Java okhttp-gson API client addressing the same issue of method signatures being too long and inflexible: #1341

However, instead of "group parameters", my solution employs the builder pattern for API requests (#1217)

Currently, the vendor extension is x-group-parameters. This name is not appropriate for my PR given my solution. However, having the same vendor extension is ideal. My proposals for a new more general name for this vendor extension are as follows:
x-flexible-parameters
x-unrestrictive-parameters

Anyone have any thoughts on this?

@thril
Copy link

thril commented Apr 12, 2019

@Kiran-Sivakumar For typescript, using a destructured options bag is pretty standard. I would be more inclined to use that over a builder pattern.

const foo = ({ bar1, bar2, bar3 }: { bar1: string, bar2?: number, bar3: string[] }) => { ... }

I found this issue because this is exactly what I am looking for. I have imported and generated FHIR objects, which can have a ton of options, which look like...

  public patientGet(
    active?: string,
    address?: string,
    addressCity?: string,
    addressCountry?: string,
    addressPostalcode?: string,
    addressState?: string,
    addressUse?: string,
    birthdate?: string,
    deathDate?: string,
    deceased?: string,
    email?: string,
    family?: string,
    gender?: string,
    generalPractitioner?: string,
    given?: string,
    identifier?: string,
    language?: string,
    link?: string,
    name?: string,
    organization?: string,
    phone?: string,
    phonetic?: string,
    telecom?: string,
    testfamily?: string,
    format?: string
  ) => { ... }

Oh look, I need format - lemme just specify 24 undefined values before the one I need. Needless to say, that is less than desirable.

Any update on if/when this may make it in? I'm currently following issue 802. Thanks!

@StefanKern
Copy link

StefanKern commented Apr 25, 2019

We have a similar issue, but for us it is the upgrade of the generated API. Some parametes are added in between, some are removed, most are strings so there is no type checking and we only set a few and mostly they are called with undefined.

An example is:

public sampleFkt(param1: string = "1", param2: string = "2", param3: string = "3", para4: string = "4", param5: string = "6", param7: string = "7", param7: string = "8", param9: string = "9", param10: string = "10");
// called with
public public sampleFkt(undefined, undefined, undefined, undefined, undefined, undefined, "Hello", "World", "!");
// which gets changed to
public sampleFkt(param1: string = "1", param2: string = "2", param3: string = "3", para4: string = "4", param5: string = "6", param7: string = "7", NEWPARAM: string = "NEWPARAM" param7: string = "8", param9: string = "9", param10: string = "10");

Always a pain to find this, and very error prone.

@wing328 wing328 removed this from the 4.0.0 milestone May 13, 2019
@wing328 wing328 added this to the 4.0.1 milestone May 13, 2019
@wing328 wing328 modified the milestones: 4.0.1, 4.0.2 May 31, 2019
@macjohnny
Copy link
Member

related: #3349

@macjohnny
Copy link
Member

this is implemented for typescript-fetch, see #3349

@macjohnny
Copy link
Member

this is implemented for typescript-angular, see #4479

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

7 participants