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

fix: only load Promise polyfill if window.Promise is missing #1470

Merged
merged 1 commit into from
Apr 4, 2019

Conversation

straker
Copy link
Contributor

@straker straker commented Apr 2, 2019

es6-promises doesn't check if window.Promise exists before overriding it, but instead checks if the browser natively supports Promises by seeing if Promise.resolve() returns '[object Promise]' when passed to toString:

let P = local.Promise;

if (P) {
  var promiseToString = null;
  try {
    promiseToString = Object.prototype.toString.call(P.resolve());
  } catch(e) {
    // silently ignored
  }

  if (promiseToString === '[object Promise]' && !P.cast){
    return;
  }
}

local.Promise = Promise;

Adding a check before we run the polyfill to see if window.Promise exists.

I looked at changing to anther promise library that would do the check better, but I'm not sure if @JKODU chose es6-promsie for a specific reason or not. promise-polyfill claims it's <1 kb when gzipped, which appears to be much smaller than es6-promise (2.4 KB gzipped), and it looks to see if window.Promise exists before polyfilling.

Closes: #1468

Reviewer checks

Required fields, to be filled out by PR reviewer(s)

  • Follows the commit message policy, appropriate for next version
  • Has documentation updated, a DU ticket, or requires no documentation change
  • Includes new tests, or was unnecessary
  • Code is reviewed for security by: @WilcoFiers

@straker straker requested a review from a team as a code owner April 2, 2019 16:02
@WilcoFiers WilcoFiers merged commit 1d70306 into develop Apr 4, 2019
@WilcoFiers WilcoFiers deleted the fixPromisePolyfill branch April 4, 2019 15:09
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.

Axe-Core 3.2.0 added an ES6 Promise polyfill that overwrites native promise behavior
2 participants