Skip to content
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

Add conditional polyfill for promise, fetch #2041

Merged
merged 1 commit into from
Jul 28, 2017
Merged

Add conditional polyfill for promise, fetch #2041

merged 1 commit into from
Jul 28, 2017

Conversation

aduth
Copy link
Member

@aduth aduth commented Jul 26, 2017

Fixes #1397
Fixes #1863

This pull request seeks to polyfill Promise and Fetch for environments where they are unavailable (IE11). It uses document.write ( 😱 ) to inline script dependencies for browsers which do not meet the necessary conditions, in a yepnope fashion. I sourced the technique from another project I maintain (source).

Testing instructions:

Verify that there are no network requests (in browser developer tools Network tab) for fetch or promise polyfills in capable browsers.

Verify that the Gutenberg demo content loads in IE11 without errors.

@aduth aduth requested a review from afercia July 27, 2017 18:35
@afercia
Copy link
Contributor

afercia commented Jul 28, 2017

Good news: I can access the Demo page with IE 11
Not so good news: I get random (and obscure) errors and also managed to crash IE 11. Refreshing the page multiple times, errors happen inconsistently which makes me think at loading order of the scripts or something like that.

screenshot 9

screenshot 10

Copy link
Contributor

@afercia afercia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See IE 11 errors in the comments.

@aduth
Copy link
Member Author

aduth commented Jul 28, 2017

I'm unable to encounter any such errors.

No errors

Wonder if this could be related to #2066.

In any case, I don't think they'd be related to the polyfills (if polyfills failed to load for any reason, I expect the editor wouldn't show at all). Let's merge this and try to narrow down root cause of other errors in a separate issue.

@codecov
Copy link

codecov bot commented Jul 28, 2017

Codecov Report

Merging #2041 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2041   +/-   ##
=======================================
  Coverage   20.24%   20.24%           
=======================================
  Files         130      130           
  Lines        4184     4184           
  Branches      712      712           
=======================================
  Hits          847      847           
  Misses       2811     2811           
  Partials      526      526

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c22c6c...004ced4. Read the comment docs.

@aduth aduth merged commit fb59a8b into master Jul 28, 2017
@aduth aduth deleted the fix/ie11 branch July 28, 2017 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants