Skip to content

Make Interpreter#nextState pure again#3340

Merged
Andarist merged 1 commit intov5/improved-spawn-1from
make-nextstate-pure-again
May 25, 2022
Merged

Make Interpreter#nextState pure again#3340
Andarist merged 1 commit intov5/improved-spawn-1from
make-nextstate-pure-again

Conversation

@Andarist
Copy link
Collaborator

@Andarist Andarist commented May 25, 2022

No description provided.

@Andarist Andarist requested a review from davidkpiano May 25, 2022 14:51
@changeset-bot
Copy link

changeset-bot bot commented May 25, 2022 •
edited
Loading

⚠️ No Changeset found

Latest commit: 5493e5c

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

@Andarist Andarist force-pushed the make-nextstate-pure-again branch from a5cd633 to 5493e5c Compare May 25, 2022 14:51
@codesandbox-ci
Copy link

codesandbox-ci bot commented May 25, 2022

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

Sandbox Source
XState Example Template Configuration
XState React Template Configuration

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 25, 2022

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 5493e5c:

Sandbox Source
XState Example Template Configuration
XState React Template Configuration

@ghost
Copy link

ghost commented May 25, 2022

👇 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!

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map Legend

Andarist
Andarist commented May 25, 2022
View reviewed changes
packages/core/src/interpreter.ts
// TODO: handle errors
this.forward(event);

let errored = false;
Copy link
Collaborator Author

@Andarist Andarist May 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this ain't really beautiful. It's just a quick juggle of code to satisfy current tests and make nextState pure (like it should be).

This closely follows the logic hidden in those operations:

xstate/packages/core/src/behaviors.ts

Lines 299 to 308 in dc3eba3

parent?.send(
toSCXMLEvent(doneInvoke(actorContext.name, response) as any, {
origin: actorContext.self
})
);
observers.forEach((observer) => {
observer.next?.(response);
observer.complete?.();
});

It's just a stopgap before cleaning up the whole story around error handling.

Copy link
Member

@davidkpiano davidkpiano May 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense 🙈 🧹

@Andarist Andarist mentioned this pull request May 25, 2022
Merged
@Andarist Andarist merged commit 359998d into v5/improved-spawn-1 May 25, 2022
@Andarist Andarist deleted the make-nextstate-pure-again branch May 25, 2022 20:27
Andarist added a commit that referenced this pull request May 26, 2022
@davidkpiano @Andarist
[v5] Explicit spawn (#3148)
7a68cbb 
* All tests pass!

* Fix all tests

* Some types cleanup

* Fix types WIP

* Add TODOs for types

* Skip questionable test

* Remove stray console logs

* Add `spawn.machine`

* Add `spawn.promise`

* Add `spawn.observable`

* Add tests for referenced behaviors

* Revert "Add `spawn.observable`"

This reverts commit 6df48b5.

* Revert "Add `spawn.promise`"

This reverts commit 9fce5a9.

* Revert "Add `spawn.machine`"

This reverts commit 0a3ac9f.

* Cleanup

* Temp fix for string references

* Ignore excessive stack depth warnings

* This actually passes

* Remove invoke*

* Rename createDeferredBehavior -> createCallbackBehavior

* Make getContextAndActions private

* Fix & clarify unskipped test

* Add test from todo

* Clean up test

* Make parent "private"

* Make sure children is "immutable" (+ todos)

* Revert initialState change

* Add comment

* Oops

* Remove invoke* functions (except for invokeMachine)

* Quell 1 type error

* Fixed the conditional type in the `interpret`

* Bring back removed `spawn` type test

* Tweak one `assign` test

* Remove lazy argument from `createMachineBehavior` and remove `invokeMachine` entirely

* Add a TODO comment about hiding `ActorRef["_parent"]`

* Require a machine with all implementations provided as an argument for `createMachineBehavior`

* Cache the `initialState` getter in `createMachineBehavior`

* Unwrap `createCallbackBehavior` argument from a redundant lazy layer

* Renaming WIP

* Renaming WIP

* Add back spawnBehavior but call it createActorRef

* behaviors -> actors

* Tweak tests

* Remove Actor class

* Remove actor.ts

* Fix missing function

* Remove createBehaviorFrom

* Make behavior private

* Add onEmit and remove complete

* Tighten up types (a little)

* Fix types

* Remove createBehaviorFrom

* No more magic events

* Update packages/core/src/actors.ts

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

* Update packages/core/src/types.ts

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

* Fix types

* Fix sendError (send raw error data, not events)

* Attempt to fix type

* Fix type

* Rename onEmit -> onSnapshot

* Rename types: TEmitted -> TSnapshot

* Change src to allow behaviors directly (WIP)

* Fix type errors

* Add back InvokeSrcDefinition for now

* Add fromEventObservable

* Remove outdated `xstate/behaviors` references (#3280)

* Avoid wrapping `behaviorImpl` in a throwaway function (#3281)

* Update packages/core/test/actions.test.ts

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

* Update packages/xstate-svelte/src/useSelector.ts

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

* Update packages/xstate-svelte/src/useSelector.ts

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

* Update packages/xstate-vue/src/useSelector.ts

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

* Update packages/xstate-vue/src/useSelector.ts

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

* Remove unused type

* Remove subscriptions from behaviors (they belong on ObservableActorRef only)

* Add TODO

* Remove unused type

* Refactor: do not use observers for invoked promises

* Remove latestData logic in interpreter

* Remove observer logic from fromObservable/fromObservableEvent

* Remove observers

* Remove subscription logic from interpreter

* Remove unused type

* Fixed `ObservableActorRef` subscriptions and remove some outdated/redundant code (#3319)

* Fixed `ObservableActorRef` subscriptions and remove some outdated/redundant code

* Add changesets

* Remove `createActorRef`

* Make `Interpreter#nextState` pure again (#3340)

* Rename `EmittedFrom` to `SnapshotFrom` (#3341)

* Add changesets

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidkpiano davidkpiano davidkpiano approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Andarist @davidkpiano