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

Improve interceptor registration order by letting users decide it. #613

Open
arthurfiorette opened this issue Jul 19, 2023 · 0 comments
Open
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@arthurfiorette
Copy link
Owner

First of all these are in the reverse order (source).

The request cache interceptor should be the first one to execute and the response interceptor should also be the first one to resolve by default. This way, retry/rate limiter interceptors will only handle requests that would actually go through the network.

Ideal registration order would be like this:

1. any request interceptor
2. axios request interceptor as last
---
1. axios response interceptor as first
2. any response interceptor

Our current api suggests the use of setupCache(Axios.create()), which mostly forces the usage of the following order:

1. axios request interceptor as fist   <-
2. any request interceptor             <-
---
1. axios response interceptor as first
2. any response interceptor

which are caused by these lines:

axiosCache.requestInterceptor.apply();
axiosCache.responseInterceptor.apply();

My current idea is to do these 3 following ways:

// This calls to register the request and response interceptor. no need to setupInterceptor();
// Desired default to avoid breaking changes.
const axios = setupCache(Axios.create(), { attach: true } )//
// only registers the response interceptor.
// Should only be default in the next major version.
// (Or maybe we push a new version now alongside with other PR changes that are open)
// because this is a super easy and fixable breaking change
const axios = setupCache(Axios.create(), { attach: 'headless' })

// this calls to register the request interceptor.
axios.setupInterceptor();
// In case you want full modularity
const axios = setupCache(Axios.create(), { attach: false })

// this calls to register the request and response interceptor.
// maybe a different method/parameter to be possible to register each separatelly?
axios.setupInterceptor();
@arthurfiorette arthurfiorette added documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers labels Jul 19, 2023
@arthurfiorette arthurfiorette self-assigned this Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant