One does not simply extend Promise

that's the issue ... nobody is calling the promise constructor or invoking its execution when a .then(calback) is meant ... it's an explicit intent whenever the Promise that generated that .then(callack) is done, the then executes.

But as I've already mentioned: how would anyone create a fetch utility that doesn't curl behind the scene X times when created, yet it preserves it's a fetch call when instanceof Fecth operation happens, even on then(...)?

quite the opposite, I am questioning the correctness of what is happening around .then or .catch ... the current specs re-execute extends for no reason, promises don't re-execute any callback on the super ... there is a huge glitch here to explain and no solutions (so far) to my us ecase ... make a fetch like utility that is a promise and an instance of Fectch and it doesn't re-trigger, behind the scene, any curl request per each then invoked. Alternatively, please tell me a single use case where current behavior around then is desired, with the source callback being re-executed each time, thanks.

Plain and simple: it is not. Your understanding of your own test is wrong.

The test shows 4 invokes, the then calls the constructor and it executes the callback, which part is wrong?

But most importantly: can you please show me a single use case this behavior I don't understand is desired?

This conversation is a bit like "you are wrong, but I don't have examples to prove current state is right" which is odd.

I've already explained 3 different times what the Promise behavior is, and why it is not what you claim it is. You keep insisting that your examples show something it doesn't show. I honestly don't know what else I can say.

then basically does this:

Promise.prototype.then = function (
  onFulfilled = (x) => x,
  onRejected = (r) => {
    throw r;
  }
) {
  return new this.constructor((resolve, reject) => {
    this.#reactions.push({
      deferred: { resolve, reject },
      onFulfilled,
      onRejected,
    });
  });
};

As you can see, then invokes the this.constructor (or the species on it) with a new executor. It does not use the original executor, which is not saved anywhere.

Your duplicate console.log come from the fact the custom constructor is called once for the original promise, and once for the chained promise returned by then.

I am not claiming anything, I am observing undesired side-effects on multiple invokes which is not desired ... I am not here to state how it works, I am here to question how it works, or why it works like that, and your example is spot-on to underline my questioning:

  // why is this the current thenable state at all?
  // why is the constructor invoke needed when the constructor
  // already executed and this is just a derived result of it?
  // why doesn't this return instead an Object.create(this.constructor.prototype)
  // with all the needed fields to just propagate, as opposite of invoking over and over
  // the constrcutor?
  // why is default Promise not re-invoking the passed callback ever, but an extend
  // results into invoking the super callback each time?
  return new this.constructor((resolve, reject) => {
    this.#reactions.push({
      deferred: { resolve, reject },
      onFulfilled,
      onRejected,
    });
  });

I understand where it comes from and I understand what you are saying, which is: this is how it is.

My question is: can anyone in here provide a single use case where the current behavior is desired, where a Promise extend callback is re'executed every single time a .then(...) happens?

Can anyone explain how to create a fetch like API without using Symbol.species and without having the callback within the Promise extend to re-execute each time a then(...) happens?

There is no difference between a derived promise and a base platform promise, I still don't understand your claim that the executor is called multiple times.

Regarding how the new chained promise is constructed, the constructor is called because that's how you construct objects? Where would you get the resolve and reject from if not from the constructor invocation? That is simply the Promise API contract: it's a revealing constructor pattern, where the constructor reveals an internal capability (the resolver functions) to the creator of the object.

Object.create does not install slots/private fields, that's the job of the constructor. Furthermore, built-ins usually don't reach the slots of objects that aren't this. The constructor not being invoked when constructing an object would be the surprising part.

Propagation of promise species exists so you can extend promise in a way that carries the behavior forward on promise chaining. If you want a derived promise that doesn't propagate its behavior, you can simply set the CustomPromise.prototype.constructor to Promise. Plain and simple.

you can copy and paste the first JS in this previous answer and see that the callback is executed multiple times, actually 4 there, where there are 2 sub classes and one then ... but I have the feeling you are focusing on the final consumer callback executed only once, which is correct, but when extending, that final callback is not the issue, what happens in the extend super call is.

Object.create construct objects too and I am suggesting a then(...) should not need to new this.constructor(callback) to have anything installed ... also your code uses a private #reactions field which is a bit off, but I take your code as an example. Can that field though be installed without re-invoking the constructor?

does this mean something like this is the way to extend promises?

class Sub extends Promise {}
Sub.prototype.constructor = Promise;

if that's the case, can you see this is a bit counter-intuitive compared with how every other class extend works?

I am OK with this answer (I need to test it but I trust you) but that's not how I'd extend anything out of the box, and that also (I think) would still lose the brand after a .then(...) call, so that looks like an indirection to Symbol.species to me, which might work, but it doesn't make Symbol.species less needed for these kind of cases ... would you agree in there?

Object.create doesn't invoke constructors, Object.create(x, p) is basically just sugar for Object.defineProperties({ __proto__: x }, p). So it creates an object but I wouldn't call what it's doing "constructing".

1 Like

And there it is, you understood. An executor callback provided to a promise constructor is executed only once, by the base promise constructor. Of course if the promise constructor is executed multiple times (with different callbacks), there will be multiple callback invocations, one for each construction.

I used a private field as the equivalent of a slot. Same concept.
Private state is initialized through construction. As @ljharb mentions Object.create simply creates an object with a given prototype, it does not construct an object (initializes its state), which is the job of constructors...

No. That's what you need to do, like for every other built-in, if you don't want to propagate your derived class to the result of the built-in methods. Aka if you want to construct a base object instead of a derived one. There is nothing specific to promises here.

OK, I believe I've found a way to fulfill my requirements:

  • new Thing is an instance of Thing (brand check)
  • .then(...args) is still an instanceof Thing (brand preserved)
  • the internal super(() => { ... }) is never executed more than once

The boilerplate that solves both cases is:

const {Promise: Builtin} = globalThis;
class Promise extends Builtin {
  // this is kinda mandatory / needed
  static [Symbol.species] = Builtin;
  constructor(callback, {signal}) {
    super((resolve, reject) => {
      signal.addEventListener('abort', reject);
      callback(resolve, reject);
    });
  }
  then(...args) {
    // this is to preserve the brand
    return Object.setPrototypeOf(
      super.then(...args),
      this.constructor.prototype
    );
  }
}

and can be tested via:


const controller = new AbortController;
const p = new Promise(
  (resolve) => {
    setTimeout(resolve, 100, 'auto');
  },
  controller
);

// all conditions must be satisfied
console.assert(p instanceof Promise, 'brand not ok');
console.assert(
  p.then(console.log, console.error) instanceof Promise,
  'then not ok'
);

// test abort controller
setTimeout(() => controller.abort(), 50);

This thread title is still relevant, as I wouldn't need Symbol.species that de-brand to then re-brand in other builtin cases, or at least I can't think of any other constructor where that's desired or needed, so while I thank you all for hints my doubts remain:

  • it is awkward to need that de/re-brand dance, but at least it's clear now why that's needed
  • extending Promise still might have undesired side-effects if the correct way is unknown
  • Symbol.species is discouraged so now I don't know if I should use it, because I don't want to publish a module that will suddenly fail at the next browser update

What are your thoughts, specially on the latte point? :thinking:

If Symbol.species support were removed, then Builtin would be the assumed constructor anyways, I suspect, so you'd be fine.

1 Like

Personally I would write this as a utility rather than as a constructor. Changing the prototype of an object is a performance cliff.

function promiseWithSignal(callback, { signal }) {
  if (signal.aborted) return Promise.reject(new Error("Aborted"));
  let rejectCb;
  return new Promise((resolve, reject) => {
    rejectCb = reject;
    signal.addEventListener("abort", reject, { once: true });
    callback(resolve, reject);
  }).finally(() => {
    signal.removeEventListener("abort", rejectCb);
  });
}

But perhaps there are other reasons Promise is being subclassed beyond modifying the constructor?

1 Like

the module is meant as drop-in replacement for the Promise and the AbortController so that Promise.resolve among other methods, must be available and work as expected like it would with the Builtin. However, my current module solution uses a Function and re-brand the function itself as Promise so that there is no brand to preserve, everything is just a promise and all promises are abortable or resolvable.

That's the summary [in here]( promise/index.js at main ยท WebReflection/promise ยท GitHub)

Thanks for the extra info. That looks like a good compromise.

Side note: if the AbortController is kept alive and used for multiple promise constructors it could leak listeners if they are not removed. This is one of the downsides of AbortController being built on EventTarget, it's not very GC friendly.

If you want to make your promise a drop-in replacement but don't care about adding behavior to the promise instances beyond the explicitly construct calls, the function approach may actually be the better one. Here is a slight improvement on your code so that Promise can still be extended:

const {Promise: Builtin} = globalThis;

function Promise (executor, {signal} = {}) {
  if (!new.target) throw TypeError();
  Reflect.construct(Builtin, [(resolve, reject) => {
    if (signal) {
      signal.addEventListener('abort', reject);
    }
    executor(resolve, reject);
  }], new.target);
}

Promise.prototype = Builtin.prototype;
Object.setPrototypeOf(Promise, Builtin);

All your instanceof checks will pass, the new constructor will be usable both with and without the controller, and static methods of Promise will work too.

The only quirk will be that Promise !== Promise.prototype.constructor, which honestly almost no-one will notice.

that's basically what I've done in here except I think you might have forgotten a return there? True that my solution doesn't really allow extends (I care less about throwing on no new target as I think Reflect throws already when new.target is undefined) so thanks for the hint, I'll improve my module :+1:

edit your code gives me maximum callstack ... I think there's something wrong with it (like when there is no extend so that Reflect.construct there invokes itself as new target)

interesting ... all I wanted to do there is use the primitives the platform provides already ... sad to learn the platform provided leaky primitives but to me being able to abort multiple promises at once passing the same signal is a great feature, not really something to avoid.

good hint to keep in mind though

No return needed

Oops typoed Reflect.construct(Promise instead of Refect.construct(Builtin

In the case of EventTarget, well one that's on the HTML side, and two, that's kinda the design of the API to have to call removeEventListener.

That said, this pattern will exacerbate a leaky behavior of most Promise implementations: most engines keep a strong reference from the resolvers onto the promise instance, even after resolution, and thus onto the fulfillment value if any. That causes all kind of leaks like Promise.race with a long pending promise causing all raced promises to be kept alive until the last promise resolves. We work around some of these issues in our wrappers, but some leaks need engine fixes.

It's not a problem of the spec per se, it's just a consequence of implementations naively following the spec text. Implementations are allowed to collect promises independently of their resolver, especially when the promise is resolved. I need to get back to formalizing the collection behavior of promises and their resolvers, and push implementers in not leaking so egregiously. Here is an outdated audit I did some time ago: Test cases for promise and resolver collection ยท GitHub

Back to your case, I would wrap the resolvers and unregister the event listener from the signal once either resolver is called. That should solve both leaking problems.