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

breaking interface change #4109

Closed
andrei9669 opened this issue Oct 1, 2021 · 6 comments · Fixed by #4116
Closed

breaking interface change #4109

andrei9669 opened this issue Oct 1, 2021 · 6 comments · Fixed by #4116
Assignees

Comments

@andrei9669
Copy link

Describe the bug

In #2995 merge request, there was introduced a bug.

There came 2 issues.

  1. input and output types have to be the same.
  2. existing usages are broken

To Reproduce

pass input and output type to axios.post

interface Input {
  name: string;
}
interface Output {
  value: string;
}
/* existing usages are broken
 * TS2345: Argument of type '{ name: string; }' is not assignable to parameter of type 'undefined'.
 */
const { data } = await axios.post<Output>('url', {
  name: 'hello',
});
/* Input and output have to be the same type
 * TS2345: Argument of type '{ name: string; }' is not assignable to parameter of type 'AxiosResponse<Output>'.   
 * Object literal may only specify known properties, and 'name' does not exist in type 'AxiosResponse<Output>'.
 */
const { data } = await axios.post<Input, AxiosResponse<Output>>('url', {
  name: 'hello',
});

Expected behavior

No type errors.

Environment

  • Axios Version 0.22.0
  • Adapter HTTP
  • Browser Chrome
  • Browser Version 94.0.4606.61
  • Node.js Version v14.17.5
  • OS: Windows 10
  • Additional Library Versions none

Additional context/Screenshots

@ChristianIvicevic
Copy link

Why is this closed? I just noticed that the type for post is erroneous as you explained and causes compilation issues on my end:

export class Axios {
  post<T = never, R = AxiosResponse<T>>(url: string, data?: T, config?: AxiosRequestConfig<T>): Promise<R>;
}

@andrei9669
Copy link
Author

andrei9669 commented Oct 1, 2021

@ChristianIvicevic yeah, sorry about that, I checked the merge request and there was link to ts sandbox and it seemed to work there so I thought it was issue on my end. will reopen.

@andrei9669 andrei9669 reopened this Oct 1, 2021
@caugner
Copy link
Contributor

caugner commented Oct 1, 2021

Same here, observed in the failing CI run of Dependabot's PR:

    src/lib/ApiClient.ts:99:17 - error TS2345: Argument of type '{ state_event: string; }' is not assignable to parameter of type 'LimitedInvitation'.
      Object literal may only specify known properties, and 'state_event' does not exist in type 'LimitedInvitation'.
    99                 state_event: 'accept',
                       ~~~~~~~~~~~~~~~~~~~~~

Here, LimitedInvitation is the expected return type, whereas { state_event: string; } is the parameter type:

    interface LimitedInvitation {
        token: string
        state: string
    }

    async invitationAccept(token: string): Promise<LimitedInvitation> {
        return axios
            .patch<LimitedInvitation>(`/api/invitations/${token}`, {
                state_event: 'accept',
            })
            .then(({ data }) => data)
    }

@rijkvanzanten
Copy link
Contributor

Ref #3002 (the original PR that included this change)

@tjhorner
Copy link

tjhorner commented Oct 5, 2021

@rijkvanzanten I don't think that was the PR that introduced this issue. The issue is caused by the data parameter's type changing from any to T (the response type) — that PR changes the default type for T to never, but doesn't touch data.

It looks like data's type was changed to T in #2995.

@remcohaszing
Copy link
Contributor

I would really like to point out that:

jasonsaayman added a commit that referenced this issue Oct 6, 2021
Fixes #4109.

Co-authored-by: Jay <jasonsaayman@gmail.com>
mbargiel pushed a commit to mbargiel/axios that referenced this issue Jan 27, 2022
Fixes axios#4109.

Co-authored-by: Jay <jasonsaayman@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants