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

Exposing the Axios constructor in index.d.ts #2872

Merged
merged 2 commits into from Sep 5, 2021

Conversation

@TimWolla
Copy link
Contributor

@TimWolla TimWolla commented Apr 2, 2020

This patch allows TypeScript users to extend the Axios class without
the type checker complaining.

see 7548f2f


Fixes #3017

@TimWolla
Copy link
Contributor Author

@TimWolla TimWolla commented May 7, 2020

Could you please take a look? After I used the “Update branch” button on April, 8th Travis is green instead of a stuck yellow as well.

Loading

@donaldpipowitch
Copy link

@donaldpipowitch donaldpipowitch commented Jun 15, 2020

@chinesedfan Is there something which still needs to be done for this MR?

Loading

@ArrayKnight
Copy link

@ArrayKnight ArrayKnight commented Jun 26, 2020

Would it be possible to get this merged soon, please?

Loading

@jasonsaayman jasonsaayman self-assigned this Jun 26, 2020
@jasonsaayman
Copy link
Member

@jasonsaayman jasonsaayman commented Jun 26, 2020

Will look at this as soon as I can

Loading

@TimWolla TimWolla force-pushed the typescript-axios-constructor branch from 11d3801 to ab71639 Aug 31, 2020
@TimWolla TimWolla force-pushed the typescript-axios-constructor branch from ab71639 to 5cb4987 Sep 17, 2020
@TimWolla
Copy link
Contributor Author

@TimWolla TimWolla commented Oct 27, 2020

@remcohaszing Can you return the favor and review this one, please? The reason behind this change is being able to class Foo extends Axios with import { default as axios, Axios } from 'axios';.

It should be non-breaking, but a second pair of eyes would be helpful.

Loading

@remcohaszing
Copy link
Contributor

@remcohaszing remcohaszing commented Oct 27, 2020

I looked into it. This allow to use the following statements that were previously not possible:

import axios from 'Axios';

const foo = new axios.Axios();  // Use Axios constructor
foo instanceof axios.Axios;  // Instanceof check for Axios instance
foo('/');  // This is unsupported, and now also an error in TypeScript

The instanceof check doesn’t work for instances created axios.create(), but the call expression does work in that case.

All of these cases are properly handled by the new type definitions, and it’s non-breaking. LGTM 😃

Loading

@TimWolla TimWolla force-pushed the typescript-axios-constructor branch from 5cb4987 to 423f1ab Oct 27, 2020
@TimWolla
Copy link
Contributor Author

@TimWolla TimWolla commented Oct 27, 2020

@jasonsaayman I'd appreciate if you could take a look at this one. I'm happy to answer all your questions about it, like we did in #2797 👍

Loading

@TimWolla TimWolla force-pushed the typescript-axios-constructor branch from 423f1ab to c367d83 Dec 11, 2020
@TimWolla TimWolla force-pushed the typescript-axios-constructor branch from c367d83 to 36b6114 Jan 12, 2021
@TimWolla
Copy link
Contributor Author

@TimWolla TimWolla commented Jan 12, 2021

@jasonsaayman Friendly ping. I just rebased this PR once again and it's already been reviewed by Remco.

Loading

@TimWolla TimWolla force-pushed the typescript-axios-constructor branch from 36b6114 to cef5420 Mar 1, 2021
@TimWolla TimWolla force-pushed the typescript-axios-constructor branch from cef5420 to 61db8ad Apr 30, 2021
This patch allows TypeScript users to extend the `Axios` class without
the type checker complaining.

see 7548f2f
@TimWolla TimWolla force-pushed the typescript-axios-constructor branch from 61db8ad to c72c87a Aug 13, 2021
@TimWolla
Copy link
Contributor Author

@TimWolla TimWolla commented Aug 13, 2021

@jasonsaayman Any chance? I just rebased the PR onto the latest master to allow for the GitHub Actions to run. You'll need to approve them once, though, because technically I'm a new contributor.

Loading

@jasonsaayman
Copy link
Member

@jasonsaayman jasonsaayman commented Sep 5, 2021

@TimWolla thanks for being so tenacious, I am merging this now 🥳

Loading

@jasonsaayman jasonsaayman merged commit 4f25380 into axios:master Sep 5, 2021
3 checks passed
Loading
@TimWolla
Copy link
Contributor Author

@TimWolla TimWolla commented Sep 5, 2021

Thanks! 🚀

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

6 participants