Add withCredentials option. Fix #29 #33
Conversation
| if (options.cors) { | ||
| xhr.withCredentials = true | ||
|
|
||
| if ("withCredentials" in options) { |
There was a problem hiding this comment.
can we default it to true.
We can use if (options.withCredentials !== false) { ?
There was a problem hiding this comment.
Oh that would be bad actually. What we want is
if (options.cors && options.withCredentials !== false) {
Meaning opt into cors & opt out of withCredentials
There was a problem hiding this comment.
See browserify/http-browserify#47, which I mentioned in #29. The spec says withCredentials should default to false, so that's what people expect. Also, the wildcard is not allowed in Access-Control-Allow-Origin when it's set to true.
There was a problem hiding this comment.
The spec says withCredentials should default to false, so that's what people expect
The spec has an annoying default, xhr defaults to true because that's what you want. Also I don't think breaking back compat is a good idea. Note that it should still be optin by setting cors to true, so it DOES default to false
Also, the wildcard is not allowed in Access-Control-Allow-Origin when it's set to true.
The wildcard was always a bad idea, no production service should be using it.
There was a problem hiding this comment.
Thanks, I wasn't aware of the best practice there, and I imagine I'm not the only one. Maybe worth a word in the README? Do you have any good resources around this issue to point at?
In the meantime I'll change it to default to true.
Add withCredentials option. Fix #29
|
Published v1.12.0 |
No description provided.