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

[TypeScript] typescript-axios: Added possibility to add custom axios instance. #1274

Merged

Conversation

sruehl
Copy link
Contributor

@sruehl sruehl commented Oct 19, 2018

This comes in handy if you want to use mocks in tests.

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 and ./bin/security/{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\.
  • Filed the PR against the correct branch: master, 3.4.x, 4.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

This PR adds the possibility to supply a custom axios instance to the typescript-axios clients.
This then can be used to supply a axios mock adapter in unit-tests: const mock = new MockAdapter(axiosInstance, { delayResponse: 600 }).
To test this feature a simple test-case has been added which just supplies the global axios instance.

Updated remark (2018/10/22): One thing we could do different is to supply the instance via configuration or constructor parameter so one doesn't need to supply the instance on every call.

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

This comes in handy if you want to use mocks in tests.
@wing328
Copy link
Member

wing328 commented Oct 21, 2018

@nicokoenig does the enhancement look good to you?

@nicokoenig
Copy link
Contributor

@sruehl @wing328 Looks good to me! 🔥

3 similar comments
@nicokoenig
Copy link
Contributor

@sruehl @wing328 Looks good to me! 🔥

@nicokoenig
Copy link
Contributor

@sruehl @wing328 Looks good to me! 🔥

@nicokoenig
Copy link
Contributor

@sruehl @wing328 Looks good to me! 🔥

@nicokoenig
Copy link
Contributor

@wing328 @sruehl Look good to me 👍🔥

@nicokoenig
Copy link
Contributor

@wing328 @sruehl looks good to me 👍🔥

@sruehl
Copy link
Contributor Author

sruehl commented Oct 22, 2018

I just added a alignment to fetch regarding the custom instance but somehow my comment, my commit and everything else doesn't appear here? github bug? last commit should be: typescript-axios: aligned to fetch API for custom instance 07c5b30

@sruehl sruehl force-pushed the feature/typescript_custom_axios_instance branch from 07c5b30 to c7e4f7d Compare October 22, 2018 09:25
@sruehl
Copy link
Contributor Author

sruehl commented Oct 22, 2018

Ok time to clean up this PR a bit: https://blog.github.com/2018-10-21-october21-incident-report/ :)

@sruehl
Copy link
Contributor Author

sruehl commented Oct 22, 2018

@nicokoenig could you recheck after my c7e4f7d commit? I updated the api according to the fetch example so its more consistent. Due to the github outage we had a race-condition.

@wing328
Copy link
Member

wing328 commented Oct 22, 2018

@sruehl quick question if you don't mind. Is it correct to say that your change is backward-compatible (non-breaking change) as you just added an optional parameter to the API method signature?

@nicokoenig
Copy link
Contributor

@sruehl I will give it a try when I am in the office tomorrow.

@wing328 I will validate backward compatibility with our project, once I am in the office again.

👍

@sruehl
Copy link
Contributor Author

sruehl commented Oct 23, 2018

@wing328 yep that's the idea (the supplied instance is optional).

@nicokoenig
Copy link
Contributor

@sruehl @wing328 works like a charme! ✔️

@wing328
Copy link
Member

wing328 commented Oct 23, 2018

@nicokoenig thanks for the review.

@sruehl thanks for the enhancement.

@wing328 wing328 merged commit 51d2e4b into OpenAPITools:master Oct 23, 2018
@sruehl sruehl deleted the feature/typescript_custom_axios_instance branch October 23, 2018 08:48
@wing328
Copy link
Member

wing328 commented Oct 31, 2018

@sruehl thanks for the PR, which is included in the v3.3.2 release: https://twitter.com/oas_generator/status/1057649626101112832

A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
…instance. (OpenAPITools#1274)

* typescript-axios: Added possibility to add custom axios instance.
This comes in handy if you want to use mocks in tests.

* typescript-axios: aligned to fetch API for custom instance
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants