-
Notifications
You must be signed in to change notification settings - Fork 832
Use fetch instead of XMLHttpRequest #1335
base: dev
Are you sure you want to change the base?
Conversation
|
+1 since it would help this library to run on the server as well. I think it should continue to read from |
Thanks for the interest, but this library was never intended to run on the server. There are already others that fill this gap. |
Actually |
|
Sure, but what's the benefit of changing if what's used is working (in the browser context since that's the only requirement)? |
|
As I stated above, XMLHttpRequest is not available inside service workers. Service workers ARE in the browser context. |
|
And IE11 is deprecated. |
Ok, fair point. I don't know how the rest of the library will work in a service worker, tho. |
It works great with this PR. :-) We are using this fork ourselves in our Chrome browser extension using manifest 3 which has logic in service workers. |
|
So, whats missing to merge it? |
Me having time to review. |
|
There is one TODO, maybe @brockallen can provide input on that before the review. |
XMLHttpRequest is not available inside service workers. One can always polyfill fetch onto XMLHttpRequest (there are many available) but I could not find any working polyfill for the opposite (polyfill XMLHttpRequest using fetch).
Also, I tried to use
Oidc.Global.setXMLHttpRequestbut I didn't really work,windowis not defined so it is not returning setrequest, even when set. I think that check should be removed, or at least wrapped around|| XMLHttpRequestcheck only.TODO:
Oidc.Global.setXMLHttpRequestandget XMLHttpRequest? Should I change them to be aboutfetch?See also: #1336