Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[typescript] change accessToken name parameter to optional #3555

Merged
merged 2 commits into from
Aug 5, 2019

Conversation

blackdev1l
Copy link
Contributor

@blackdev1l blackdev1l commented Aug 4, 2019

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.
  • Filed the PR against the correct branch: master, 4.1.x, 5.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

fixes #2956
Iif you generate an axios API, you get a compilation error if tsconfig is set to 'strict'

because configuration.accessToken() requires name as paramater (not optional) but here in the code generated it calls configuration.accessToken() without passing any parameters.

I just made name an optional parameter.
I don't know if this is the right way to solve the issue, i would like to have some feedback about this.

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

Copy link
Member

@macjohnny macjohnny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
I think this change is good, since the accessToken function is called with a name parameter in some cases, but not in all cases, see

{{#isBasicBearer}}
// http bearer authentication required
if (configuration && configuration.accessToken) {
const accessToken = typeof configuration.accessToken === 'function'
? configuration.accessToken()
: configuration.accessToken;
localVarHeaderParameter["Authorization"] = "Bearer " + accessToken;
}
{{/isBasicBearer}}
{{#isOAuth}}
// oauth required
if (configuration && configuration.accessToken) {
const localVarAccessTokenValue = typeof configuration.accessToken === 'function'
? configuration.accessToken("{{name}}", [{{#scopes}}"{{{scope}}}"{{^-last}}, {{/-last}}{{/scopes}}])
: configuration.accessToken;
localVarHeaderParameter["Authorization"] = "Bearer " + localVarAccessTokenValue;
}
{{/isOAuth}}

@macjohnny
Copy link
Member

macjohnny commented Aug 5, 2019

@blackdev1l thanks for the PR. Could you please re-generate and commit the samples?

@blackdev1l
Copy link
Contributor Author

hey @macjohnny thanks for the review,
i run ./bin/typescript-axios-petstore-all.sh and committed, since it is a change only in the typescript generator, should i run another script to re-generate all the samples ?

@macjohnny
Copy link
Member

running ./bin/typescript-axios-petstore-all.sh is enough

@macjohnny macjohnny closed this Aug 5, 2019
@macjohnny macjohnny reopened this Aug 5, 2019
@macjohnny macjohnny added this to the 4.1.0 milestone Aug 5, 2019
Copy link
Member

@macjohnny macjohnny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@macjohnny macjohnny merged commit 00f7134 into OpenAPITools:master Aug 5, 2019
@wing328 wing328 changed the title #2956 [typescript] change accessToken name parameter to optional [typescript] change accessToken name parameter to optional Aug 9, 2019
@wing328
Copy link
Member

wing328 commented Aug 10, 2019

@blackdev1l thanks for the PR, which has been included in the 4.1.0 release: https://twitter.com/oas_generator/status/1160000504455319553

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG][Typescript axios] Generated call to accessToken doesn't match type definition
3 participants