xstate icon indicating copy to clipboard operation
xstate copied to clipboard

Document stopping actors.

Open nathanhammond opened this issue 5 years ago • 8 comments

This PR includes two pieces of information that @Andarist explained to me in issue #1403, actions.pure() and actions.stop().

However, after attempting to document (and reading the code in interpreter.ts for 4.X), I have additional questions that imply to me that there may be additional unaddressed issues here.

  • Just sending xstate.stop to a StateNode does not seem to be complete to me as it doesn't seem like it is recursive. The receiver of xstate.stop may also be a machine with children which would also need to be stopped. The event does not appear to be propagated. Code.
  • It seems like you should still call .stop() on the interpreter as well. Code.

Assuming that my hypotheses are correct, maybe Interpreter#stop should also send (from leaf to root?) xstate.stop?

nathanhammond avatar Sep 03 '20 08:09 nathanhammond

💥 No Changeset

Latest commit: ce786ddf5ba8da08f1a4a7d78abb03d586cb8635

Merging this PR will not cause any packages to be released. If these changes should not cause updates to packages in this repo, this is fine 🙂

If these changes should be published to npm, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

changeset-bot[bot] avatar Sep 03 '20 08:09 changeset-bot[bot]

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 ce786ddf5ba8da08f1a4a7d78abb03d586cb8635:

Sandbox Source
XState Example Template Configuration
XState React Template Configuration

codesandbox-ci[bot] avatar Sep 03 '20 08:09 codesandbox-ci[bot]

Oh, I've missed your questions in the PR comment itself. Let's get to them:

Just sending xstate.stop to a StateNode does not seem to be complete to me as it doesn't seem like it is recursive. The receiver of xstate.stop may also be a machine with children which would also need to be stopped. The event does not appear to be propagated. Code.

StateNode is a "pure" representation of a machine, it's stateless but the information in it holds a list (or a map, don't remember) of things like activities and children`, those shouldn't be "live" things though.

OTOH the Interpreter is this "live" thing that manages the pure machine that it references. It receives actions that should be executed (apart from assigns, those are completely evaluated & resolved within StateNode). In here it stops a particular child upon receiving a stop action: https://github.com/davidkpiano/xstate/blob/78e998674b8ac8cbcf926f43b2588787b5631953/packages/core/src/interpreter.ts#L885-L886 and stop should recursively stop all "nested" machines as well: https://github.com/davidkpiano/xstate/blob/78e998674b8ac8cbcf926f43b2588787b5631953/packages/core/src/interpreter.ts#L492-L497

This should also answer the second question as those were highly interconnected.

Andarist avatar Sep 03 '20 10:09 Andarist

How do y'all prefer your modifications to PRs? A. New commits. B. Update-in-place (continuously squash to a single commit). C. Other

And prepping for landing: A. Squash B. Commit History C. Other

And landing strategy: A. Merge B. Rebase C. Other

And test strategy: A. Failing test in a separate commit first. (red => green) B. Tests pass at every commit. C. Other

(Can also drop this into CONTRIBUTING.md if you'd like.)

nathanhammond avatar Sep 03 '20 11:09 nathanhammond

Sorry for the delay - was quite busy lately with other stuff. I haven't forgotten and will get back to you this week.

Andarist avatar Sep 17 '20 15:09 Andarist

How do y'all prefer your modifications to PRs?

IMHO - whatever works best for you. Ideally, we'd like to receive often contributions from the community, and requiring a particular git-flow makes it harder to achieve that, not everyone is well-versed with more advanced git techniques etc. As to the landing strategy - this one can actually be quite easily chosen from the GitHub UI so it's not much of a concern either way.

Andarist avatar Sep 20 '20 20:09 Andarist

@nathanhammond Sorry for the delay in reviewing this! @Andarist and I discussed the need for a dynamic stop(ctx => ctx.child) and how it could be useful (and simpler) with regard to this, so I'll make those changes and then re-review this PR with the necessary changes.

Thanks so much for doing this!

davidkpiano avatar Oct 19 '20 18:10 davidkpiano

Now that #1577 is in, we can simplify these docs:


Stopping Spawned Machines

To stop a spawned machine, use the actions.stop(...) action creator and reference the spawned machine as an expression in the first argument:

on: {
  STOP_SPAWNED: {
    actions: actions.stop(context => context.someSpawnedMachine)
  }
}

davidkpiano avatar Oct 24 '20 14:10 davidkpiano

The docs have moved to https://github.com/statelyai/docs and we will be addressing suggestions & improvements there.

davidkpiano avatar Aug 29 '23 20:08 davidkpiano