#1395 breaks custom config

[edgesite]

Summary

#1395 launched a fix that instance config may override global config, but it ignore custom configs.
So it will break something we'd like to attach into the config and access in interceptor (e.g. CookieJar).

For example (https://codepen.io/anon/pen/gjZRdR?editors=1111):

const inst = axios.create();
inst.interceptors.request.use(config => {
  console.log('should be "test", actual', config.testAttr);
})
inst.get('/', {
  testAttr: 'test',
});

Context

• axios version: v0.19.0-beta.1
• Environment: node & chrome

[zcei]

To be honest, I'm not convinced this is a good idea. Arbitrary user data shouldn't be root level keys to a library from my design perspective.

Even though TypeScript might not be a "blocker" in a traditional sense to a JS project: e.g. metod: 'get' wouldn't error, as metod might be a custom configuration.

Usually I'd recommend abstracting user data into your own class that extends Axios - though I can see how people currently using Axios might rely on this, so my recommendation for an easy upgrade path would be exactly one root key whose value is an object of shape { [key: string]: any }. This way it can contain all the user data you might want to store, but encapsulated (also preventing undesired shadowing of Axios configuration).

Implementation in current code base would be trivia - just copy over that key as well.
Access in interceptors just have one more indirection: config.<key>.testAttr

WDYT @rikuayanokozy?

[edgesite]

@zcei
Great point. Can we just make config.userData: any to store user data.
But since it's a regression, we still need to fix it or put it into breaking changes since it will break some old codes.

[zcei]

I agree we should list it as a breaking change. I'm not sure whether it makes sense to first deprecate it, but still support it.

Would be quite easy though, I guess. We can assign the lists with known keys to variables and then get all the keys from the two configs. difference(actualKeys, expectedKeys) would then yield the keys we need to warn for. (This might increase bundle size slightly for the time until removed, as I don't think we are using a difference implementation anywhere currently)

What is the reason to type this any? As everyone was using object properties to store data until now, I'd just provide them a clean slate object to start from.

[emilyemorehouse]

Unless there's opposition, I'm okay with noting this as a breaking change and keeping the current functionality in the 0.19 beta

[Khaledgarbaya]

Although the change makes sense I still agree with @rikuayanokozy here.
This change is breaking the rate limit logic in our SDKs. It makes sense for some custom configs to be carried around that defines how the http client behave for some intercepted responses, in our case 429.

I would like to have a customConfig: any to be able to have some global configs.
This is a known pattern for example with Express.js and Happi there is the .app property where you can set some global configs on it.

[Khaledgarbaya]

I made a PR that allows a customConfig option

[ustbhuangyi]

I am looking forward to this feature😀

[oleduc]

This is still an issue and I've create a PR that fixes it without requiring a new feature #2006

[cristianocca]

Faced this as well, custom config parameters for interceptors is very important. Any work around? Or is downgrade the only option?

[thorn0]

@zcei

Even though TypeScript might not be a "blocker" in a traditional sense...

TypeScript isn't a blocker at all. In order to support custom config properties, Axios doesn't need to change its type definitions anyhow. Modules are easily augmented in TS.

E.g. if you want to use a custom property in your project, just do this:

declare module 'axios' {
    interface AxiosRequestConfig {
        myCustomProperty: MyType;
    }
}

That's it.

[cristianocca]

What if I'm not using typescript?

Looks like there are PRs, but no changes here yet. Any news?

[yasuf]

Closing, #2207 is merged and will be released as part of 0.19.1