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: implement Symbol.species of Promise #34162

Closed

Conversation

@JiaLiPassion
Copy link
Contributor

JiaLiPassion commented Nov 30, 2019

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: #34105, #33989

ZoneAwarePromise should pass the following test

var USE_NATIVE = !!function () {
  try {
    // correct subclassing with @@species support
    var promise = $Promise.resolve(1);
    var FakePromise = (promise.constructor = {})[Symbol('species')] = function (exec) {
      exec(empty, empty);
    };
    // unhandled rejections tracking support, NodeJS Promise without it fails @@species test
    return (isNode || typeof PromiseRejectionEvent == 'function') && promise.then(empty) instanceof FakePromise;
  } catch (e) { /* empty */ }
}();

Does this PR introduce a breaking change?

  • Yes
  • No
@JiaLiPassion JiaLiPassion requested a review from angular/fw-zones as a code owner Nov 30, 2019
@googlebot googlebot added the cla: yes label Nov 30, 2019
@JiaLiPassion JiaLiPassion changed the title fix: make sure ZoneAwarePromise not be overwrite by Object.defineProperty WIP: make sure ZoneAwarePromise not be overwrite by Object.defineProperty Nov 30, 2019
Close #34105, #33989
@JiaLiPassion JiaLiPassion force-pushed the JiaLiPassion:zone-defineproperty-promise branch from b7bad35 to 91accd3 Dec 1, 2019
@JiaLiPassion JiaLiPassion changed the title WIP: make sure ZoneAwarePromise not be overwrite by Object.defineProperty fix: implement Symbol.species of Promise Dec 1, 2019
@ngbot ngbot bot added this to the needsTriage milestone Dec 1, 2019
@JiaLiPassion JiaLiPassion requested a review from mhevery Dec 1, 2019
@mhevery
mhevery approved these changes Dec 2, 2019
@mhevery

This comment has been minimized.

Copy link
Member

mhevery commented Dec 2, 2019

@nnit-TroelsJensen

This comment has been minimized.

Copy link

nnit-TroelsJensen commented Dec 18, 2019

@mhevery @JiaLiPassion
Has this been cancelled? It seems that the PR is closed but not merged?
This fix was supposed to handle a general issue on older safari. Is it fixed elsewhere?

@angular-automatic-lock-bot

This comment has been minimized.

Copy link

angular-automatic-lock-bot bot commented Jan 18, 2020

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants
You can’t perform that action at this time.