Xstate: Parent is not sent on spawned machines

Created on 22 May 2020  Β·  9Comments  Β·  Source: davidkpiano/xstate

Description
I'm unable to use sendParent from a spawned machine to send an event back to the machine that spawned it. Something that would be nice is if the sendParent action could automatically forward an event from the child as well rather than having to write out the event completely. (ie { on: { CREATE: sendParent() }} would just send the whole event automatically)

Expected Result
The spawned machine should be able to use sendParent to send events back to the parent machine.

Actual Result
Nothing happens. Adding service.onEvent(console.log) shows this message "Event "CREATE" was sent to stopped service "(machine)". This service has already reached its final state, and will not transition.
Event: {"type":"CREATE"}"

Reproduction
https://codesandbox.io/s/spawn-test-554os

bug

Most helpful comment

Well - I assume that when one develops an application in React they need to learn how it works. I agree though that even knowing React quite well this might not be that obvious when to wrap things with asEffect because when abstracting the behavior with machines you kinda disconnect yourself from React (mentally). When developing regular React components it's much easier to feel this stuff intuitively because you are closer to the bare bones and you directly see if you are executing something in event handlers or somewhere else whereas with XState you always just send events and things are handled in a separate place (the machine). I don't particularly feel that's a big issue though as machines used by useMachine are usually tightly coupled to the UI and they mostly receive and respond to events generated within DOM event handlers. Introducing asEffect is only helpful for a couple of cases - mainly when needing to work with host elements comparatively, like for example when you need to focus input and only when it doesn't exist yet because if it exists you can just call .focus() on it immediately. So to sum it up - I don't this shouldn't be needed that often and once you need it it should be quite obvious what's the problem at hand (that you are in a moment when things are not ready to be used yet) and thus the solution (using asEffect) should come naturally quite quickly. To some extent this might feel like "trial & fail" when you fail to recognize the need for this when modeling but I think those situations should be quite spare. It's, of course, hard to quantify this though right now - the future will tell us if this gets people confused or not.

All 9 comments

This is strange. When I try this today, the warning about stopped service is not happening. The CREATE event is sent to listMachine. I've added actions.log to listMachine and nothing is logged after clicking a button.

https://codesandbox.io/s/spawn-test-hufjx?file=/src/App.js

It seems that actions are not interpreted at all with this pattern.

Just want to let you know that I'm investigating this issue.

Duplicate of #1125

The issue got explained here: https://github.com/davidkpiano/xstate/issues/1125#issuecomment-613425210 . We plan to fix it soon - although in a slightly different manner than mentioned there. This PR is related to the planned fix: https://github.com/davidkpiano/xstate/pull/1202

@Andarist I am looking at the linked PR and if I am understanding it correctly, we will have to apply as(Layout)Effect to mitigate this problem? Is that a good approach?

I mean when I have actions declared in the config of the machine, it's nicely separated logic, framework agnostic. With that PR it would mean I will have to copy code of the "broken" action and wrap it into asEffect?

Especially with internal actions like sending events to the parent. Why should I expose that in the component code? Feels more like a patch than a proper solution :/

I don't mean to throw away the work in that PR, but wouldn't it be more beneficial to basically have a configuration option of useMachine to always wrap actions into the effect? Potentially list names of actions that should be included.

@davidkpiano What do you think?

The current RC version is scheduling all actions through React's effect callbacks. This is not necessarily good. If you are interested in my thoughts about this you can dive into this:


lengthy explanation πŸ˜…

I'd like to discuss how executing actions should be done when using @xstate/react. This seems like not an obvious decision, concerning how little actual experience we have with Concurrent Mode and any discussion about this so far has been based more on our thoughts, hypothesis, materials shared by React team and some discussions with them as well.
​
Problem is that code written in a more traditional way, with more traditional assumptions, often doesn't blend OK with what React is trying to enforce. The main difference is that sometimes executing side-effects is being considered unsafe because it might lead to them being executed more than once (and this is not something that we want). It's worth mentioning why this can happen - everything boils down to a fact that React can call render multiple times and thus any side-effects executed during render phase is being considered unsafe.
​
How one can deal with this problem? The answer is "simple" - schedule those side-effects to be executed during commit phase (inside React's effect callbacks). Does it mean that all side-effects should be scheduled through effects? Not really and React team member has confirmed that.
​
Let's recap the information gathered so far:

  1. Do we have to schedule all actions (our side effects) through effect callbacks? No
  2. Can we do it? Yes, it's our design decision that we can make
  3. Should we do it? It's uncertain, but I'm leaning towards - no.
    ​
    Why I don't believe this is a good thing for us to schedule all side-effects for commit phase
    ​
  4. There are valid cases when one would like to execute asap. I don't feel like the decision about this should be ours - it should lie in developers' hands.
  5. It's not explicit and it's a magical - so it stands against what we are trying to do: promote explicitness.
  6. As it is not explicit the behavior of useMachine differs between it and regular machines, forces people to think about 2 different scheduling mechanism. It's like learning 2 separate APIs (to some extent) and moving machines between React and non-React stops to be seamless. Yes, it might not require changes most of the time, but there might be cases when it might matter and by making React behavior different we lose the ease of spotting places where it does and which places should be reconsidered.
  7. It might cause some actions to get lost. Think about sending telemetry events just before machine unmounts - if we dont get to next render and thus next commit phase the action in question gets lost inadvertently, which might be easily missed and might not cover behavior requirements ideally.
  8. It leads to poorer performance for some cases as all actions needs to trigger rerender (and thus React need to make some rerendering work which could easily be avoided). A simple example of this would be focusing an existing input after click on a button - in this case there is no reason to postpone the execution of this focus action, it might even lead to deteriorated user experience. The expectancy for this scenario is for the input being focused immediately whereas if this gets scheduled for the next commit phase we don't know when it will execute. It probably should get fired "soon enough" but there is no guarantee of that
  9. It might even lead to runtime errors and crashing the component. Consider the same example from the previous point - if there is arbitrary work happening between click on a button and actual focus action being executed then there is no guarantee that the input in question still exists, it could have been removed in between (state could have changed for some reason leading to different render output).
    ​
    Is postponing side-effects till the commit phase a bad idea?
    ​
    Not entirely. There are valid cases when it is preferred to schedule an action for the commit phase, but I believe that this should be done through a dedicated API - which would be explicit (for example some extra scheduleForCommit action creator) and wouldn't as easily lead to the outlined pitfalls. More than that - we could try to introduce dev-only warnings when we detect misusage and actions being executed at bad times (which is only during rendering). This might not be possible for all cases, but should at least be doable for initial actions (which currently always happen during rendering).

What is actually happening in that linked PR is that we are reverting this behavior and we get back to firing actions as we go but we provide a specialized action creator which allows you to schedule a particular action to be executed during React's commit phase.

Oh that makes sense now. Thank you. So the problem of this issue is coming from the fact that effects are used, not that they wouldn't be used πŸ˜…

Still, it makes me wonder how is the developer going to decide which actions should be executed in the effect? Apparently, it requires some deep knowledge of React and not everyone is capable of absorbing that. From that PR I don't see much of explanation behind use cases. Is it going to be a "trial & fail" approach?

Well - I assume that when one develops an application in React they need to learn how it works. I agree though that even knowing React quite well this might not be that obvious when to wrap things with asEffect because when abstracting the behavior with machines you kinda disconnect yourself from React (mentally). When developing regular React components it's much easier to feel this stuff intuitively because you are closer to the bare bones and you directly see if you are executing something in event handlers or somewhere else whereas with XState you always just send events and things are handled in a separate place (the machine). I don't particularly feel that's a big issue though as machines used by useMachine are usually tightly coupled to the UI and they mostly receive and respond to events generated within DOM event handlers. Introducing asEffect is only helpful for a couple of cases - mainly when needing to work with host elements comparatively, like for example when you need to focus input and only when it doesn't exist yet because if it exists you can just call .focus() on it immediately. So to sum it up - I don't this shouldn't be needed that often and once you need it it should be quite obvious what's the problem at hand (that you are in a moment when things are not ready to be used yet) and thus the solution (using asEffect) should come naturally quite quickly. To some extent this might feel like "trial & fail" when you fail to recognize the need for this when modeling but I think those situations should be quite spare. It's, of course, hard to quantify this though right now - the future will tell us if this gets people confused or not.

Duplicate of #1125

The issue got explained here: #1125 (comment) . We plan to fix it soon - although in a slightly different manner than mentioned there. This PR is related to the planned fix: #1202

Just to be clear, the symptom here is opposite of #1125 but it appears to be due to the same cause? Just looking at that issue it seems like it’s the exact opposite situation (parent sending events to children) from this issue (children sending events to the parent).

Just to be clear, the symptom here is opposite of #1125 but it appears to be due to the same cause?

Yes - I can confirm that the root cause is the same. The symptoms are actually also the same, but it's slightly obscured by how the "test case" is presented in #1125. The problem is not that you can't send an event to the child, but only that child is not executing its actions. Take a look at this adjusted demo and console output after clicking the button twice:
https://codesandbox.io/s/xstate-react-reddit-example-with-actors-l0r4q?file=/src/machine.js

Was this page helpful?
0 / 5 - 0 ratings

Related issues

suku-h picture suku-h  Β·  3Comments

ifokeev picture ifokeev  Β·  3Comments

bradwoods picture bradwoods  Β·  3Comments

jfun picture jfun  Β·  3Comments

hnordt picture hnordt  Β·  3Comments