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

Expose axios interceptor api #43

Open
mgred opened this issue Apr 13, 2018 · 3 comments
Open

Expose axios interceptor api #43

mgred opened this issue Apr 13, 2018 · 3 comments
Assignees

Comments

@mgred
Copy link
Collaborator

mgred commented Apr 13, 2018

There is a problem with the current behaviour of the interceptor as explained by @M3psipax in detail here.
Basically:

a reject handler cannot be added

We should use axios InterceptorManager to fully expose axios interceptor capabilities.

@mgred mgred self-assigned this Apr 13, 2018
@mgred
Copy link
Collaborator Author

mgred commented Apr 19, 2018

axios unfortunately does not export InterceptorManager, so we cannot instantiate one for every type in the defaults.js

@M3psipax
Copy link

In that case, I would propose one of the following things:

a) change rapidjs documentation so that it says in order to use interceptors, you need to override the initializeAPI() method and add interceptors to this.api after the super call. Just like I do in my current workaround.

b) provide a a function on the Rapid-Object with the same signature as axios that does a) for you. So that you can call inside the constructor this.useRequestInterceptors(fulfilled, rejected). The functions should then store and forward these arguments to the axios.interceptors.use() method inside the initializeAPI() call. change documentation accordingly.

c) ask axios maintainers to expose InterceptorManager for this. Should be quick work for them?

What do you think?

@mgred
Copy link
Collaborator Author

mgred commented Apr 20, 2018

@M3psipax sorry for my late answer

c) ask axios maintainers to expose InterceptorManager for this. Should be quick work for them?

That was my first thought, too :)

b) provide a a function on the Rapid-Object with the same signature as axios that does a) for you. So that you can call inside the constructor this.useRequestInterceptors(fulfilled, rejected). The functions should then store and forward these arguments to the axios.interceptors.use() method inside the initializeAPI() call. change documentation accordingly.

That's a nice solution. But axios also gives the possibility to eject interceptors which we should also respect, so we would need another function, say this.removeInterceptor.

a) change rapidjs documentation so that it says in order to use interceptors, you need to override the initializeAPI() method and add interceptors to this.api after the super call. Just like I do in my current workaround.

I kind of fell in love with your workaround honestly. This makes the interceptors property in the defaults.js and the handling in writeInterceptorsToApi() unnecessary and gives full control to the user. Since one then also has the possibility to eject interceptors as well.

class Base extends Rapid{
.....
  initializeAPI() {
    super.initializeAPI();
    this.interceptor = this.api.interceptors.response.use(response => {
      //response interception
     return response;
    }, error => {
      // error interception
      return Promise.reject(error.response);
    });
  }

  ejectInterceptor() {
    this.api.interceptors.eject(this.interceptor);
  }
}

Thanks again for your feedback and thought, that helps a lot!

@drewjbartlett what do you think, any thoughts on this?

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

No branches or pull requests

2 participants