Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

onDone not triggered for parallel states #1111

Closed
renerbaffa opened this issue Apr 3, 2020 · 9 comments · Fixed by #1165
Closed

onDone not triggered for parallel states #1111

renerbaffa opened this issue Apr 3, 2020 · 9 comments · Fixed by #1165

Comments

@renerbaffa
Copy link

renerbaffa commented Apr 3, 2020

Description
I was studying parallel states and more specifically trying out the example in the documentation (https://xstate.js.org/docs/guides/final.html#parallel-states). The onDone function does not get fired when children states get into the final state.

This issue might be related to #335?

Expected Result
As the example in the documentation, onDone should be fired when the children get into the final state.

Actual Result
onDone is not fired when the children get into the final state.

Reproduction
https://codesandbox.io/s/nifty-ganguly-8bnei

Additional context
Latest version of xstate (4.8.0)

@renerbaffa renerbaffa added the bug label Apr 3, 2020
@davidkpiano
Copy link
Member

Try doing it in a child parallel state, rather than directly on the machine.

@renerbaffa
Copy link
Author

It works only for the first parallel child moving to a final state.
The second onDone is also not being fired:

https://codesandbox.io/s/infallible-haze-h73e2

You can change the order of events sent to see it.

In any case, if that onDone in the machine level does not work, how would you fill up data to pass context to the parent?

@renerbaffa
Copy link
Author

what would be the direction here?

@knobo
Copy link
Contributor

knobo commented Apr 23, 2020

I have the same problem.

@knobo
Copy link
Contributor

knobo commented Apr 23, 2020

Wrapping the whole thing in as a child state worked. But it's a bug anyway.

@davidkpiano
Copy link
Member

davidkpiano commented Apr 23, 2020

Looking into this now.

EDIT: This might be a trickier issue than thought. onDone gets triggered due to an event, but the problem is that since this is the root node, the machine has already terminated and stopped receiving events before the done event is sent.

Can you please tell me your use-cases @knobo @renerbaffa ? You can just use service.onDone(...).

@knobo
Copy link
Contributor

knobo commented Apr 27, 2020

My use-case was fetching all resources needed by a webpage before displaying it.
It works for me to do it as I did, by wrapping it as a child state. I don't have to have this fixed now, but it should be documented. Because it took me some time to figure this out.

@Andarist
Copy link
Member

@davidkpiano maybe we could also add a dev-only working when we find onDone defined on the root? As it can't work (because the machine is determined by then, like you have said) then we could just surface misusage to the user early.

@renerbaffa
Copy link
Author

My use-case is similar to @knobo. Fetching from different sources and moving on with the parent state afterwards. service.onDone(...) worked as pointed out.
I liked the idea of adding this use-case in the documentation. Would have saved me some time 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants