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

Update documentation to clarify side effects of setupCache #732

Merged
merged 2 commits into from
Dec 12, 2023

Conversation

Rototu
Copy link
Contributor

@Rototu Rototu commented Nov 23, 2023

Key Changes:

Update documentation to clarify that using setupCache on the default axios import or on window.axios will apply the cache interceptor everywhere that default export instance is used.

Reason for change:

Adds necessary clarification so that the end user has full knowledge of the consequences of invoking setupCache on the default export. For those who are not very familiar with JS or axios, such a clarification could help them avoid unwanted caching of data.

Such unwanted caching can be even more problematic when resources are not uniquely identified by their URL and instead distinguished through headers (it's a bad practice but it is a practice nonetheless), which can lead even to sharing of data between users for instance.

This is by no means a fault of this library but one of the end user, but still, if we can help the user avoid such a situation, why not? 😃

Copy link

codecov bot commented Nov 23, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (621a725) 99.53% compared to head (8a531b5) 99.53%.
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #732   +/-   ##
=======================================
  Coverage   99.53%   99.53%           
=======================================
  Files          19       19           
  Lines        2366     2366           
  Branches      208      208           
=======================================
  Hits         2355     2355           
  Misses         10       10           
  Partials        1        1           

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

@arthurfiorette
Copy link
Owner

Rather than adding comments to all examples, It's better to update them into using a custom axios instance.

@Rototu
Copy link
Contributor Author

Rototu commented Nov 30, 2023

I agree :) Just was trying to be respectful of the project since I assumed you had a reason for telling users to call setupCache with the default axios instance

I have updated the docs now as you've requested.

@arthurfiorette arthurfiorette merged commit bbb00df into arthurfiorette:main Dec 12, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants