8000 Add support for axios cancellation token by MariuszKogut · Pull Request #2846 · RicoSuter/NSwag · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add support for axios cancellation token #2846

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 15, 2020
Merged

Add support for axios cancellation token #2846

merged 1 commit into from
May 15, 2020

Conversation

MariuszKogut
Copy link
Contributor
@MariuszKogut MariuszKogut commented May 15, 2020 •
edited
Loading

Fixes #2844

I have also noticed that src/NSwag.CodeGeneration.TypeScript.Tests/AxiosTests.cs did not use the axios template. I have switched to axios to make sure, that everything works fine.

function isAxiosError(obj: any | undefined): obj is AxiosError {
  return obj && obj.isAxiosError === true
}

was tweaked, because I noticed, that the thrown error is undefined, if an ongoing was canceled.

Please let me know if I have to tweak something.

@RicoSuter RicoSuter merged commit 305da24 into RicoSuter:master May 15, 2020
@RicoSuter
Copy link
Owner

LGTM, thanks for the PR.

@NewDark90
Copy link

@MariuszKogut Yep, I think there's a problem with it being passed into the options...

Example generation:

let options_ = <AxiosRequestConfig>{
            method: "GET",
            url: url_,
            headers: {
                "Accept": "text/plain"
            },
            cancelToken
        };

It's subtle, and syntactically sound, but a problem.
cancelToken should be cancelToken: cancelToken.

cancelToken is valid as undefined, so the property not getting a value is valid, technically.

@NewDark90
Copy link

Also, it would be cool if that bit checked to make sure that the CancelToken parameter exists before writing that out. If someone were to opt out of the functionality with excludedParameterNames or some other possible configuration, that shouldn't break.

@RicoSuter
Copy link
Owner

@NewDark90 Can you create a PR with this change?

@MariuszKogut
Copy link
Contributor Author
MariuszKogut commented Aug 6, 2020 •
edited
Loading

Hey @NewDark90

I have tested this implementation in one of my applications using the cancel token behavior of Axios and it works fine. cancelToken in an object literal short hand and will be transpiled into cancelToken: cancelToken by typescript (when your target is es5):

image

A parameter opt out is not necessary in my point of view. It's an optional parameter therefore undefined will be passed and everything is as before. So in my point of view it's already opted out. ;-)

Any thoughts or did I missed something?

@NewDark90
Copy link
NewDark90 commented Aug 6, 2020 via email

@nulladdict nulladdict mentioned this pull request Oct 9, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add CancelToken to AxiosClient
3 participants
@MariuszKogut @RicoSuter @NewDark90
0