Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Make the check for ZoneAwarePromise more stringent #495

Merged
merged 1 commit into from
Nov 16, 2016

Conversation

markandrus
Copy link
Contributor

Hi,

Here is my proposed fix for #494. I've opted to check for the properties "__zone_symbol__state" and "__zone_symbol__value". It just occurred to me that checking the constructor might also work, but I am not sure. Commit message below.

Thanks,
Mark


resolvePromise assumes that if a value is an instanceof ZoneAwarePromise then it has the properties "__zone_symbol__state" and "__zone_symbol__value" and it is a true ZoneAwarePromise; however, a user can construct a value that breaks this assumption by inheriting from ZoneAwarePromise without actually having those properties or being a true ZoneAwarePromise (for example, by attempting to subclass Promise).

We can fix this by adding checks for "__zone_symbol__state" and "__zone_symbol__value" to resolvePromise.

`resolvePromise` assumes that if a value is an `instanceof` ZoneAwarePromise
then it has the properties "__zone_symbol__state" and "__zone_symbol__value"
and it _is_ a true ZoneAwarePromise; however, a user can construct a value that
breaks this assumption by inheriting from ZoneAwarePromise without actually
having those properties or being a true ZoneAwarePromise (for example, by
attempting to subclass Promise).

We can fix this by adding checks for "__zone_symbol__state" and
"__zone_symbol__value" to `resolvePromise`.
markandrus added a commit to twilio/twilio-video.js that referenced this pull request Oct 25, 2016
For context, refer to the following GitHub issues and pull request:

* #24
* angular/zone.js#494
* angular/zone.js#495

Ideally, we can add back the `inherits` call (if indeed this method of
subclassing is correct) after Zone.js responds.
@mhevery mhevery merged commit c69df25 into angular:master Nov 16, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants