Further errors-related adjustments #4492
Conversation
🦋 Changeset detectedLatest commit: f63144b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit f63144b:
|
packages/core/src/interpreter.ts
Outdated
| case 'done': | ||
| // it's disputable if `next` observers should be notified about done snapsots |
There was a problem hiding this comment.
While somewhat disputable - I'm coming to the realization that maybe it's just better in our case. We don't necessarily have to maintain 1 to 1 semantics with RxJS and we can treat done snapshots as regular snapshots, just with a small addition of executing complete right after updating our internal state with one.
| // TODO: atm children don't belong entirely to the actor so | ||
| // in a way - it's not even super aware of them | ||
| // so we can't stop them from here but we really should! | ||
| // right now, they are being stopped within the machine's transition | ||
| // but that could throw and leave us with "orphaned" active actors |
There was a problem hiding this comment.
This is a fairly big concern. The core of the problem is outlined in this comment. Children's lifetime is somewhat related to the parent actor... but that parent actor alone doesn't really manage them. On top of that, children are exclusive to machines today.
To somewhat remedy that I could "duplicate" some of the try/catches within the StateMachine and update the snapshots and things there. We can't remove those try/catches from the Actor though. Other logic types might not be as nice and they might just throw.
Alternatively, we could make an assumption that actor logics can't really throw and that the error handling is their responsibility... that's kinda a big assumption though (even if we'd document this)
packages/core/src/stateUtils.ts
Outdated
| // TODO: we should likely only allow transitions selected by very explicit descriptors | ||
| // `*` shouldn't be matched, likely `xstate.error.*` shouldnt be either | ||
| // what about `xstate.error.actor.*`? what about `xstate.error.actor.todo.*`? |
There was a problem hiding this comment.
related to: #2935 (comment)
Andarist
changed the title
Co-authored-by: David Khourshid <[email protected]>
No description provided.