rapidjs / rapid.js

An ORM-like Interface and a Router For Your API Requests
https://rapidjs.drewjbartlett.com
710 stars 45 forks source link

Expose axios interceptor api #43

Open mgred opened 6 years ago

mgred commented 6 years ago

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 commented 6 years ago

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

M3psipax commented 6 years ago

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 commented 6 years ago

@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?