-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(web): Gracefully degrade Proxy usage to fix IE11 #2759
fix(web): Gracefully degrade Proxy usage to fix IE11 #2759
Conversation
core/package.json
Outdated
@@ -59,7 +59,8 @@ | |||
}, | |||
"testMatch": [ | |||
"**/__test__/*.(ts|tsx|js)" | |||
] | |||
], | |||
"testURL": "http://localhost/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI I had to add that to make the tests work as jest
throws SecurityError: localStorage is not available for opaque origins
otherwise, the reason being that testURL
defaults to about:blank
in v0.22 whereas it has been fixed to http://localhost/
in the latest version.
This can be removed once jest
has been upgraded, but this was beyond the scope of the PR.
Thanks @laurentgoudet, looks good to me. Curious what @jcesarmobile thinks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that fixing the tests is out of the scope of this PR, so can you remove the testURL
?
I've sent a separate PR updating jest.
Other than that, looks good to me, plugins seem to work on IE11 and doesn't seem to break on newer browsers.
3c13b58
to
6b58239
Compare
Done |
Fixes #456. This guards the
Proxy
usage is order to ensures that loading Capacitor does not throw a Syntax Error in IE11 and other older browser engines.