arthurfiorette / axios-cache-interceptor

📬 Small and efficient cache interceptor for axios. Etag, Cache-Control, TTL, HTTP headers and more!
https://axios-cache-interceptor.js.org
MIT License
703 stars 58 forks source link

Bug: Breaking change in 1.6.0 #906

Closed JonathanViau closed 1 month ago

JonathanViau commented 1 month ago

What happened?

In the latest 1.6.0 release, a breaking change has been introduced by exposing a new function in an interface: https://github.com/arthurfiorette/axios-cache-interceptor/pull/848/files#diff-153c6b38971390ad001e44eef88dd5fd8f62c9b450c64092f280c4b5d2bd1935R74 It should be included in a major release.

axios-cache-interceptor version

v1.6.0

Node / Browser Version

node 20

Axios Version

1.7.7

What storage is being used

Web Storage

Relevant debugging log output

error TS2345: Argument of type '{ find(key: string): any; set(key: string, value: NotEmptyStorageValue): void; remove(key: string): void; }' is not assignable to parameter of type 'BuildStorage'.
  Property 'clear' is missing in type '{ find(key: string): any; set(key: string, value: NotEmptyStorageValue): void; remove(key: string): void; }' but required in type 'BuildStorage'.

 92   const genericCacheStorage = buildStorage({
                                               ~
 93     find(key) {
    ~~~~~~~~~~~~~~~
... 
103     },
    ~~~~~~
104   });
    ~~~

  ../node_modules/axios-cache-interceptor/dist/storage/build.d.ts:35:5
    35     clear: () => MaybePromise<void>;
           ~~~~~
    'clear' is declared here.
arthurfiorette commented 1 month ago

Oh shoot! Nice catch.

Although axios-cache-interceptor major releases for now just follows the axios major version, so i guess there's nothing i could do for now, just create guides on how to fix this issue :/

Are you up to a PR?

JonathanViau commented 1 month ago

Well, there are few options I suppose:

  1. Make this new property as optional and adjust the code in case it is not provided
  2. Revert the change in a 1.6.1 version (but will break anybody who already adjust their code like us)
  3. Revert the change and find a new way to achieve the same expected behavior?

Option 1 is probably the one you want to keep the clear functionality.

arthurfiorette commented 1 month ago

Making the property optional and provide a default implementation that throws not implemented seems a good alternative

JonathanViau commented 1 month ago

throwing "not implemented" means additional error handling on the consuming applications, which still means breaking change.

arthurfiorette commented 1 month ago

No, it does not. clear() is not used by axios-cache-interceptor and was merged to allow 3rd party code to use it.

arthurfiorette commented 1 month ago

Fixed in v1.6.1