Tweak logic around stopping actors #3737

Andarist · 2023-01-02T14:14:21Z

No description provided.

changeset-bot · 2023-01-02T14:14:25Z

⚠️ No Changeset found

Latest commit: 9f6e95a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, 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

codesandbox-ci · 2023-01-02T14:14:58Z

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 9f6e95a:

Sandbox	Source
XState Example Template	Configuration
XState React Template	Configuration

ghost · 2023-01-02T14:15:26Z

👇 Click on the image for a new way to code review

Make big changes easier — review code in small groups of related files
Know where to start — see the whole change at a glance
Take a code tour — explore the change with an interactive tour
Make comments and review — all fully sync’ed with github

Try it now!

Legend

Andarist · 2023-01-02T14:17:30Z

packages/core/src/stateUtils.ts

@@ -1665,7 +1665,7 @@ function stopStep(
  for (const stateNode of nextState.configuration.sort(
    (a, b) => b.order - a.order
  )) {
-    actions.push(...stateNode.definition.exit);


This was quite a 🦋 effect.

definition is a getter:

xstate/packages/core/src/StateNode.ts

Line 211 in 68b24d7

public get definition(): StateNodeDefinition<TContext, TEvent> {

each StateNode was reading there context of the machine:

xstate/packages/core/src/StateNode.ts

Line 216 in 68b24d7

context: this.machine.context,

StateMachine['context'] is a getter as well:

xstate/packages/core/src/StateMachine.ts

Lines 90 to 92 in 68b24d7

public get context(): TContext {

return this.getContextAndActions()[0];

}

This means that we were calling "lazy context" functions like crazy~

Andarist · 2023-01-02T14:18:14Z

packages/core/src/interpreter.ts

-    if (isStateLike(this._state)) {
-      Object.values((this._state as AnyState).children).forEach((child) =>
-        child.stop?.()
-      );
-    }


This is already resolved through regular stop actions coming from the stopStep

Andarist · 2023-01-02T14:20:38Z

packages/core/src/actions/invoke.ts

+          if (actorRef.status === ActorStatus.Stopped) {
+            return;
+          }


IMHO it's much better to use the actorRef from the outer scope here instead of looking this up in the state. I don't currently have a failing test case for the old code (and maybe there is none to be created) but, on principle, it's way more error-prone to look this up by the key/id.

The intention of this function is to start this concrete actorRef and not an actorRef that happens to have the same id at the time this deferred function gets finally called.

I was looking it up on the state before to "emulate" invokeStates from statesToInvoke in the SCXML algorithm, but this is cleaner.

davidkpiano

Nice!

* Test cleanup * Committing this just to checkpoint it * Add status, change Actorcontext.exec sig * Actions resolve to tuple * Declassify ExeuctableAction * Goodbye ExecutableAction * Goodbye exec.ts * Refactor createDynamicAction * More refactoring * Yay more deleting * More refactoring * exectute2 -> execute * Enable root initial actions (addresses #466) * Working through types * Still working on types * Sorry, getting a little messy again * Revert "Sorry, getting a little messy again" This reverts commit a609de8. * Fix test * Ensure no side-effects in getInitialState * I guess this makes sense * Started state should come from behavior.start if it exists * Get rid of state-specific stuff * Remove test * Update packages/core/test/predictableExec.test.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Refactor test * Update packages/core/test/actions.test.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Update packages/core/test/actions.test.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Update packages/core/test/predictableExec.test.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Remove unused import * Update packages/core/test/actions.test.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Update packages/core/test/actions.test.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Interpreter tests * Remove unused `ActorContext['exec']` (#3734) * Fixed initial state cache eviction in the `Interpreter` (#3733) * Tweak logic around stopping actors (#3737) * Tweak logic around stopping actors * remove unused imports * Reuse the `ActorStatus` enum in the `handleAction` * Tweak types * Free the stopped `sessionId` in the registry * Make `getInitialState` in the `Interpreter` private (#3738) * Update packages/core/test/interpreter.test.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Update packages/core/src/stateUtils.ts * Small cleanup of internal implementations of actors (#3740) * Small cleanup of internal implementations of actors * Remove redundant completion notifications sent out by some actors * Wrap sending `xstate.snapshot.${id}` events with `defer` * Drop the unused "next" case from the event observable Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>

* WIP * WIP types * Fix sendParent weird issue * service.state -> service.getSnapshot() * Move symbolObservable * Skip sync, change default sessionId to undefined, add actorCtx to StateMachine * Move logic to state * Make stopping more general * Add getInitialState to behavior type * Fixing types WIP * Use strings instead of symbols * Cache initial state * Delete ObservableActorRef * Add getActions to Behavior * Move state-specific stuff outside of interpreter WIP * Make defer public (temporarily) * Fix types * Fix types * Skip failing tests for now * Move stopping logic * Add internal state to behaviors * Fixing tests WIP * Fix a test * Big refactor; only 2 tests left to fix! * Almost there... * Skipping tests * Fix types * Cleanup * Cleanup * Unskip a couple tests * Update packages/core/src/interpreter.ts * Simplify interpreter actorContext * Move machine-specific code to exec.ts * Cleanup * Add test from #3549 * Add failing test * interpreter.machine -> interpreter.behavior, fix tests caused by invalid cached initial state * Lint * Remove fromMachine * Re-enable predictableExec test * Fix memory leak * Inline stopStep * Extract machineMicrostep (bikeshed) * Make resolveStateValue part of machine; machine.microstep state must be State instance * Add get state.meta, get state.tags, start to remove State.create in favor of machine.createState * WIP simplifying stateUtils (only need historyValue, not state for some functions) and getting rid of State.from * WIP Make state.machine required * Remove machine argument from macroStep * More redundant machine removal * More redundant machine removal * More cleanup * Remove matchState * Compute value from configuration * Refactoring WIP * Add state.clone * Simplify microstep * Fix use of null * More cleanup * Linting * Fix linting in xstate/inspect * More linting * Remove interpreter.listeners * Sigh, the comment is right * Move more functions internal * Move exitStates * Make selectEventlessTransitions internal * Add AnyTransitionDefinition and AnyHistoryValue * Scope actions * Fix history * Use Array.flatMap and .flat * Optimization: use for..of instead of .forEach * Cleanup types * Restore state.meta in JSON * Remove unnecessary generic types * Cleanup types * Fix use of observers * Remove unused `empty` from a test file * Update packages/core/src/StateNode.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Update packages/core/src/actions/send.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Update packages/core/src/actions/send.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Fixed `AnyBehavior` to actually allow any `Behavior` (#3579) * Resolve action objects as lazily as possible to avoid stale closure problem in React (#3581) * Remove `Interpreter#nextState` (#3580) * Cleanup redundant code related to `execAction` * Remove `Interpreter#nextState` * Removed unused variables * Fixed looking up inlined action in the implementations objects (#3582) * Fixed looking up inlined action in the implementations objects * Fixed test titles Co-authored-by: David Khourshid <davidkpiano@gmail.com> * Add changeset Co-authored-by: David Khourshid <davidkpiano@gmail.com> * Stop tracking `preservedContexts`, only the last one is relevant anyway * Dont wrap non-resolvable action objects in `ExecutableAction` * Remove uncalled code * Improve error message * Don't use `null` as initial event * Lint * Resolve event earlier * Rename machine.key -> machine.id * Add TODO for explicit type for `output` * Machine is required; fix up check in output * Fix this.actions on State * Make exec required * Revert ActorRefFrom typing * Update packages/xstate-vue/test/UseActor.vue Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Use `ActorRefFrom` over `InterpreterFrom` in a couple of places * Drop redundant casts to `StateFrom` * Update packages/core/test/types.test.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Remove `state.inert` * Remove State.create * Remove outdated comment * Add changeset for removal of `matchState(...)` * Remove unnecessary cast * Remove unused `StateFrom` import * Remove traces of the `StateNodeConfig["key"]` * Add ActorRef.id, remove ActorRef.name * Update packages/core/src/interpreter.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Fix imports * Update packages/core/src/interpreter.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Update packages/core/src/interpreter.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Update packages/core/src/interpreter.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Update packages/core/src/stateUtils.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Update packages/core/src/interpreter.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Reflect the type for internal `Interpreter#observers` closer to the possible runtime values (#3593) * Simplify `transitionParallelNode` a little bit (#3591) * Simplify `transitionParallelNode` a little bit * Rename variables * Handle events sent to `self` closer to events send to other targets (#3592) * Refactor deferred * Fix execution issue * Cleanup * Cleanup * Cleanup * Remove Interpreter.doneListeners and Interpreter.stopListeners * Add getStatus to behavior to detect when done * Remove Interpreter.onStop * Use getStatus to remove state-specific done checking * Remove machine check from `interpret` fn * Remove errorListeners and .onError() * Remove state check from forwarding * Add restoreState * Move error throwing to state machine behavior * Lint * .initialState -> .getInitialState(...) * Remove `strict` * Refactoring to avoid mutation part 1 * macrostep now returns multiple states * Refactoring microstep cont'd * Clean up microstep tests * Add test * Add preserved actions test * .initialState -> .getInitialState() * Fix types * Cleanup * Cleanup * Fix: memo eviction * Add behaviors.test.ts * Move tests * Cleanup * Fix tests * Fix tests * Deprecate toActorRef * Remove `Interpreter#onError` (#3691) * Update .changeset/green-brooms-laugh.md Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Externalize functions * Remove import * Update packages/core/src/actions/send.ts * Adjust configuration types * Provide precomputed `State["done"]` and `State["output"]` (#3710) * Update packages/core/src/StateMachine.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Address getInitialState comment * Actor observer impl refactor WIP * Cleanup observable handling in fromObservable * Finish observer handling * Update packages/core/src/StateMachine.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Make cloneState a "private" function * Update packages/xstate-vue/test/UseActorSimple.vue Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Update packages/xstate-vue/test/UseActorSimple.vue Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Update packages/xstate-inspect/src/server.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Update packages/xstate-inspect/src/browser.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Revert temporary changes in a test file * Allow behaviors and behaviorCreators in provided actors object * Do todos * bring back remived comments in a test file * Simplify some tests * inline `infinite$` observable in the remaining call sites * rename a test block * add a changeset about `off` removal * Localice `createMachine` per test in one of the test files * Remove `spawn`+`autoForward` tests (#3718) * Remove `sync` option (#3717) * Remove `sync` option * remove more leftovers * remove another leftover * remove another leftover * Enable one of the skipped tests * Update packages/core/src/utils.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Remove `ActorContext['observers']` (#3722) * Update packages/core/src/stateUtils.ts * reuse existing local variable * Update packages/core/src/stateUtils.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Update packages/core/src/stateUtils.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Avoid double traversal when adding ancestors in `getConfiguration` * Update packages/core/src/stateUtils.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Remove underscore convention * Refactor * Fix type comment * Unused type * Initial state shouldn't be memoized (TODO: fix React tests) * Attempt to fix some React tests * Cleanup * Defer stopping actors (#3719) * Remove a custom `done` callback from the `Observer` (#3720) * Remove a custom `done` callback from the `Observer` * Spike removing the leftover dependencies on done listeners * [v5] Simplify `interpreter.send(...)` method (#3568) * Simplify function signature of `interpreter.send(eventObj)` * Remove unused types * Add changeset * Cleanup types * Simplify more event-related methods and functions (#3570) * Add changeset for `@xstate/fsm` * Fix tests Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * StateMachine implements Behavior * Rename defer -> delaySend * [v5] Refactor actions (#3726) * Test cleanup * Committing this just to checkpoint it * Add status, change Actorcontext.exec sig * Actions resolve to tuple * Declassify ExeuctableAction * Goodbye ExecutableAction * Goodbye exec.ts * Refactor createDynamicAction * More refactoring * Yay more deleting * More refactoring * exectute2 -> execute * Enable root initial actions (addresses #466) * Working through types * Still working on types * Sorry, getting a little messy again * Revert "Sorry, getting a little messy again" This reverts commit a609de8. * Fix test * Ensure no side-effects in getInitialState * I guess this makes sense * Started state should come from behavior.start if it exists * Get rid of state-specific stuff * Remove test * Update packages/core/test/predictableExec.test.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Refactor test * Update packages/core/test/actions.test.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Update packages/core/test/actions.test.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Update packages/core/test/predictableExec.test.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Remove unused import * Update packages/core/test/actions.test.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Update packages/core/test/actions.test.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Interpreter tests * Remove unused `ActorContext['exec']` (#3734) * Fixed initial state cache eviction in the `Interpreter` (#3733) * Tweak logic around stopping actors (#3737) * Tweak logic around stopping actors * remove unused imports * Reuse the `ActorStatus` enum in the `handleAction` * Tweak types * Free the stopped `sessionId` in the registry * Make `getInitialState` in the `Interpreter` private (#3738) * Update packages/core/test/interpreter.test.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Update packages/core/src/stateUtils.ts * Small cleanup of internal implementations of actors (#3740) * Small cleanup of internal implementations of actors * Remove redundant completion notifications sent out by some actors * Wrap sending `xstate.snapshot.${id}` events with `defer` * Drop the unused "next" case from the event observable Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Add a test for unique promises created by the same behavior Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>

Tweak logic around stopping actors

0292778

Andarist requested a review from davidkpiano January 2, 2023 14:14

Andarist commented Jan 2, 2023

View reviewed changes

remove unused imports

9f6e95a

davidkpiano approved these changes Jan 2, 2023

View reviewed changes

Andarist merged commit f74338a into v5/refactor-actions Jan 2, 2023

Andarist deleted the andarist/stop-invoke-tweak branch January 2, 2023 15:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tweak logic around stopping actors #3737

Tweak logic around stopping actors #3737

Andarist commented Jan 2, 2023

changeset-bot bot commented Jan 2, 2023 •

edited

codesandbox-ci bot commented Jan 2, 2023 •

edited

ghost commented Jan 2, 2023 •

edited by ghost

Andarist Jan 2, 2023

Andarist Jan 2, 2023

Andarist Jan 2, 2023

davidkpiano Jan 2, 2023

davidkpiano left a comment

	public get context(): TContext {
	return this.getContextAndActions()[0];
	}

Tweak logic around stopping actors #3737

Tweak logic around stopping actors #3737

Conversation

Andarist commented Jan 2, 2023

changeset-bot bot commented Jan 2, 2023 • edited

⚠️ No Changeset found

codesandbox-ci bot commented Jan 2, 2023 • edited

ghost commented Jan 2, 2023 • edited by ghost

Legend

Andarist Jan 2, 2023

Choose a reason for hiding this comment

Andarist Jan 2, 2023

Choose a reason for hiding this comment

Andarist Jan 2, 2023

Choose a reason for hiding this comment

davidkpiano Jan 2, 2023

Choose a reason for hiding this comment

davidkpiano left a comment

Choose a reason for hiding this comment

changeset-bot bot commented Jan 2, 2023 •

edited

codesandbox-ci bot commented Jan 2, 2023 •

edited

ghost commented Jan 2, 2023 •

edited by ghost