fix: address compatibility issues with axios 1.7.8 (#953)

[ArnaudBuchholz]

• Added typescript validation during the unit tests
• Fixed the issues detected during typescript validation

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.60%. Comparing base (4626ec0) to head (7b10ca9).
Report is 27 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #958   +/-   ##
=======================================
  Coverage   99.60%   99.60%           
=======================================
  Files          19       19           
  Lines        2509     2510    +1     
  Branches      223      221    -2     
=======================================
+ Hits         2499     2500    +1     
  Misses         10       10           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[arthurfiorette]

this will always be present

[ArnaudBuchholz]

same here : that is exactly what breaks the compatibility with axios 1.7.8

[ArnaudBuchholz]

By the way, I modified the test to assess this id is present.

[arthurfiorette]

but where does it break the compatibility? that's where you need to fix it, an interface cannot block its extension to add new fields.

[ArnaudBuchholz]

You are right, the problem appears when using this interface, after removing the optional flag, here is the transpiler message :

The types of 'interceptors.response.use' are incompatible between these types.
    Types of parameters 'onFulfilled' and 'onFulfilled' are incompatible.
      Type 'AxiosResponse<any, any> | Promise<AxiosResponse<any, any>>' is not assignable to type 'CacheAxiosResponse<any, any> | Promise<CacheAxiosResponse<any, any>>'.
        Type 'AxiosResponse<any, any>' is not assignable to type 'CacheAxiosResponse<any, any> | Promise<CacheAxiosResponse<any, any>>'.
          Type 'AxiosResponse<any, any>' is missing the following properties from type 'CacheAxiosResponse<any, any>': id, cached

because of the following :

export interface AxiosCacheInstance extends CacheInstance, AxiosInstance {
  /* ... */
  interceptors: {
    request: AxiosInterceptorManager<InternalCacheRequestConfig>;
    response: AxiosInterceptorManager<CacheAxiosResponse>;
  };

I noticed that in the InternalCacheRequestConfig, id and cache were optional, I assumed it was to solve the same issue.

But I get your point : if anybody relies on the CacheAxiosResponse interface, this represents a breaking change. However, since it is not explicitly documented, I assumed it was OK.

[arthurfiorette]

There is a dedicated doc page just for these 2 fields: https://axios-cache-interceptor.js.org/config/response-object.

Isn't there another way to fix it?

Probably just as I did with InternalCacheRequestConfig

[ArnaudBuchholz]

Actually, reviewing the implementation, theCacheRequestConfig uses optional on id and cache as documented here : https://axios-cache-interceptor.js.org/config/request-specifics

[ArnaudBuchholz]

I pushed a new version that keeps backward compatibility but it requires many changes across the code. The most important is to ensure that, in the end, the onFulfilled receives the CacheAxiosResponse response... which is the case.

I am not able to evaluate if this change can break anybody implementation (do people rely on the AxiosCacheInstance interface ?), but it works on my use.

[arthurfiorette]

Having a single ts-expect-error comment covering a single field is safer than casting the whole object into the shape you want.

[ArnaudBuchholz]

OK, proposing a different approach to remove TypeScript error without special handling

[arthurfiorette]

would be great if you could keep the CacheAxiosResponse type here.

[ArnaudBuchholz]

Unfortunately changing this cascades up to the initial problem : the definition of AxiosCacheInstance.
Can you elaborate on why you want this responseInterceptor type to be using CacheAxiosResponse, it may help me understand what are my options ?