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

POST call expected response body type also set to request body type #4132

Closed
peetjvv opened this issue Oct 4, 2021 · 6 comments · Fixed by #4140
Closed

POST call expected response body type also set to request body type #4132

peetjvv opened this issue Oct 4, 2021 · 6 comments · Fixed by #4140

Comments

@peetjvv
Copy link

peetjvv commented Oct 4, 2021

Describe the bug

When specifying an expected response.data type to the post method it also sets that expected type to the request body. Also, when not specifying a type, the type for the response.data gets inferred to be the same as the type of the request body. This leads to typescript errors in VS Code and during compile time when the response and request bodies in code doesn't have the same type (which is usually the case). This was not an issue in v0.21.4 - which I reverted to in order to get around this issue.

To Reproduce

  1. Install axios v0.22.0
  2. Add a type for the response.data like in this example:
axios.post<ExpectedResponseBody>(`post url`, { foo: "bar" }, {});

Also look at the CodeSandbox example here

Expected behavior

When specifying the expected type of the response.data in the post call, the request body shouldn't have the same expected type. And the inferred type for the request body shouldn't automatically get applied to response.data.

Environment

  • Axios Version 0.22.0
  • Adapter N/A
  • Browser N/A
  • Browser Version N/A
  • Node.js Version: 14.17.3
  • OS: Win 10
  • Additional Library Versions: Typescript 4.4.3

Additional context/Screenshots

Type for post method in axios v0.21.4
image

Type for post method in axios v0.22.0. Note that T is also set to request data and config in this version - which is what I believe is the cause of the issue.
image

@alessandrojcm
Copy link

Same problem here when updating to 0.22; it literally broke the whole app.

@TheVaan
Copy link

TheVaan commented Oct 4, 2021

Duplicate to #4109

@peetjvv
Copy link
Author

peetjvv commented Oct 4, 2021

Duplicate to #4109

Thank you. I couldn't find an existing issue for this when I started writing up my issue on Saturday.

@TheVaan
Copy link

TheVaan commented Oct 4, 2021

You're welcome. Axios team messed up with the last release. Maybe they deserve issue overflow 😶

@tjhorner
Copy link

tjhorner commented Oct 5, 2021

Axios team messed up with the last release. Maybe they deserve issue overflow 😶

Yeah it's frankly concerning how this got through. We all make mistakes but this is a big one to miss 🙃

It looks like their TypeScript tests may be incorrectly written? I'm not sure, and I don't really want to dig into it... but I'm not sure why the tests didn't catch this.

axios.post<Partial<UserCreationDef>, string>('/user', { name: 'foo' }, { headers: { 'X-FOO': 'bar' } })
.then(handleStringResponse)
.catch(handleError);

If I'm not mistaken, string should actually be AxiosResponse<string> here.

@peetjvv
Copy link
Author

peetjvv commented Oct 18, 2021

Axios team messed up with the last release. Maybe they deserve issue overflow 😶

Yeah it's frankly concerning how this got through. We all make mistakes but this is a big one to miss 🙃

It looks like their TypeScript tests may be incorrectly written? I'm not sure, and I don't really want to dig into it... but I'm not sure why the tests didn't catch this.

axios.post<Partial<UserCreationDef>, string>('/user', { name: 'foo' }, { headers: { 'X-FOO': 'bar' } })
.then(handleStringResponse)
.catch(handleError);

If I'm not mistaken, string should actually be AxiosResponse<string> here.

Your assumption on the type is incorrect. it should be axios.post<string> which then returns a response of type AxiosResponse<string>. But good point on the tests - definitely should've picked this up.

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

Successfully merging a pull request may close this issue.

4 participants