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

Typescript definitions for AxiosInterceptorOptions included in response interceptors #5074

Open
amitsainii opened this issue Oct 9, 2022 · 5 comments · May be fixed by #5237
Open

Typescript definitions for AxiosInterceptorOptions included in response interceptors #5074

amitsainii opened this issue Oct 9, 2022 · 5 comments · May be fixed by #5237

Comments

@amitsainii
Copy link
Contributor

amitsainii commented Oct 9, 2022

Describe the bug

With Axios 1.x we introduced an optional third argument for our interceptor use function. When I looked at the source code, I realized this third param of type AxiosInterceptorOptions is only used for request interceptor.
The issue I'm reporting is related to the typescript definition for the use method. Currently, when it comes to this third parameter, type definitions don't distinguish between request and response interceptors. Because of this currently, this third argument can be used on the response interceptor as well without typescript yelling at you. This is misguiding for our consumers as they may end up using runWhen on response interceptors which won't do anything.

To Reproduce

In a typescript project try writing the below code

Axios.interceptors.response.use((response) => response.data, undefined, {
  runWhen: (config) => config.method === 'GET',
});

Here users will expect that their response interceptor should only be fired when the request has the GET method, but in reality, this third argument would have no effect on the response interceptor.

Expected behavior

Typescript should report an error when using this third argument with a response interceptor.

Solution

This can be easily achieved by replacing the following type definition

export interface AxiosInterceptorManager<V> {
  use(onFulfilled?: (value: V) => V | Promise<V>, onRejected?: (error: any) => any, options?: AxiosInterceptorOptions): number;
  eject(id: number): void;
  clear(): void;
}

with the one mentioned below

type AxiosRequestInterceptorUse<T> = (onFulfilled?: (value: T) => T | Promise<T>, onRejected?: (error: any) => any, options?: AxiosInterceptorOptions) => number;

type AxiosResponseInterceptorUse<T> = (onFulfilled?: (value: T) => T | Promise<T>, onRejected?: (error: any) => any) => number;

export interface AxiosInterceptorManager<V> {
  use: V extends AxiosRequestConfig ? AxiosRequestInterceptorUse<V> : AxiosResponseInterceptorUse<V>;
  eject(id: number): void;
  clear(): void;
}

I can push my changes in a PR the moment you guys are on board with this.

Alternate Path

We may also consider incorporating features like runWhen for response interceptors as well. This is one of my 2 suggestions on Ability to skip request/response interceptors on per request basis. I already created a PR for another type issue with use method. But I've created this issue separately because we can go for implementing these for response interceptors as well.

Environment

  • Axios Version [e.g. 1.1.2]
  • Adapter [e.g. XHR/HTTP]
@amitsainii
Copy link
Contributor Author

Hi, @jasonsaayman I'd love to know the reasoning behind this getting closed. No rush though, I know you're kind of donating your free time here

@jasonsaayman
Copy link
Member

Will have a look as soon as possible. I just presumed that should take care of all types however I see what you said on your PR so I will test it out

@amitsainii
Copy link
Contributor Author

Sounds good @jasonsaayman. BTW #5070 is the issue that is tied to that PR. This is a separate issue related to typings on the response interceptor.

@ForeshadowRU
Copy link

Aren't there plans for adding such runWhen option to response interceptors too?
I would use this feature on project to parse server response with response interceptor based on responseType === 'blob', while other response interceptor can still work with json.

@amitsainii
Copy link
Contributor Author

amitsainii commented Dec 9, 2022

Aren't there plans for adding such runWhen option to response interceptors too? I would use this feature on project to parse server response with response interceptor based on responseType === 'blob', while other response interceptor can still work with json.

@ForeshadowRU I did start a discussion in the same direction. If @jasonsaayman agrees, I can even PR that feature myself 👍

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