schibsted / account-sdk-browser

Schibsted Account SDK for browsers
https://schibsted.github.io/account-sdk-browser/
MIT License
16 stars 11 forks source link

Internet Explorer 11 support #31

Closed schibsted-martin closed 6 years ago

schibsted-martin commented 6 years ago

I think this issue affects both the pre-built version (/es5) and the self-compiled one. I am myself using the self-compiled version and when I do so I get a validation error when instantiating the Identity class. The reason why it claims my redirectUrl is incorrect is in fact that IE 11 does not support the new URL call and this bubbles up and gets caught in the "redirect url assertion" code.

The reason why I only think it also affects the /es5 version is because I have done so many changes to the code base as of late that it is a little hard to assure if it is because of this error I still can't use the library. I apologise for not digging deeper into this but I felt at least reporting the issue as a first step would be a good starting point.

No issue without a solution, right? I ended up using https://www.npmjs.com/package/url-polyfill adding it before the main entry of the file. I did try to get both the universal url parser and whatwg url to work but they are not really polyfills for specific browser, more a way to add support for URL in general inside a node environment. There were some threads describing on how to use shims in order for it to work inside browsers, but none of the methods worked for me. Since I have a working solution now I will proceed with that and hope this clears up by itself when we 1) dump IE 11 support 2) this makes it into babel/core-js 3) the world ends.

As always, thank you for the support in Slack guys and keep up the good work!

torarvid commented 6 years ago

For anyone else who might be interested: Martin discovered that the url-polyfill will not throw if called as new URL('foo') even though the WHATWG spec implementation does throw a TypeError (callers are supposed to either pass a valid URL as the first param, or pass a base url as the second param).

To work around this issue, I reversed some logic in #32 so that we won't hit the code that called... new URL('PRE'), for instance.

Also, the documentation is updated to be clearer that this code needs certain polyfills to work in older browsers (specifically for IE11, Promise, fetch, Object.entries, URL and Number.isInteger was needed)

schibsted-martin commented 6 years ago

To simplify. If you are running Internet Exploader 11 you need to polyfill the URL class in the browser. We ended up using https://www.npmjs.com/package/url-polyfill.

The second part of this problem was however that this polyfill does not meet the exact specification of WHATWG-URL; falsely accepting i.e. new URL('not a valid url') as a valid URL with the base information (hostname, port etc) taken from whatever the current host is.

This particular "error" was avoided by not using the URL class to do initial setup (setting base host for the login/logout calls etc) of the library (account-sdk-browser).