Subclassing ES6 objects with ES5 syntax.

[Allen Wirfs-Brock]

On Apr 29, 2015, at 2:36 PM, C. Scott Ananian wrote:

> 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.

you're correct.  Let's go with:

class DefensivePromise extends Promise {
  constructor(x) {
    super(x);
    if (new.target === DefensivePromise) {
      Object.freeze(this);
    }
  }
  static resolve(x) {
    if (this===DefensivePromise) {
       switch (true) {
          default:
             // 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;
        };
       return new this(r => {r(x)});
    }
    // must be called on a subclass of DefensivePromise, so we don't need to enforce the DefensivePromise 'then' invariant 
    return super.resolve(x);
    }
 }
Object.freeze(DefensivePromise);
Object.freeze(DefensivePromise.prototype);

> 
> 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.
> 

That can't right.  x could be an object that is not a C but whose SpeciesConstructor is C.  In that case, a new C promise on x still has to be created.  But your code would just return x.  I would use a 'construtor' test in step 2. But if 'constructor' is not C and SpeciesConstructor is also not C then an extra level of C wrapping is needed.



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/es-discuss/attachments/20150429/e31c6137/attachment-0001.html>