Creating computations when resolving Promises
webstrand opened this issue · 2 comments
When using the CKEditor5 API, creating a new editor returns a Promise which must be resolved to obtain the newly created editor object: ClassicEditor.create(textarea).then(editor => { ... })
. I needed to bind a data signal to the editors change event, unfortunately by the time the promise is resolved, Owner information is no longer available so any computations created will never be disposed.
A simple solution would be to resolve the promise into a DataSignal and create a computation which depends thereby. Unfortunately, this means that the dependent computation will be run twice, and the DataSignal will remain in the graph until the parent is disposed.
I've implemented a solution S.resolve(promise, onfulfilled, onrejected)
which behaves with the same semantics as Promise.then, but restores the Owner information when onfulfilled or onrejected is executed.
S.resolve = function <T, TResult1 = never, TResult2 = never>(promise: PromiseLike<T>, onfulfilled: ((value: T) => TResult1 | PromiseLike<TResult1>), onrejected?: ((reason: any) => TResult2 | PromiseLike<TResult2>) | undefined | null): PromiseLike<TResult1 | TResult2> {
const owner = Owner;
const _onfulfilled = function (value: T): TResult1 | PromiseLike<TResult1> {
Owner = owner;
const result = onfulfilled(value);
Owner = null;
return result;
}
const _onrejected = onrejected ? function (reason: any): TResult2 | PromiseLike<TResult2> {
Owner = owner;
const result = onrejected(reason);
Owner = null;
return result;
} : undefined;
return promise.then(_onfulfilled, _onrejected);
}
This way any computations created when resolving the Promise will be owned by the correct ComputationNode.
I tried to implement a wrapper for Promises, so that async/await could be used while preserving Owner. But await has to resolve the Promise fully before continuing execution of the async function, so I'm fairly confident it's impossible to preserve the Owner inside async without exposing Owner as part of the API or through a proxy object.
First, congrats on reading the S source well enough to figure out how to extend it! I need to add some comments there ...
I've usually handled Promises via the method you mention, pushing the result of the promise into a data signal watched by a computation. The issue with the computation running twice can be fixed by defining it with S.on(result, fn, true)
.
A variant, which is a little cleaner and works ok with async/await syntax, is to have the promise chain return a thunk, which then gets assigned to a data signal watched by a computation which executes it and therefore owns any computations produced. You can see this strategy in the async router used in the surplus-realword demo.
I haven't added anything to core S for dealing with Promises because there are design questions which I don't have good answers for yet:
-
What if Owner has been disposed before the Promise completes? Should subsequent parts of the Promise chain run or not? If we think of those subsequent actions like child computations, then the answer is no, but if we think of them like actions spun out of data changes in the app (aka these Promises could be saving changes to the server) then the answer is yes.
-
What if the computation that created the Promise has re-run, generating another Promise, before the original Promise completes? Should subsequent steps for the original Promise be run or not? (In the async router code linked above, you can see that it decides not to run stale Promises, but that's a domain-specific decision.)
-
Is this an issue specific to Promises or a more general one? Should there be a utility that makes ownership a first-class value, so that we can say "run this fn, and any computations created follow the lifecycle of this other computation"?
I suppose that's a long way to say that a) I consider interaction with async code an area of current research, b) love to hear your thoughts on it, and c) in the meantime, and especially with the fact that answers may be domain-specific, I've followed the practice of proxying through a data signal to handle async code.
I've updated my local version of S.js, and due to changes in the library (I have no idea what getCandidateNode
is doing), I've had to update my approach:
S.await = function <T, TResult1 = never, TResult2 = never>(promise: PromiseLike<T>, onfulfilled: ((v: T) => TResult1 | PromiseLike<TResult1>), onrejected?: ((reason: any) => TResult2 | PromiseLike<TResult2>) | undefined | null): PromiseLike<TResult1 | TResult2> {
// undefined when the promise hasn't yet resolved or rejected
// null when the owner computation has been disposed
// instanceof Function when the promise has resolved and the owner has not yet been disposed.
let childDisposer: null | (() => void) | undefined;
S.cleanup(() => childDisposer && childDisposer() && (childDisposer = null));
return promise.then(
function (value: T): TResult1 | PromiseLike<TResult1> {
// Protect ourselves from custom Promises with incorrect semantics
if (childDisposer !== undefined) throw new Error("Promise resolved or rejected twice");
if (childDisposer === null) return Promise.reject();
return S.root((disposer) => {
childDisposer = disposer;
return onfulfilled(value);
});
},
onrejected ? function (reason: any): TResult2 | PromiseLike<TResult2> {
// Protect ourselves from custom Promises with incorrect semantics
if (childDisposer !== undefined) throw new Error("Promise resolved or rejected twice");
if (childDisposer === null) return Promise.reject();
return S.root((disposer) => {
childDisposer = disposer;
return onrejected(reason);
});
} : undefined
);
}
I think this version mostly addresses your concerns:
-
In the use case I envision, the
Promise
is created by the parent computation, so any child computations should have their lifetime tied to the parent. -
I haven't found a reason to disallow subsequent
Promise
s, the lifetime is still tied to the parent computation. But because I have to reject the Promise when the parent computation is disposed before the Promise resolves, the onrejected handler becomes a little bit more useless. -
I've attached a cleanup to the parent computation, so that when it re-runs or is disposed, the child computations are also correctly disposed.
There may be a more general solution, something like:
function SDefer<T extends unknown[], U>(fn : (...args: T) => U) : (...args: T) => U | undefined;
function SDefer<T extends unknown[], U, V>(fn : (...args: T) => U, defaultValue: V) : (...args: T) => U | V;
function SDefer<T extends unknown[], U, V>(fn : (...args: T) => U, defaultValue? : V) : (...args: T) => U | V {
let childDisposer : (() => void) | undefined | false;
S.cleanup(() => childDisposer && childDisposer() && (childDisposer = false));
return function resolver(...args : T): U | V {
if (childDisposer === false) return defaultValue!;
if (childDisposer) childDisposer();
return S.root((disposer) => {
childDisposer = disposer;
return fn(...args);
});
}
}
My original intent, however, was to avoid the cost of creating a new ComputationNode
and cleanup callback, since most handlers will immediately create new ComputationNodes
anyway.