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
Revert #3002 - set type of data to any #4141
Comments
+1 for this. |
This seems related to #4109 |
It’s easy to specify the response data should be any. const { data } = axios.get<any>('/');
// data is now any I asked on the TypeScript Discord: Lets say you have a TypeScript function which could return anything, for example a library for making HTTP requests. The return type can be configured using a generic for convenience. Which would be the preferred default return type? any or unknown? Everyone immediately responsed
It’s easy to reduce type safety by specifying the response data is
This really confuses me, as it seems to contradict your own point. You want to disallow the literal of
What exact API do you mean by
I don’t think the user’s options are restricted. They can use Although in my experience TypeScript users prefer type safety over |
I don't get this type at all, why is input and output the same data?
Edit: I just found #4109 (comment) .. (y) please get axios in good shape again. Thanks :-) |
I want to clarify that #4109 is unrelated to this issue. Linking them to each other is confusing. |
You are right, setting it to any is just the only solution to the issue with same input/output type right now. Therefore they are at least "similar" for people who are looking for the issue ;-) And the title still refers to a similar issue: it's a breaking interface change. Something that was any before, is now something that needs to be defined. This breaks a lot of code base. |
Closing as the issue will be reverted in 0.23 |
@jasonsaayman Do you have an idea of when 0.23 is slated to be released on npm? 0.22 fixes a bug that was affecting us but introduces this, so we can't use it. Right now we're just using a forked version of 0.22 with fixed types, but I'd much rather use the official version 😅 |
The unknown type is still a breaking change in master, If you are looking for alternatives I really can recommend gaxios (which is based on fetch, therefore http/2 etc is supported) or I also just created haxios which is a wrapper on gaxios but extends the interface to be more identical to the original axios.. haxios is still in a very early stage though. |
Was this a lie or what?? Reverting doesn’t mean changing the type to unknown… |
@ImRodry please open a pull request if you still have an issue with the way types are handled. Also note that should you continue to not follow our code of conduct I will be forced to ban you. Thanks |
Is this the right place to continue the discussion @jasonsaayman ? (If not, could I be directed on what to do next, eg. open a new issue? 😅) I have mixed feelings about this. Personally, I'm not a big fan of the change to
|
@ITenthusiasm thanks for your long and explanative examples above. Point taken. Honestly TypeScript in Axios is very much something that has been bolted on and is not at the core of how the library functions, in fact the With that being said, I felt that using @remcohaszing I would like to get your opinion on this too if possible? |
I get that. I think React JSS ran into a similar issue too. This is the really hard part of awesome libraries like Thanks for re-opening! |
The various TypeScript related changes in axios 0.22.0 has left many users confused. Mostly because it included some bugs, making axios unusable. These bugs are unrelated and shouldn’t be confused with the discussion of Also the change from Now that most type related bugs in 0.22.0 have been fixed, I feel like this is a proper time to discuss this. First of all I would like to point out this issue is highly subjective, but there are some arguments to make for selecting either default. The change has received both positive and negative feedback. I agree this is about developer experience, but at a different level. Given the replies so far, it has become clear different people have different opinions about what a good developer experience means. Personally I like the added type safety: it has already helped me catch a small bug. The reason I think that Let’s say we have the following code const { data } = await axios.get('/user');
console.log(user.name); Users who prefer const { data } = await axios.get<User>('/user');
console.log(user.name); If If the developer prefers to opt-out of type safety, they can still do so, even if the default type is const { data } = await axios.get<any>('/user');
console.log(user.name); As for keeping ESLint happy, I feel that using
A linter should help increase code quality by making code adhere to a code standard. If for a project the choice was made to decrease type safety by allowing the use of
This statement was about preferring
|
I'll handle the quoted responses last. I agree that this is a subjective topic. But it's for that very reason that I don't find the use of That is, I'm not just evaluating preferences and positives. I'm evaluating negatives. Type purists will already be using the type casting that would be forced by returning The rest of the users are significantly more inconvenienced. If you read my earlier statement, you'll see that there can easily be scenarios where a person does not have access to officially defined types (or DT types). And these scenarios, especially when working with simple props like ImRody mentioned, result in unnecessary effort. We have to create more files/types, and we have to type cast everywhere. It's also important to remember that changing to It's also worth considering that the common standard for native APIs has been to use Considering that the type was previously Quoted Responses
I doubt that, since the Discord user was encouraging the use of
Going from Thanks for the |
To touch further on that breaking changes note... For comparison today:
Very close, with TS being slightly larger. Even TS devs have times when they acknowledge previous decisions that were probably mistakes. But even then, they consider the impact of a breaking change on existing users. (Not that there are never breaking changes, of course. Value, ease of fixing, how many benefit vs how many get bitten, and more come into play.) Though related to whether or not we should go back to |
👋🏻 I got pinged about this issue as I generally have to think about this sorta stuff for the DOM APIs which have similar impact scales for changes (4.4 was a particularly rough transition) I generally side on It's a set of trade-offs and tensions. If it came out as IMO, cases like this (as with |
The change was made from version 021.0 to 0.23.0 (The types in 0.22.0 were very broken, so let’s ignore those). As far as can tell from semver and the axios upgade guide, it’s ok to make breaking changes. If the Axios team disagrees with this, I definitely think the default type should be reverted to /cc @jasonsaayman microsoft/TypeScript#46347 looks very promising. That would end this discussion and similar discussions for other libraries with a compiler setting. If that get’s implemented, the correct type wouldnt be I think all arguments have been made. Ultimately the choice is up to the Axios team. |
I'm also interested in the issue orta created. Since it may take some time until that suggestion gets approved, developed, merged, and released, my vote is for switching back to From what jasonsaayman said, it seems that the hope was to benefit the consumers of the |
Well, there's one more thing I missed. 😅 As for clarity on this statement:
The problem that ImRody and I mentioned is that passing Many people got confused about ImRody saying he didn't want to use generic All that to say, using
This applies for users writing their own code. But again, the situation is different when it involves a library and consumers. Consumers like ImRody and I do like to have good types. I actually take types that I can control very seriously. And I even helped ReactJSS improve its types in the past. But as far as this discussion is concerned, |
Simply put, no, because the point of using unknown is to override it with a custom type, you can't even access or work with an unknown variable if it's explicitly typed as unknown. The same is also not possible/good practice in most projects due to linter rules that disallow the explicit use of |
I can see why you think this. The problem here is what Both
If the default is If the default is The TypeScript community is divided on this. Many similar libraries use
This is an interesting thought, but I doubt anyone wants to create an axios wrapper function just to change the default of the generic. Axios isn’t the only library which provides function that return |
I am unfortunately/fortunately (depending on where you allegiances lie) going to revert this back for now to use I want to thank everyone in this thread, it was pretty heated and a lot of people contributed quite a bit to this, all the opinions expressed are valued. I will cut a 0.24 to release this. |
Not to spark any discussion whatsoever, but I just wanted to let you guys know that I actually loved the change to I would love to see the |
Yeah some kind of RFC leading up to v1 could be useful. (Or the discussion here could be re-used for data when v1 comes along too, I suppose. 🤔) If microsoft/TypeScript#46347 gets addressed in time, it may not even need to be a big discussion. |
So it's not a sure thing that v1 will have unknown? In that case, let me give my two cents: I think unknown should be the default for data, and with all due respect to those discussing whether unknown or any is better, that's not the point. The point I think can be shown through a comparison:
In summary, (1) results in visible problems for some people, and (2) results in invisible problems for some people. Some people are going to have problems either way. I would argue that causing users visible problems is better than causing users invisible problems and that therefore (1) default unknown is the best choice. That being said, I'm happy to wait until v1 cuz breaking changes.
The problem is that those who want to do |
TypeScript is only as good as the developer that uses it. Per the official TypeScript docs regarding the use of
And from the beginning of the thread:
If it is so simple, why not just declare the types? That just sounds lazy and incompetent. But on the same coin, failing to provide types just results in technical debt, a future debt repaid when a new developer comes on to the project and works with the axios logic written by someone who didn't want to take a few minutes to declare the types of "extremely simple responses". |
Types not fixed yet
|
@popuguytheparrot that issue is completely unrelated to the original issue reported here |
See axios/axios#4141 and axios/axios#3002 (cherry picked from commit a817670)
Is your feature request related to a problem? Please describe.
Yes. After #3002 was merged, we are now required to set explicit type definitions for every single axios request we make, even if we don't want to, because these would result in an error.
Describe the solution you'd like
The default type for AxiosResponse#data should be any so that we can modify it if we want, or keep it as is if we don't care about the typings for that specific request
Describe alternatives you've considered
As suggested in that PR, we could set the type to
any
explicitly, however, many people use typescript-eslint's no-explicit-any rule that prevents us from doing this. I believe everyone should be able to choose what types they wanna work with, because for extremely simple responses from which you only need one or two simple properties you don't always need explicit type declarationsAdditional context
TypeScript's DOM library sets the return type of Body#toJSON to Promise, so we should be following the native approach and not restricting user's options by setting a default that "most users may want", even if most users do not want this.
The text was updated successfully, but these errors were encountered: