Expand Headers types#4144
Conversation
|
Afaik HTTP headers are always transferred as strings. Could you elaborate on which headers accept |
|
This update to require i.e. const headerA = 5; // simplified for example, imagine all of these headers came from something
const headerB = false;
const headerC = 'someStr';
await axios.get(someUrl, { headers: { headerA, headerB, headerC } });is a lot simpler from a user's perspective than const headerA = 5; // simplified for example, imagine all of these headers came from something
const headerB = false;
const headerC = 'someStr';
await axios.get(someUrl, {
headers: {
headerA: headerA.toString(),
headerB: headerB.toString(),
headerC
}
}); |
|
I thought about this as well, but I concluded that since HTTP headers are strings by definition and the types should reflect this. Also none of the official HTTP headers support the values There are some headers that support numeric strings, for example Access-Control-Max-Age. Although I don’t think axios should support numbers, I can see why this is convenient. If this is supported, I think it should be explicit (documented and tested). |
|
@remcohaszing I can agree with that overall, makes sense. I guess my pain point is that this can be seen as a regression for any applications that were already using it in that way – If an app previously passed |
|
I thought about this and think it would make sense to also allow @bfaulk96 Could you verify this using the network tab of your favorite browser's Dev Tools? PS: You might want to rebase your branch on latest master, or merge latest master, as #4136 introduced two interface |
Values are simply cast to string. I think the axios maintainers should determine what values they want to support and the types should reflect this. |
Agreed. I got a little carried away with To answer the other question, yes: Those seem fairly harmless to allow – but I surely understand if not. On the note of perhaps a a feature request – would be cool to allow |
|
Also, updated this branch to reflect our conversation – used the latest types already existing in master, and removed |
|
Merged in latest changes 👍 |
|
Any update on this one? Do I need to do something/reach out to somebody? Or just be patient? 😄 |
|
@jasonsaayman what do you think? Currently users have to explicitly pass strings as headers: const maxAge = 1337;
const customHeader = true;
axios.get('/api', {
headers: {
'Access-Control-Max-Age': String(maxAge),
'X-Custom-Header': String(customHeader)
}
})Basically these types say the following is also acceptable: axios.get('/api', {
headers: {
'Access-Control-Max-Age': maxAge,
'X-Custom-Header': customHeader
}
}) |
|
@jasonsaayman any thoughts on this? |
|
@bfaulk96 can you please have a look at the failing tests? |
|
Done – sorry about that |
|
@jasonsaayman Any chance of getting this merged? |
|
bump |
|
Yip will merge today |
|
shouldn't this changes be added to response headers as well ? |
No, since HTTP Headers are always sent as strings, Response headers will be strings that you will need to parse if you want different types from them |
|
@jasonsaayman Any estimate on when the next Axios release might be? |
|
* Expand Headers types * Add undefined to allowed types * Add semicolon
Update headers type for a request to allow
booleanandnumbertypes in addition to strings.Export
RequestHeadersandResponseHeaderstype aliases