Make the default type of response data never

[remcohaszing]

This requires TypeScript users to explicitly define the type of the data they
are consuming.

Before this, data was any by default. This means TypeScript consumers didn’t
get type safety if they forgot to specify the type.

Technically this is a breaking change for TypeScript users, as this will report
errors if they forgot to specifiy the response type. The simplest workaround
would be to explicitly set the response type to any, so it’s not breaking
much.

The unknown type is probably a slightly better fit, but this requires
TypeScript ^3.

data is still any in the very specific use case mentioned in
microsoft/TypeScript#38969

[carloschida]

I do agree with you in that unknown would be the optimal choice.

Having said that If we have to choose between defaulting to never or to any, I'd got for the latter.
The reason is that users that want type safety are already overriding the default, whereas users that don't want it will be just confused as to why they should declare it.

Additionally, if one were to re-write axios in TS, I'm convinced that never wouldn't be the best choice for the functions that employ AxiosResponse.

[rijkvanzanten]

@jasonsaayman Mind adding this to the release notes? It's currently omitted from https://github.com/axios/axios/blob/master/CHANGELOG.md#0220-october-01-2021

[jasonsaayman]

@rijkvanzanten this change has not been released, for 0.22.0 we worked in a separate branch from master, this change will probably only make it into v1

[rijkvanzanten]

@jasonsaayman Are you sure about that? Looking at the axios folder in my node_modules for 0.22.0, the "AxiosResponse" type is already set to "never" as per this PR 🤔

In 0.22.0:

https://unpkg.com/axios@0.22.0/index.d.ts

..

export interface AxiosResponse<T = never>  {

..

In 0.21.4:

https://unpkg.com/axios@0.21.4/index.d.ts

..

export interface AxiosResponse<T = any>  {

..

[TheVaan]

I can confirm that this change is inside 0.22.0 - I've the same issue and was surprised about breaking changes in 0.22.0 without marking it breaking in the change log.

[ImRodry]

Please undo this PR. The never type is for things that can NEVER be present, not for something that has an unknown type. Typescript v3 is pretty old and there is no reason to support that, however, this should remain as any because any is a type that is supposed to be mutated, unlike unknown which acts as a "We don't know what this is, please don't touch this property" which is very clearly not the case. I changed to axios because node-fetch did this stupid change and now you guys are doing it too. Please don't kill your package

[jasonsaayman]

Ok well if this is in that release that is a pretty large issue, I will check out the fixes provided and try release asap. Someone must have not worked from the given 0.22 branch.

[remcohaszing]

Let’s change this to unknown. The lowest supported TypeScript version by DefinitelyTyped is 3.7, so I think it’s safe to match that.

I would make a PR, but it’s going to have merge conflicts with #4116, so I’ll wait.

[huineng]

why not just leave it to 'any', even changing it to unknown is a 'breaking' change as we will have to do several modifications to our code.

before ...
if (response.data?.message) {
// do something
}

after 
if ((response.data as any)?.message) {
// do something
}

[remcohaszing]

This change provides better type safety, which is what most TypeScript users want.

Changing it from any to never was slightly breaking, which is why it was released as semver minor (below 1.0.0). Changing it from never to unknown isn’t.

Looking at your example, your code should look like this:

interface MyResponseData {
  message: string;
}

const response = await axios.get<MyResponseData>('…');

if (response.data.message) {
  // do something
}

But if you really want to lose type safety, you could still make it explicit:

const response = await axios.get<any>('…');

if (response.data.message) {
  // do something
}

[ImRodry]

Why are you the ones to decide what TypeScript users want? In my code I fetch an API that returns one JSON object with 2 string properties, of which I only need to use one of them, so it makes no sense to make an interface for this. Having to add an explicit any is even more nuts and some of us have rules that disallow that. The library shouldn’t have a say in what users want, and should just let them use it in whichever way they want, and this change is a big step in the wrong direction

[rijkvanzanten]

and this change is a big step in the wrong direction

This is very subjective. I for one welcome this change, but that doesn't mean that "The TypeScript Community" as a whole wants this.

[ImRodry]

Well but that’s why the package should allow everything and let the users decide what they wanna do, not do shit for them because you don’t always need an explicit type assertion

[rijkvanzanten]

Well but that’s why the package should allow everything and let the users decide what they wanna do, not do shit for them because you don’t always need an explicit type assertion

You're still allowed to do anything you'd like, this is just a matter of deciding what the better default is for the project as a whole 🤷🏻

[ImRodry]

No we’re not allowed to do what we want because it’s good practice not to allow explicit any types and most of us have that rule set. This change requires us to make an interface for every damn axios request we make

[rijkvanzanten]

No we’re not allowed to do what we want because it’s good practice not to allow explicit any types

Wait, so you're arguing that people should NOT use any, and therefore axios should default to any? If anything, your example shows why you'd be in favor of this change, not against it 😅

Anywho, lets move this discussion to a new issue or elsewhere on the internet, as I'm sure it'll get old pretty quick to get notifications from people bickering over this on a closed PR 👍🏻

[bratanon]

I agree with @ImRodry on this one.

We are also facing this issue where we don't allow explicit any, which forces us to create interfaces for every request.

Wait, so you're arguing that people should NOT use any, and therefore axios should default to any?
@rijkvanzanten YES, and it makes perfect sense. Axios might not have the rule mentioned above, and that is ok, but don't force that on to all our consumers.