-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
[typescript-fetch] Fix access token type #3323
Conversation
👍 Thanks for opening this issue! The team will review the labels and make any necessary changes. |
@@ -113,8 +113,8 @@ export class {{classname}} extends runtime.BaseAPI { | |||
} | |||
{{/isBasicBasic}} | |||
{{#isBasicBearer}} | |||
if (this.configuration && (this.configuration.accessToken || this.configuration.apiKey)) { | |||
const token = this.configuration.accessToken || this.configuration.apiKey; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why don't we want to use apiKey here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the this.configuration.apiKey
is used for # isApiKey
block (lines 126), not for # isBasicBearer
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's a fair argument and this PR would make the code easier to read.
I will mark this as a breaking change because it might affect people relying on configuration.apiKey
being used as an accessToken.
circle ci failure is unrelated |
Breaking changes without fallback should be merged into 5.0.x branch instead. I do not plan to revert this. Let's see if the users have an opinion on this change. |
PR checklist
./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.master
,4.1.x
,5.0.x
. Default:master
.Description of the PR
There was a typescript error, because only
this.configuration.accessToken
function takes two arguments,this.configuration.apiKey
doesn't. Moreover,apiKey
is unnecessary in bearer mode, and it is used a few lines below.@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)