Subclassing ES6 objects with ES5 syntax.

[C. Scott Ananian]

On Wed, Apr 29, 2015 at 4:16 PM, Allen Wirfs-Brock <allen at wirfs-brock.com>
wrote:

> On Apr 29, 2015, at 12:40 PM, C. Scott Ananian wrote:
>
> On Wed, Apr 29, 2015 at 3:32 PM, Allen Wirfs-Brock <allen at wirfs-brock.com>
> wrote:
>
>> On Apr 29, 2015, at 12:24 PM, C. Scott Ananian wrote:
>>
>> On Wed, Apr 29, 2015 at 3:09 PM, Allen Wirfs-Brock <allen at wirfs-brock.com
>> > wrote:
>>
>>>  class DefensivePromise extends Promise {
>>>   constructor(x) {
>>>     super(x);
>>>     if (new.target === DefensivePromise) {
>>>
>>            // I'm assuming this test is just to be subclass friendly and
>> allow subclasses to freeze later?
>>
>>>       Object.freeze(this);
>>>     }
>>>   }
>>>   static resolve(x) {
>>>     switch (true) {
>>>        default:
>>>
>>          // I guess a do { } while(false); would work as well?
>>
>>>           // assuming frozen primordial
>>>           if (x.constructor !== DefensivePromise) break;  //just a quick
>>> exit, doesn't guarantee much
>>>           if (!Object.isFrozen(x)) break;
>>>           If (Object.getOwnPropertyDescriptor(x, 'then')) break;
>>>           //a more subclass friendly approach would walk x's
>>> [[Prototype]] chain to ensure that the correct 'then' is inherited from
>>> frozen prototypes
>>>           if (Object.getPrototypeOf(x) !== DefensivePromise.prototype)
>>> break;
>>>           //Assert: x.then === Promise.prototype.then, now and forever
>>> after
>>>           return x;
>>>      }
>>>      // must be called on a subclass of DefensivePromise, so we don't
>>> need to enforce the 'then' invariant
>>>      If  (x.constructor ===  this) return x;   //in which case a
>>> constructor check is good enough
>>>
>>           // ^^ this is a mistake right?  I think this line doesn't
>> belong.
>>
>>>      return new this(r => {r(x)});
>>>    }
>>>  }
>>> Object.freeze(DefensivePromise);
>>> Object.freeze(DefensivePromise.prototype);
>>>
>>
>> It's not clear what the `x.constructor` test is still doing in your
>> implementation.
>>
>>
>> Do you mean the first or second occurrence?
>>
>
> The second occurrence.  At that point all we know about x is that it's
> *not* something safe.  We shouldn't be looking at `constructor` in that
> case.
>
>
> the last two lines of the above `resolve` method should be replaced with:
>      return super.resolve(x)
>
> x.construct === this is the test that I propose Promise.resolve should
> make instead of  testing [[PromiseConstructor]]
> There should probably also be another test that
> SpeciesConstrutor(x)===this
>

This is still not correct.

You can't call `super.resolve(x)` safely unless you know that
this.constructor is trustworthy.  If you've bailed out of the `switch`, you
don't know that.

A correct implementation just does `return new this(r => { r(x); })` after
the switch.

The "another test" that you propose seems to be exactly the fix which
Brandon proposed and I codified, to wit:

2. If IsPromise(x) is true,
>     a. Let constructor be the value of SpeciesConstructor(x, %Promise%)
>     b. If SameValue(constructor, C) is true, return x.


I wouldn't jump the gun until we actually have TC39 consensus on a proposal.
>

Mark, after you've slept on this and assuming you haven't changed your
mind, would you be willing to be the TC39 champion?  I'm not a participant
in that process.


> What we've shown is that the [[PromiseConstructor]] test was never
> sufficient to guarantee the invariant that Mark is concerned about and also
> that there probably isn't a generic check we can do that would satisfy his
> requirements.  He has do it himself like shown above.  But the specified
> [[PromiseConstructor]] test doesn't  really hurt anything.  It places some
> slight limitations on what subclasses can do, but probably doesn't
> interfere with anything an actual subclass is likely to do.  It seems like
> a restriction than can be relaxed in the future without anybody (or than
> conformance tests) noticing.
>

Well, the [[PromiseConstructor]] test is breaking `prfun`, `es6-shim` and
`core-js` right now, which is why I noticed it in the first place.  (v8
seems to have implemented the `[[PromiseConstructor]]` internal property
even though they don't have `new.target` implemented, so it's always
hard-wired to `Promise`.)  And `babel` and `core-js` have implemented
non-standard semantics for `Promise.resolve` as a result.

In particular, with the current state of `Promise.resolve`, I can't
subclass `Promise` in a meaningful way using ES5 syntax (see, the thread
title turns relevant again!).  So I have a vested interest in getting the
semantics fixed so that people can start using `Promise` subclasses (like
`prfun` does) instead of writing their "extra" Promise helpers directly on
the `Promise` global object, while we're all waiting for ES6 syntax to make
it to the mainstream.

(And when I say "getting the semantics fixed" what I really mean is
"consensus among TC39" so that shim libraries can implement the correct
semantics as early adopters, with assurance the browser engines will
follow.)

Since I believe the goal is to eventually introduce some new methods on the
global `Promise` in ES7, I think this list would have a keen interest in
ensuring that libraries which stomp on the global Promise (due to lack of
an alternative) don't take root.
  --scott
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/es-discuss/attachments/20150429/38306d40/attachment-0001.html>