-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
iOS Safari 10 bug where SockJS couldn't be found #1238
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
Conversation
Fixes iOS Safari 10 bug. At the root, this works around a bug where Safari's eval's scope was getting confused. Something to do with [this issue](https://stackoverflow.com/questions/46036960/evaluated-expression-const-variable-scope-in-safari) [bug reference](webpack#1090 (comment))
@abcd-ca please fill out the pull request template, those aren't optional. we ask that of everyone. |
@shellscape how's that? |
not clear why Travis CI is failing |
Better, but as a practice you shouldn't remove portions of a template. Looks like you clipped
Templates (issues and pull requests) are usually there to help everyone interacting with you out :) Looks like there are some failures in CI that need to be addressed. |
Clicking on the Details link for Travis will show you the builds that failed. Looks like you didn't lint before pushing :) |
@shellscape I added that missing part of the template. Next time I'll leave the template there and modify |
@shellscape I can't see what it is that needs fixing exactly. I don't see anything I can click on to give me an explanation in the Travis CI error view |
will re-issue pr to fix travis issue |
@abcd-ca you don't have to reissue anything (please don't do that). you just need to run Here's the sequence in which you should click on Travis links And here's the log in Travis that shows the error Always run |
@shellscape thanks for reminding me to pull the repo and run tests before issuing PR. Pushed a fix. Once PR is merged, do you know how I update my project to get the fix? Never been clear on that as it seems you would maybe have to issue a new release and somehow master may not be current with npm but I'm not too sure how that all works. |
Codecov Report
@@ Coverage Diff @@
## master #1238 +/- ##
=======================================
Coverage 76.31% 76.31%
=======================================
Files 5 5
Lines 477 477
Branches 154 154
=======================================
Hits 364 364
Misses 113 113 Continue to review full report at Codecov.
|
@abcd-ca thanks for your patience on this one. we've merged it. |
What kind of change does this PR introduce?
bug fix on iOS Safari 10 (iOS 10)
Did you add or update the
examples/
?No, not applicable, no API changes
Summary
Fixes iOS Safari 10 bug. At the root, this works around a bug where Safari's eval's scope was getting confused. Something to do with this issue
bug reference
Does this PR introduce a breaking change?
No
Other information
No