Do not crash when stopping already stopped interpreter. #1710
Conversation
🦋 Changeset detectedLatest commit: 572a711 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
| @@ -503,6 +503,11 @@ export class Interpreter< | |||
| this.doneListeners.delete(listener); | |||
| } | |||
|
|
|||
| if (!this.initialized) { | |||
| // Interpreter already stopped; do nothing | |||
| return this; | |||
There was a problem hiding this comment.
Shouldn't we throw an error that just would lead users to fix their mistake? isn't stopping twice breaking an invariant rule of XState? I see arguments both ways (throwing vs making this noop) and I'm somewhat undecided what's better myself - just raising this as a general question.
There was a problem hiding this comment.
There can exist use-cases where the not-started interpreter is stopped by no fault of the developer; throwing would be counter-productive and unexpected.
Contrived example: in React, a component that is rendered but immediately unmounted (for some reason) would need to .stop() all services, even if they haven't started yet.
I think the best thing to do is to no-op, since the intention is to stop the service, and if the service is already stopped, mission accomplished 👍
There was a problem hiding this comment.
Contrived example: in React, a component that is rendered but immediately unmounted (for some reason) would need to .stop() all services, even if they haven't started yet.
This actually can't happen - if we don't reach the commit phase we won't start anything and unmounting just wont happen so stop won't be called either.
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
Fixes #1697