[v5] Interpret all behaviors #3455

davidkpiano · 2022-07-13T06:42:45Z

The biggest changes:

~~ObservableActorRef~~ is removed
interpret now works with any Behavior: interpret(behavior)
The Behavior interface now has Behavior.getSnapshot, which is important for determining which "state" should be exposed, and which state is internal state
predictableActionArguments flag is removed; actorCtx.exec is used instead
Most state-machine-specific code is removed from the Interpreter; still have some more work to do here

changeset-bot · 2022-07-13T06:42:48Z

🦋 Changeset detected

Latest commit: fc3ca72

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
xstate	Major
@xstate/fsm	Major
@xstate/react	Major
@xstate/vue	Major

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

codesandbox-ci · 2022-07-13T06:43:17Z

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

Sandbox	Source
XState Example Template	Configuration
XState React Template	Configuration

ghost · 2022-07-13T06:43:48Z

👇 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

…teMachine

davidkpiano · 2022-08-07T23:00:33Z

~~@Andarist There are 2 tests in @xstate/react only failing; can you help me investigate those?~~ EDIT: resolved

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>

packages/core/src/stateUtils.ts

Andarist · 2022-12-20T22:10:34Z

packages/core/src/StateMachine.ts

+  public getStatus(state: State<TContext, TEvent, TResolvedTypesMeta>) {
+    return state.done
+      ? { status: 'done', data: state.output }
+      : { status: 'active' };
+  }


We probably need an extra error status for cases when something throws an error and we "forcefully" stop the machine (and notify subscribers about the error)

packages/core/src/interpreter.ts

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>

Andarist · 2022-12-21T11:11:45Z

packages/core/src/actors.ts

@@ -2,30 +2,18 @@ import type {
  InvokeCallback,


All of this is in the actors.ts but we refer to them as behaviors. It feels like we should do something about it because it feels like we are getting lost ourselves in the used nomenclature

I'll do this in a separate PR (want to rename to behaviors.ts because it's a noisy change

Andarist · 2022-12-21T11:28:57Z

I feel that it's crucial for this PR to make StateMachine implement the Behavior interface. As long as it doesn't implement it, it feel like interpret isn't actually accepting Behavior - it's accepting Behavior | StateMachine, and thus the unification process of all the things didn't actually succeed. I have a comment that goes into this a little bit further here
with interpret accepting any Behavior we end up with some problems in @xstate/react etc. Its useInterpret has some very machine-oriented options (and it's still accepting TMachine and not TBehavior). That probably can be handled in a separate PR but it's worth noting here.
There is one minor unanswered comment here
Behavior['getInitialState'] is calling factories embedded in behaviors and those are usually side-effectful. This is a big problem because those functions should not be called before Interpreter['start']. I think that this code should be moved to Behavior['start'] (which is a method declared in the type but in practice it's not implemented by anything) and that Behavior['getInitialState'] has to be made pure. Accompanying tests should be introduced to make sure that we don't call side-effects within getInitialState (or within interpret(behavior).getSnapshot() as that's a more public-facing API and it's something that might lead to calling behavior.getInitialState). I've mentioned this here
Interpreter has two different defer methods (yes, one is on the ActorContext but that doesn't quite make it less confusing especially when both are used within some exec-related code), the first one and the second one. I think that it's best to rename one of them and that possibly it's easier to rename the second one.
I think that observer.done has been shoehorned on the existing concept that doesn't support this. If we have to do that, it raises the question if we are using this concept correctly. I think that it would be best to avoid such custom extensions if we plan to use to the concepts of observers, observables etc. No observable-specific code will actually be able to utilize this custom extension in a generic way and it's a bad thing. One of the main places in which Observer['done'] is being used is when the child actor gets started - we then subscribe to it (in the parent) and listen for its done callback to send the done.invoke.* even to ourselves. This is conceptually a little bit weird because the responsibility to send that event has been moved to the parent whereas the more straightforward way of doing that would be to send that even directly from the child to the parent. We don't need to use a separate "channel" for communication like this as we already have our own, standard, channel that we can use for communication. I managed to remove this in this PR. The only downside there is that onDone's implementation is a little bit funky - but that's perhaps easily solvable by just changing the callback's argument: it could be TOutput (once we add it) instead of DoneEvent. This would match what you have noted as a desired semantic for done here

* Remove a custom `done` callback from the `Observer` * Spike removing the leftover dependencies on done listeners

…lyai/xstate into v5/interpret-all-behaviors

davidkpiano · 2022-12-28T04:54:11Z

@Andarist

~~Agree re: making StateMachine implement a Behavior interface. Will look into this soon.~~ Done in 39104d6
Discussed in Discord; we'll handle this in a separate PR with the goal of useMachine -> useBehavior and useInterpret working with any Behavior.
See reply - unsure what to test there.
This is resolved and all tests passing in [v5] Refactor actions #3726 ✅ The last thing to fix there is invoke(...) types.
~~Will rename one of the defers, and eventually figure out a better way to handle "deferred" actions (not really deferred, just moved to the end of the queue)~~ Renamed in 951847a
Your PR Remove a custom done callback from the Observer #3720 has been merged for this 👍

We're getting close!

* 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>

* 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>

davidkpiano added 5 commits July 10, 2022 18:10

WIP

a8a0ee6

WIP types

5335728

Fix sendParent weird issue

ba70a20

service.state -> service.getSnapshot()

3eac678

Move symbolObservable

5665cb3

davidkpiano added 13 commits July 16, 2022 20:15

Skip sync, change default sessionId to undefined, add actorCtx to Sta…

438b528

…teMachine

Move logic to state

c41e93d

Make stopping more general

f321464

Add getInitialState to behavior type

7f24a51

Fixing types WIP

63b1a77

Use strings instead of symbols

f42c0a3

Cache initial state

a9a8270

Delete ObservableActorRef

2cadb0f

Add getActions to Behavior

95950f9

Move state-specific stuff outside of interpreter WIP

1f36caf

Make defer public (temporarily)

7ce18ed

Fix types

0c51d58

Fix types

5cdba60

davidkpiano added 8 commits August 12, 2022 14:50

Merge branch 'next' into v5/interpret-all-behaviors

cf63723

Skip failing tests for now

dee6651

Move stopping logic

f27d374

Add internal state to behaviors

0422241

Fixing tests WIP

6ab24c2

Fix a test

b79e6df

Big refactor; only 2 tests left to fix!

515544b

Almost there...

6d4485f

Update packages/core/src/stateUtils.ts

29bf0da

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>

Andarist reviewed Dec 20, 2022

View reviewed changes

packages/core/src/stateUtils.ts Show resolved Hide resolved

Avoid double traversal when adding ancestors in getConfiguration

d26b760

Andarist reviewed Dec 20, 2022

View reviewed changes

packages/core/src/stateUtils.ts Outdated Show resolved Hide resolved

Andarist reviewed Dec 20, 2022

View reviewed changes

packages/core/src/interpreter.ts Outdated Show resolved Hide resolved

davidkpiano and others added 5 commits December 20, 2022 22:01

Update packages/core/src/stateUtils.ts

6bcf0f6

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>

Remove underscore convention

2d62a02

Refactor

bbcbafd

Fix type comment

10fb4a2

Unused type

d9624de

Andarist reviewed Dec 21, 2022

View reviewed changes

davidkpiano and others added 6 commits December 21, 2022 09:27

Initial state shouldn't be memoized (TODO: fix React tests)

c0d7f39

Attempt to fix some React tests

3609870

Cleanup

816c8d8

Defer stopping actors (#3719)

ae19142

Remove a custom done callback from the Observer (#3720)

b28c34b

* Remove a custom `done` callback from the `Observer` * Spike removing the leftover dependencies on done listeners

Merge branch 'v5/interpret-all-behaviors' of https://github.com/state…

3675b79

…lyai/xstate into v5/interpret-all-behaviors

davidkpiano and others added 6 commits January 2, 2023 08:41

StateMachine implements Behavior

39104d6

Rename defer -> delaySend

951847a

Add a test for unique promises created by the same behavior

8234a8c

Merge branch 'next' into v5/interpret-all-behaviors

fc3ca72

davidkpiano merged commit ec39214 into next Jan 3, 2023

davidkpiano deleted the v5/interpret-all-behaviors branch January 3, 2023 00:50

github-actions bot mentioned this pull request Jan 3, 2023

Version Packages (alpha) #3375

Merged

davidkpiano mentioned this pull request Feb 13, 2023

interpret, useInterpret etc should return an Actor #3192

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[v5] Interpret all behaviors #3455

[v5] Interpret all behaviors #3455

davidkpiano commented Jul 13, 2022 •

edited

Loading

changeset-bot bot commented Jul 13, 2022 •

edited

Loading

codesandbox-ci bot commented Jul 13, 2022 •

edited

Loading

ghost commented Jul 13, 2022 •

edited by ghost

Loading

davidkpiano commented Aug 7, 2022 •

edited

Loading

Andarist Dec 20, 2022

Andarist Dec 21, 2022

davidkpiano Dec 21, 2022

Andarist commented Dec 21, 2022

davidkpiano commented Dec 28, 2022 •

edited

Loading

[v5] Interpret all behaviors #3455

[v5] Interpret all behaviors #3455

Conversation

davidkpiano commented Jul 13, 2022 • edited Loading

changeset-bot bot commented Jul 13, 2022 • edited Loading

🦋 Changeset detected

codesandbox-ci bot commented Jul 13, 2022 • edited Loading

ghost commented Jul 13, 2022 • edited by ghost Loading

Legend

davidkpiano commented Aug 7, 2022 • edited Loading

Andarist Dec 20, 2022

Choose a reason for hiding this comment

Andarist Dec 21, 2022

Choose a reason for hiding this comment

davidkpiano Dec 21, 2022

Choose a reason for hiding this comment

Andarist commented Dec 21, 2022

davidkpiano commented Dec 28, 2022 • edited Loading

davidkpiano commented Jul 13, 2022 •

edited

Loading

changeset-bot bot commented Jul 13, 2022 •

edited

Loading

codesandbox-ci bot commented Jul 13, 2022 •

edited

Loading

ghost commented Jul 13, 2022 •

edited by ghost

Loading

davidkpiano commented Aug 7, 2022 •

edited

Loading

davidkpiano commented Dec 28, 2022 •

edited

Loading