Interceptors array potentially growing indefinitely

[pendenaor]

Hi,

While testing my application, i notice that the handlers array of axios interceptors only increase, even if i use the eject method.

In fact, eject only nullify interceptor entry

axios/lib/core/InterceptorManager.js

Line 32 in a105872

this.handlers[id] = null;

and use always push new handler and return length - 1

axios/lib/core/InterceptorManager.js

Line 22 in a105872

return this.handlers.length - 1;

Is it not dangerous (or expensive) to have this type of behaviors?

Regards

[Axnyff]

Hello,
I had a quick look and I don't really know what could be a good alternative (without breaking changes) because the ids are necessary to eject so new interceptors always need to be a new array key, we cannot just filter the array as this will change the ids.

Deleting keys instead of just putting null could help a little but it would just create sparse arrays, I'm not sure this is much better.

Maybe using a plain object to store the interceptors and another array to know in which order call the interceptor could work ( I don't think relying on keys order in object is a good idea, it's only sure to work in ES6 compliant implementation)

[pendenaor]

And reusing nulled index instead of returning directly the last one in use?

[Axnyff]

Wouldn't that change the order of execution of interceptors?
I also think that there's a chance that somebody depends on the fact the the indexes always grow to keep track of the last added interceptor so I don't know if that would be ok

The more I think about it, the more I'd say that using just an object to store the interceptors could actually be a good idea.

[pendenaor]

Ok, order is important... maybe, a Map with an auto-incremented numeric key will be sufficient?

[Axnyff]

Yes, that seems like the best idea. That's kind of what I was suggesting but with a plain JS object to make sure it will be ES5 compatible

[viktoriiakrause]

Hi!
Are there any updates on this issue?

[rohitkhatana]

@Axnyff Using plain JS object(Map implementation) looks good approach. is there any update/plan to implement it ?

[pendenaor]

You have no guarantee that order of insertion is respected with plain js objet.

[rohitkhatana]

We could have used Map :)
is there any other alternative/solution to the above problem @pendenaor

[pendenaor]

ES5 compatible? See #1380 (comment) where @Axnyff suggests object + array

[Axnyff]

My PR should work with only an id and a count: #1410
I had some requested changes but I don't know how to profile for memory though

[jasonsaayman]

Hi,

Please read more on this issue over at #1914

Thanks