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

Bug: Breaking change in 1.6.0 #906

Closed
JonathanViau opened this issue Sep 27, 2024 · 6 comments · Fixed by #921
Closed

Bug: Breaking change in 1.6.0 #906

JonathanViau opened this issue Sep 27, 2024 · 6 comments · Fixed by #921
Assignees
Labels
bug Something isn't working triage Needs triage

Comments

@JonathanViau
Copy link

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.
@JonathanViau JonathanViau added bug Something isn't working triage Needs triage labels Sep 27, 2024
@arthurfiorette
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

arthurfiorette commented Sep 28, 2024

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

@JonathanViau
Copy link
Author

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

@arthurfiorette
Copy link
Owner

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

@arthurfiorette arthurfiorette linked a pull request Oct 18, 2024 that will close this issue
@arthurfiorette
Copy link
Owner

Fixed in v1.6.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage Needs triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants