[core] Spawn/invoke behaviors #2280
Conversation
|
|
IMHO behaviors are still somewhat a subject for refinement (maybe they will not need further refinement - but this has not been decided yet), see for example this thread: #2242 (comment) . I think it would be better to skip introducing this concept into v5 to avoid risking breaking changes further down the road for something that has just been introduced recently. |
We don't have to expose/document it; behaviors can just be a refactor abstraction if you feel strongly we shouldn't be introducing this into v4. This has no breaking changes, and actually can clean up the code somewhat. |
|
I'm just somewhat hesitant to expose this publicly at the moment - and not sure if there is much value in cleaning up the v4 codebase (we'll have to continuously resolve conflicts with the next branch and all) |
There was a problem hiding this comment.
As mentioned privately - it would be really helpful to me if we could introduce fromReducer here, or at least propose the API and provide pseudocode for its usage. It would paint a better picture for me of how being able to spawn behaviors is an enabler for that feature and, in general, how things fit together.
Added here: 65b044c |
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
packages/core/src/behaviors.ts
Outdated
| observers.forEach((observer) => { | ||
| observer.error(event.error); | ||
| }); | ||
| return undefined; |
There was a problem hiding this comment.
isn't returning undefined here meaning that we won't be able to display the error in a component when using smth like this?
const actorRef = useSpawn(fromPromise(() => Promise.reject(new Error('Foo.'))));
const [state] = useActor(actorRef);should we allow it though? if not - how the user should handle those errors in the component's context
There was a problem hiding this comment.
By using an observer:
useEffect(() => {
actorRef.subscribe({ error: e => setError(e) });
}, [actorRef]);There was a problem hiding this comment.
isn't this boilerplate-y though? as a user I would expect error handling to be more of primary concern and not something that I have to wire up myself. By making it harder to handle this we risk people not handling errors as people are lazy (me included 😉 ). Some will handle them, some will not - the API doesn't make it easy though, it doesn't create a pit of success here.
There was a problem hiding this comment.
What would a more convenient API look like? What we can't do is shove the error in the state; the state is meant to represent the emitted value.
Maybe we can do something like this:
const [state, send, { error }] = useActor(actorRef);There was a problem hiding this comment.
Another alternative:
const error = useActorError(actorRef); // usually `undefined`There was a problem hiding this comment.
We might want to throw an error during rendering - which would require wrapping the internal state of useActor into a wrapper object, subscribing to errors like you have showcased here and throwing during rerender. Similarly to how, for example, react-query handles that:
https://github.com/tannerlinsley/react-query/blob/16b7d290c70639b627d9ada32951d211eac3adc3/src/react/useBaseQuery.ts#L124-L131
There was a problem hiding this comment.
@Andarist I'm revisiting this, because you're right, the way we're representing promise state is a bit awkward.
What are your thoughts on this?
const [{ data, error, status }, send] = useActor(fromPromise(/* ... */));There was a problem hiding this comment.
Made the changes here: 30c310c
There was a problem hiding this comment.
Oh, I have sort of commented on that a minute ago on Discord so let me just copy-paste my comment from there:
Huh, a lot of catching up here - I was thinking about the error stuff as well, just didnt have time to write down my thoughts here. I came to the conclusion that, in my opinion, we should throw errors and let error boundaries catch them - what @mattpocock#9242 said initially. This solution~ might be framework-specific, for now I'm thinking mostly about React and
@xstate/react. I think it's reasonable for APIs of framework integration packages to diverge because they should feel native to the users of those frameworks, exact API parity between them is nice but imho a conceptual non-goal.
I'm not saying that API definitely has to diverge, this would have to be explored - individually for each framework. I just lack experience with Vue etc so I can't say with any certainty how what I'm saying applies to those frameworks.
It might feel a little bit unnatural but I think it's the reasonable choice for React. Note that there is never anything stopping people from normalizing "errors" into states~ and just having "error data" available as the first item returned from
useActor.
A big problem with returning rejection/error values anyhow from any hook is that... errors are basically untyped in TS, so this doesn't provide any actual type safety and is unsound by definition. Guiding people towards normalizing their errors into "states" is creating a pit of success - they have to type this and normalize their error data into that expected shape. It's clear where and how one should do it - just add
.catch(() => ...)and do it there, whatever you return from there has to match the expected type. It IMHO feels nice and is what I would, as a user, prefer.
And unhandled errors would just go to an error boundary where it's expected that the received error value is any/unknown and that it has to be refined there.
This is also similar to what React Query does - however, it's currently opt-in and called as experimental: https://react-query.tanstack.com/guides/suspense
| observers.forEach((observer) => { | ||
| observer.error(event.error); | ||
| }); |
There was a problem hiding this comment.
it seems a little bit quirky that observers are called here in the error case while they are called from within flush for the next item case
I'm not sure how to change this atm though
This PR allows you to spawn and/or invoke behaviors, which is the common interface for XState actors. Especially in v5, behaviors are not a new/special thing; promises/observables/callbacks/etc. in v5 all have specific behaviors that have the same interface, and in v5, only behaviors are spawned/invoked. This is a step towards that migration.