now I have Uncaught TypeError: Method Promise.prototype.then called on incompatible receiver #<Promise>
not really, all listeners disappear if nodes are removed or GCd so that's never been needed ... otherwise every innerHTML since 2000 would've leaked listeners ... but fair enough it's a DOM primitive, not an ECMAScript one, still it's the only one that define abortability for promises such as fetch operations.
uhm ... then I need to re-think a bit the logic but this seems doable, thanks.
@mhofman indeed you do need to return the React.construct or you are creating a non referenced instance nobody knows about ... your code works like this, it doesn't the way you wrote it without return:
(() => {
const {Promise: Builtin} = globalThis;
function Promise (executor, {signal} = {}) {
return Reflect.construct(Builtin, [(resolve, reject) => {
if (signal) {
signal.addEventListener('abort', reject);
}
executor(resolve, reject);
}], new.target);
}
Object.setPrototypeOf(
Promise,
Builtin
).prototype = Builtin.prototype;
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);
})();
Ugh I'm not awake, return is needed. Really sorry.
1 Like
last comment, for history sake ... Closure Compiler that people at Angular seems to be forced to use, for some reason, doesn't support new.target ... I've updated my module to use the best way to solve what it's meant to solve but I've basically erased Closure Compilers users from being possible consumers of the module ... I don't like that a compiler can't cope with ES2015 (it's 2023!!!) but maybe others do need such compiler for some obscure reasons so, as summary, using new.target might not always be a solution out there.
Re: Symbol.species. When looking at some of the more recent proposals none of them include support for looking up Symbol.species when methods return new instances.
(probably other proposals too, these were just the ones that came to mind)
Isn't all of this made moot by the fact that both await and async implicitly call Promise.resolve() ?
The extended behavior is lost at that point anyway.
1 Like
I was surprised to see that no one here actually answered your underlying question, which was "how can I make this code work". I have no idea if this is still a thing you're looking for an answer on, but just in case it is, I'll give you (a) the working code that does what you were trying to do, (b) an explanation of why you don't actually want that, and some code that does what you do want, and (c) a hopefully more-understandable explanation of why this doesn't work like you expected.
First off, here's the code that you thought you were writing and which runs, but doesn't do what you want:
class Disposable extends Promise {
constructor(executor, {signal}) {
super(r => r());
return new class extends Promise {
constructor(executor) {
super((resolve, reject) => {
signal.addEventListener('abort', reject);
executor(resolve, reject);
});
}
}(executor);
}
}
const controller = new AbortController();
new Disposable(
(resolve, reject) => {
setTimeout(resolve, 1000, 'OK');
},
controller
)
.then(console.log)
.catch(console.error);
// abort before Promise is resolved
setTimeout(() => controller.abort(), 500);
This will in fact abort every single Promise that has been attached to the then-chain stemming from this call to new Disposable(...), and the call to console.log() will not execute. So far, so good?
Not quite, because the call to console.error won't execute either. Three Promise objects have been created[1]: one encapsulating the setTimeout(resolve...) call, one with console.log, and one with console.error. Once you call abort() on the controller, all three Promise objects will instantly get rejected... and once a Promise has been resolved in any way (by fulfillment or rejection), it will never have its then, catch, or finally methods run again. So the engine gets around to that .catch(console.error) clause, and it thinks that clause has already run, so it doesn't call console.error a "second" time. To make matters worse, you'll also get a nasty "Uncaught exception" error from your browser, because it thinks your catch handler threw an exception itself.
What you actually want to do is make sure that every Promise in the chain thinks that it has received a rejection from the Promise ahead of it in line, and you can do that with this code:
class Disposable extends Promise {
constructor(executor, {signal}) {
super(signal ? r => r() : executor);
if (!signal) return;
const activeRejectors = new Set();
signal.addEventListener("abort", e => {
for (const rejector of activeRejectors) {
rejector(signal.reason);
}
activeRejectors.clear();
});
const DisposableSpecies = class extends Disposable {
#rejector;
constructor(executor, isInitialPromise=false, tempObj={}) {
super((resolve, reject) => {
tempObj.reject = reject;
executor(resolve, reject);
},
{signal: false}
);
this.#rejector = tempObj.reject;
if (isInitialPromise) {
activeRejectors.add(this.#rejector);
}
}
then(onFulfilled, onRejected) {
onFulfilled = this.#wrapResolver(onFulfilled, true);
onRejected = this.#wrapResolver(onRejected, false);
return super.then(
v => !signal.aborted ? onFulfilled(v) : onRejected(signal.reason),
onRejected
);
}
#wrapResolver(resolver, isFulfill) {
resolver ??= (isFulfill ? v => v : e => {throw e;});
return v => {
activeRejectors.add(this.#rejector);
let isAsync = false;
try {
const rv = resolver(v);
if (typeof rv === "object" && typeof rv?.then === "function") {
rv.finally(() => activeRejectors.delete(this.#rejector));
isAsync = true;
}
return rv;
} finally {
if (!isAsync) activeRejectors.delete(this.#rejector);
}
}
}
};
return new DisposableSpecies(executor, true);
}
}
There's a version with comments inline at the bottom of the post, but first, an explanation of what's going on here, and why Promise is so hard to extend properly. (This applies to Array as well, which has similar semantics around "species".)
I'm going to start from the assumption that you understand a particular basic concept of object-oriented programming: that any derived class must implement the same API as its base class. Most (perhaps all) statically-typed languages enforce that at the compiler level, so you may be familiar with it most in the form of compiler errors, but in short, there are a few rules you need to follow whenever you extend any class in any language, or the code will break, either at compile-time or at runtime:
- If the base class has method
foo(), then callingfoo()on the derived class should return a value of the same type as the base class (or, perhaps, a subclass of that type). - If the base class has method
barwhich takes arguments(baz, bim), then you should be able to callbar(baz, bim)on the subclass using the samebazandbimvalues as you would have passed to the basebarmethod without causing anything to break. - If the base class has a public field named
zotthat can take certain values, you should be able to assign the same values to the derived class's field namedzot(without causing breakage).
That's really all that matters. In code form, those three rules look like this, which the compiler at least won't have any problem with:
class Base {
zot = "Good";
foo() {
return zot.length;
}
bar(baz, bim) {
return baz.startsWith(zot) && bim.endsWith(zot);
}
}
class Derived {
zot = 5; // bad! zot is supposed to be a string!
foo() {
return `${base.foo()} characters`; // bad! foo() is supposed to return a number!
}
// bad! I should be able to call bar("Good Morning", "All Good")!
bar(start, len) {
return this.foo().slice(start, len);
}
}
Now, you may be thinking, "but I didn't break any of those three rules", because it's true, you didn't, at least not directly. And in fact, the API of a Promise instance is extremely small; it has three methods: then, catch, and finally, and that's basically it, and you didn't override any of them. In fact, you can just ignore catch and finally, since they're both defined as convenience functions that call then with different parameters, so really it's just the then() method that has to conform to the public API of a Promise. Right?
Well... almost. Because there's actually one more element of the public Promise API, and it's the field called constructor, which is present by default on all JavaScript objects, defaulting to the identity of whatever constructor function was used to instantiate the object in the first place. Normally you don't (or at least I don't) think much about the constructor field, except if you want to check that two objects are the same type like foo.constructor === bar.constructor. But in this case, the default behavior of Promise.then makes use of that field; it expects somePromiseObject.constructor to refer to "a constructor that you can call with a single argument which is an executor function, and which returns something like a Promise". And so that means that as far as the default Promise.then implementation is concerned, the constructor property is in effect an API method that expects to have the following call signature (using Typescript syntax here):
interface Promise {
constructor: new(executor: Executor) => PromiseLike;
}
type Executor = (resolve: Resolver, reject: Resolver) => void;
type Resolver = (valueOrError: any) => void;
And on instances of your Disposable class, the .constructor field resolves to a constructor that can't be called with just one argument, and that's what's breaking your code - it's a violation of rule #2 up there, that instance methods (which .constructor() is, sort of, if you squint) need to be able to be called with the same parameters in the derived class as you could have called it in the base class.[2]
So, what's the solution here? At the core, it's the same as any other match-the-API task: you just need to make sure that the constructor property of your cancelable-promises refers to something that at least "looks like" a constructor function that can take just one argument. And the easiest and cleanest way to do that is to make it so that your class's constructor does just take one argument. That's why changing your constructor parameters from (callback, {signal}) to (callback, {signal} = {}) helped - by giving the second parameter a default value, you turned the Disposable into a class that can be instantiated with a single parameter.
The only problem is that we do want our constructor to be able to use more information than just the executor function. It's the same task as "I want a function that adds some arbitrary constant value to anything that gets passed to it", and it has the same answer: closures.
function makeConstantAdder(constantValue) {
return x => x + constantValue;
// basically the same, but using anonymous function syntax
// return function(x) { return x + constantValue; }
}
function makeConstantAdderClass(constantValue) {
return class {
constructor(x) { this.result = x + constantValue; }
};
}
const addFiveFunction = makeConstantAdder(5);
const AddFiveClass = makeConstantAdderClass(5);
console.log(addFiveFunction(7)); // 12
console.log(new AddFiveClass(7)); // Object { result: 12 }
const AddSevenClass = makeConstantAdderClass(7);
console.log(new AddSevenClass(5)); // Object { result: 12 }
// the anonymous classes are all unique, regardless of if the parameters are the same.
console.log(AddFiveClass === AddSevenClass); // false
console.log(AddFiveClass === makeConstantAdderClass(5)); // false
console.log(makeConstantAdderClass(5) === makeConstantAdderClass(5)); // false
With that out of the way, the only other weirdness about my solution is that it involves the fact that ECMAScript lets you return arbitrary objects from constructors to customize the return value of the new ClassName() expression. So, despite calling new Disposable(), the returned object isn't actually of type Disposable but rather, it's an object of type of the anonymous class created in Disposable's constructor.
That's it for storytime, on to the comments-included form of my solution:
class Disposable extends Promise {
constructor(executor, {signal}) {
// This constructor is called in two cases: (1), when outside code calls
// `new Disposable` and passes in an AbortController. (2), when a subclass
// calls `super()`. We can tell these two cases apart by if we have a signal.
// In case 1, where this is a direct call to new Disposable(), we don't want
// the base Promise constructor to call the executor just yet, so we pass it
// a dummy function that just resolves immediately.
super(signal ? r => r() : executor);
// In case 2, we've already handed the executor off to the base Promise class,
// so we're done. Just return.
if (!signal) return;
// We're in case 1 now, so we want to set up for getting an abort signal.
// When we do, we want to instantly reject any Promises that are currently
// in-flight, so record all their rejector functions here. Order doesn't matter.
const activeRejectors = new Set();
// Bind the abort listener just once for this chain of Promises.
signal.addEventListener("abort", e => {
for (const rejector of activeRejectors) {
rejector(signal.reason);
}
// we don't need those references anymore
activeRejectors.clear();
});
// The "species" of Promise that we want to return isn't actually the generic
// Disposable class that this comment appears in. We want to return a subclass
// of Disposable that will always be related to this signal and no other, no
// matter what gets passed to its constructor, and we define the subclass here:
const DisposableSpecies = class extends Disposable {
#rejector;
// Disposable will call this constructor setting isInitialPromise to true.
// All other instantiations will pass only one argument, so it will be false.
// tempObj is used here because there's no other way to declare variables
// prior to the call to super(). No, I don't like it either.
constructor(executor, isInitialPromise=false, tempObj={}) {
super((resolve, reject) => {
// record the rejector function before passing it along to the executor
tempObj.reject = reject;
executor(resolve, reject);
},
// our base constructor is Disposable, so be sure to pass something that
// won't cause the {signal} deconstruction parameter to fail
{signal: false}
);
// save the rejector locally, but don't expose it
this.#rejector = tempObj.reject;
if (isInitialPromise) {
// we've already run the executor, so this Promise is already in-flight.
// record its rejector in the list.
activeRejectors.add(this.#rejector);
}
// Done with construction! If this is a follow-on Promise (created by .then)
// then it's not executing yet, so nothing needs to happen yet.
}
// .then(), .catch(), and .finally() are all implemented in terms of .then(),
// so this is the only one of those that needs to be overridden. It's also
// the only place where you can get access to that point of time in between
// one Promise resolving and the next beginning execution.
then(onFulfilled, onRejected) {
// onFulfilled and onRejected are the functions that actually contain the
// executable code for this .then call, so wrap them in a function that
// registers our rejector with activeRejectors, then removes it after
onFulfilled = this.#wrapResolver(onFulfilled, true);
onRejected = this.#wrapResolver(onRejected, false);
// Now we're ready to pass them to the base .then implementation, but
// we want some special handling for the "fulfilled" handler, to make sure
// nothing in this chain will ever execute a fulfilled handler after the abort
// signal has been received.
return super.then(v => {
// At this point, the previous Promise in the chain just got fulfilled with
// the value v. As long as the signal hasn't been aborted yet, go ahead
// and call onFulfilled. Otherwise, call the rejection handler.
return !signal.aborted ? onFulfilled(v) : onRejected(signal.reason);
}, onRejected // doesn't need modification
);
}
#wrapResolver(resolver, isFulfill) {
// When an undefined resolver is passed to .then(), the spec defines default
// behaviors: if this is a fulfill handler, pass the input value along, but
// if this is a rejection handler, propagate the exception (by throwing it).
resolver ??= (isFulfill ? v => v : e => {throw e;});
// Resolvers can return in one of three ways:
// 1. return a value, which marks this Promise as fulfilled with that value
// 2. throw a value, which marks this Promise as rejected with that value
// 3. return a Promise-like object, which defers resolution until it's done
return v => {
activeRejectors.add(this.#rejector);
let rv;
try {
rv = resolver(v);
} catch (e) {
// synchronous rejection, case #2
activeRejectors.delete(this.#rejector);
throw e;
}
// this matches the logic in the spec for "is this Promise-like"
if (typeof rv === "object" && typeof rv?.then === "function") {
// This is an async resolution, case #3. We need to keep this on the list
// of active rejectors until the resolution is done
rv.finally(() => activeRejectors.delete(this.#rejector));
} else {
// It's not Promise-like, so this is a synchronous fulfillment, case #1
// remove us from the list of active rejectors
activeRejectors.delete(this.#rejector);
}
return rv;
}
}
};
// That's the full subclass definition. We want to return an instance of our
// new subclass as the return value for "new Disposable(...)", so construct
// and return it here.
return new DisposableSpecies(executor, true);
}
}
Actually four, but one of them gets discarded and we can ignore it, it never had access to the executor. âŠī¸
Several people are already chiming in to clarify that it's not necessarily the constructor property itself, it's the constructor's species, which is a static property on the Promise class that acts as a signal for "what class of object should I instantiate if I need to make another one of these". It's extremely confusing and it doesn't make any difference to this explanation, they're just being pedantic. âŠī¸