[Try] Async stan

[adamziel]

Description

For now this PR won't even work - it serves as a discussion prompt. I didn't bother to update types or consumers of this API as I'd like to talk it out first.

#26866 introduced a new state management system called stan. As discussed in #26866 (comment), there are some rough edges related to asynchronicity. For example, resolution rejection of dependent derived atoms are suppressed and never bubble up:

subscribe( listener ) {
	if ( ! isListening ) {
		isListening = true;
		resolve(); // This returns a promise, rejections go unnoticed and the only symptom is unresolved "parent" atom
	}
	listeners.add( listener );
	return () => {
		listeners.delete( listener );
	};
},

I was pondering what's the right way of approaching this problem and concluded that as soon as there are promises in the mix, everything handling these promises should also be "promisified". This PR is an experiment in which everything in stan becomes async.

Usage would change from this:

const sum = createDerivedAtom(
	( { get } ) => get( count1 ) + get( count2 ) + get( count3 )
);

to this:

const sum = createDerivedAtom(
	async ( { get } ) =>
		( await get( count1 ) ) +
		( await get( count2 ) ) +
		( await get( count3 ) )
);

Ubiquitious await is an obvious downside. Also in this implementation it's even more important to cancel resolution when a "concurrent" update arrives - I think rx.js could make this part very easy but I abstained from introducing a new dependency for now.

Upsides are that all the errors are now catchable as they'll bubble up the promise chain. We also no longer need any heuristics to reason about exceptions (if ( unresolved.length === 0 ) { throw e; }). Also, should we decide to switch to any other way of handling the state internally, this API opens many doors that are not trivially opened with synchronous get.

cc @jsnajdr @youknowriad @gziolo

[adamziel]

Unsubscribe callback should be available right away so maybe this should return synchronously with an array of promise and unsubscribe callback?

[youknowriad]

Why do we need all the async/wait for the getters for sync atoms and subscriptions? Note that even if the behavior is sync, JS will make it async and only resolve on the next tick and this breaks the editor (and useSelect) in various ways. It is important that sync selectors stay sync and resolve right away.

[adamziel]

Good point, I’ll think some more about this on Monday. It seems like Recoil separates atoms and selectors, and only the latter may be async - maybe that would be a better way to go, our API is already pretty similar. I wonder how it handles errors thrown on the intersection of the two though, like propagation of the atom value to async selectors

[youknowriad]

Maybe we can have an "error" property per atom, like we have get, we'd have hasFailed and getError ?

[youknowriad]

Alternatively, it can be just an internal value and if it's set we throw on .get

[adamziel]

Maybe we can have an "error" property per atom, like we have get, we'd have hasFailed and getError ?

I think that would break stack traces similarly to how rungen does

[jsnajdr]

Ubiquitious await is an obvious downside.

Yes, making everything async should not be needed and it would introduce its own set of problems. There are use cases that are completely synchronous, like flipping a toggle state in memory, others are asynchronous. The library needs to support both seamlessly.

Upsides are that all the errors are now catchable as they'll bubble up the promise chain.

I don't understand why switching between sync and async should change anything about error handling. Sync functions return values or throw exceptions. Async functions return promises of values or rejected promises with errors (and never throw exceptions). There is a complete 1:1 symmetry between them.

I think the system described below should work. It distinguishes clearly between sync and async operations, reports special states as exceptions, and should never swallow any error.

.get():
always synchronous, returns the current value. If the value is not available because the resolver failed, it re-throws the exception thrown by the resolver. If the value is not available because resolver is still running, it throws a special PendingResolution error. That error can have a .promise field, containing a promise that resolves on resolution. error.promise.then( () => atom.get() ) guarantees that the atom is resolved, either with value or an error. Useful for React Suspense that enjoys throwing promises.

It's important here that .get() returning null no longer means anything special. null is a value like any other, not reserved by the library, and available to the app that uses the library. Special states are communicated by exceptions.

.getState():
a variant of .get() that never throws exceptions, but returns a "state" object with shapes like:

• { state: 'hasValue', contents: 1 }
• { state: 'hasError', contents: Error( ... ) }
• { state: 'loading', contents: Promise( ... ) }
  Very convenient, e.g., for React code if we don't want to write try/catch blocks everywhere and prefer ifs or switches.

The .get() and .getState() getters are important for React: React always wants synchronous values when rendering, because it renders the state as it is right now. APIs that return promises are quite useless for React. If something will be available later, React is happy to rerender it after notified.

.resolve() or getResolved():
returns a promise of resolved value, an async version of .get(). Useful for code that wants the resolved value and is not interested in intermediate states. Like when implementing async flows that are currently typically found in controls.

.subscribe()
Subscribes to state updates of the atom. Nothing new here.

.peek() and peekState()
Variant of .get() that never triggers the resolver, but returns the current state without triggering any side-effects. It's a more basic primitive than .get(), which can be implemented as a combination of .peek() and .resolve().

[adamziel]

Yes, making everything async should not be needed and it would introduce its own set of problems. There are use cases that are completely synchronous, like flipping a toggle state in memory, others are asynchronous. The library needs to support both seamlessly.

Agreed, it seems like making everything asynchronous is not the right way to go here.

I don't understand why switching between sync and async should change anything about error handling. Sync functions return values or throw exceptions. Async functions return promises of values or rejected promises with errors (and never throw exceptions). There is a complete 1:1 symmetry between them.

What I meant is that a synchronous function doesn't have any way of throwing the error as soon as it happens – so the same problem you meant to address with the synchronous get() that re-throws an exception from the failed resolver.

.get(): always synchronous, returns the current value. If the value is not available because the resolver failed, it re-throws the exception thrown by the resolver.

I was trying to avoid that, but yes, there may be no other way here. I just don't like how it's unclear what's the right stack trace in this scenario. Should it stem from the first get? The second get? What if these calls stem from different components? Is it going to be confusing and cause folks to lose time by going after the wrong stack trace?

I also wonder how would the errors propagate in that model, e.g. what should happen in the following scenario:

Should derived atoms 2, 3, 4 and 5 switch to an error state or just remain unresolved forever? Would we throw the same error each time get() is called? So once in component 1, once in component 2, once in component 3 - three times in total. Would the developer see three instances of the same error with the same stack trace in the console?

[jsnajdr]

What I meant is that a synchronous function doesn't have any way of throwing the error as soon as it happens

I'm not sure I understand here: are you concerned mainly about the stack trace of the error? It's true that the "resolver threw an error" and ".get() throws an error" events are happening at different time each. So the stack trace will be "cut" into two parts. Because the error was stored somewhere and the .get() merely retrieves and throws the stored value.

The Error.cause TC39 proposal could be useful here. It introduces chained errors, something that's been in Java for a very long time. Then the resolver can store an error, with a call stack, and then .get() could throw a different error, with the original one as .cause. Looking at them together will reveal the whole stack trace.

[adamziel]

@jsnajdr TIL about Error.cause - it is a great proposal, thank you for sharing!

[jsnajdr]

I also wonder how would the errors propagate in that model, e.g. what should happen in the following scenario:

I think a good and consistent mental model is: the selectors are just like function calls, where the functions are called only once and the result is memoized (both returned value and thrown exception).

In your example: yes, derived atom 1 is resolved with error and stays in that state forever. On the first call of get( derived1 ), the error is thrown by the resolver function, stored in atom state, and immediately re-thrown from get(). On the second and subsequent calls, the resolver function is no longer called and the stored error is re-thrown.

It's the same with derived atoms 2, 3, and 4. Their resolvers also fail, this time because the resolvers call get( derived1 ) and that throws an exception. The resulting error is also cached and re-thrown on each call.

If you want to "unblock" the chain and retry the resolution, you do that basically by invalidating the memoization cache or setting some caching policy on it, other than "remember forever". We are getting very close to what a react-query-like library for data fetching, built on top of stan, would also need.

The derived atom can also have a policy to retry the resolution several times before finally reporting the error upstream. The Calypso data layer, implemented by @dmsnell , has that feature. There is a large universe of options on how to precisely configure the resolvers and their cache.

[jsnajdr]

TIL about Error.cause - it is a great proposal, thank you for sharing!

The good news is that we probably don't need to wait for anyone to implement or start supporting the proposal. I don't see a reason why we couldn't start setting .cause fields on Errors today. Standardization might bring some nice devtools support, and libraries can start using it consistently, but that's all.

[gziolo]

#26866 got reverted so I believe we should close this PR as it depends on the code that is not used.