Add support for wildcard transitions to xstate-fsm #4065

benblank · 2023-06-08T22:44:40Z

After a recent discussion regarding how to handle unexpected events (#4057) brought wildcard transitions to my attention, I wanted to see what it would take to add them to the @xstate/fsm package. Fortunately, it turned out to be pretty straightforward.

This PR introduces three behavioral changes, in addition to the supporting tests and documentation:

If a machine is created with transitions whose event type is a single asterisk, those transitions are added to the list of transitions attempted regardless of event type. This has the effect that, if no other transition has occurred, the wildcard transitions are checked before giving up and returning an unchanged state.
To prevent the above from causing typing problems, a single asterisk is considered a valid key in the transitions objects in the machine config.
If the transition method is called with an event whose type is a single asterisk, it will throw in non-production mode. (This matches the behavior of the full XState library.)

(Also, while updating "docs/packages/xstate-fsm/index.md", I noticed that it had gotten out of sync with "packages/xstate-fsm/README.md"; I corrected that while I was in there, but as a separate commit, so that it can be easily dropped if you'd prefer to keep the PR clean.)

changeset-bot · 2023-06-08T22:44:43Z

🦋 Changeset detected

Latest commit: 73592a0

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

This PR includes changesets to release 1 package

Name	Type
@xstate/fsm	Minor

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 · 2023-06-08T22:46:02Z

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 73592a0:

Sandbox	Source
XState Example Template	Configuration
XState React Template	Configuration

ghost · 2023-06-08T22:46:38Z

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

Legend

davidkpiano

This is great, thanks for adding this!

Andarist · 2023-06-09T12:02:17Z

packages/xstate-fsm/src/types.ts

@@ -136,7 +136,7 @@ export namespace StateMachine {
    states: {
      [key in TState['value']]: {
        on?: {
-          [K in TEvent['type']]?: SingleOrArray<
+          [K in TEvent['type'] | '*']?: SingleOrArray<
            Transition<
              TContext,
              TEvent extends { type: K } ? TEvent : never,


This bit here wont be correct anymore with this change. With '*' u dont want to "filter" from TEvent based on { type: K }, you just want to grab TEvent as a whole

I'm afraid you've lost me. 🙃

If I understand the typings correctly, this change is from "valid keys are of type K, where K is any type assignable to the value of property "type" on type TEvent" to "…where K is the union of any type assignable to the value of property "type" on type TEvent and "*"". The change is made here rather than on TEvent["type"] because "*" has a valid meaning when describing a transition, but is not valid as an event type in general, such as when calling Machine.transition(…).

If that doesn't address your concern, let me know!

If I understand the typings correctly, this change is from "valid keys are of type K, where K is any type assignable to the value of property "type" on type TEvent" to "…where K is the union of any type assignable to the value of property "type" on type TEvent and "*""

This change is correct. The problem was created here though because the "value position" of this mapped type has not been adjusted anyhow and it was defined (and still is) as:

Transition< TContext, TEvent extends { type: K } ? TEvent : never, TState['value'] >

If we substitute K for '*' here then we end up with:

Transition< TContext, TEvent extends { type: '*' } ? TEvent : never, TState['value'] >

So in this case we'll try to filter TEvent members that extend { type: '*' } and such should never exist (we can't truly disallow them at type-level but we already disallow them at runtime with "If the transition method is called with an event whose type is a single asterisk, it will throw in non-production mode. (This matches the behavior of the full XState library.)")

What I'm saying is that we should end up here with something closer to this:

Transition< TContext, K extends '*' ? TEvent : TEvent extends { type: K } ? TEvent : never, TState['value'] >

Take a look at how we handle this in the XState's types:

xstate/packages/core/src/types.ts

Lines 490 to 494 in 754afa0

export type TransitionsConfigMap<TContext, TEvent extends EventObject> = {

[K in TEvent['type'] | '' | '*']?: K extends '' | '*'

? TransitionConfigOrTarget<TContext, TEvent>

: TransitionConfigOrTarget<TContext, ExtractEvent<TEvent, K>, TEvent>;

};

Got it! I hadn't noticed earlier that K was being used deeper in the type. 🤦

I spent some time playing around with the types and in the end, I'm not sure how this should be addressed. If K were being used to specify the type for an event, it would certainly cause trouble, but because it's sole use is in an extends clause, the typing of Transition<…> doesn't actually change.

In fact, I wasn't able to convince myself that TEvent extends { type: K } ? TEvent : never can actually result in never here, with or without adding the wildcard. TEvent is constrained to { type: string } via extending EventObject, which means that TEvent["type"] always extends string; "*" also extends string, which means that K does, as well. A subtype of string will always extend the union of that same subtype with any subtype of string. In other words, ("a" | "b" | "c") extends ("a" | "b" | "c" | "*") ? true : false is always true.

Of course, knowing that requires more than just a glance at the types; having K show up in the type passed to Transition<…> after explicitly including "*" in K certainly suggests that it has some effect there, even though it doesn't. I can think of three ways to address that:

Remove "*" from the type in the conditional, e.g. TEvent extends { type: Exclude<K, "*"> } ? TEvent : never.

Separate the types of on[K in TEvent['type']] and on["*"], though if I'm correct, they'd simply be identical.

Get rid of the whole conditional; change it to just Transition<TContext, TEvent, TState["value"]>, which I believe it's already equivalent to. Unsurprisingly, this is the one I'm most partial to. 🙂

I also put together a CodePen sandbox to demonstrate a bunch of this. Let me know what you think!

The problem is that K is distributed through instantiations of a mapped type's template. That conditional type is not testing things like ("a" | "b" | "c") extends ("a" | "b" | "c" | "*"), it just tests a concrete K, a single member of the union over which we are iterating there. Without that you wouldn't be able to write the simplest:

type KeysAsValues<T> = { [K in keyof T]: K } type Mapped = KeysAsValues<{ a: 1, b: 2 }> // actual: { a: "a", b: "b" }, without instantiated/distributed `K`: { a: "a" | "b", b: "a" | "b" }

Of course, knowing that requires more than just a glance at the types; having K show up in the type passed to Transition<…> after explicitly including "*" in K certainly suggests that it has some effect there, even though it doesn't.

The effect, and the fact that it is narrowed to never for wildcards, can be easily observed on this TS playground

I can think of three ways to address that

We should just route this through a conditional type to a branch that narrows down the event type or to the one that skips the narrowing and just forwards the whole TEvent, just like in the quoted types from the core module:

xstate/packages/core/src/types.ts

Lines 490 to 494 in 754afa0

export type TransitionsConfigMap<TContext, TEvent extends EventObject> = {

[K in TEvent['type'] | '' | '*']?: K extends '' | '*'

? TransitionConfigOrTarget<TContext, TEvent>

: TransitionConfigOrTarget<TContext, ExtractEvent<TEvent, K>, TEvent>;

};

Bah; it's very frustrating that I wasn't able to come up with an example of adding "*" having an effect on Transition<…>. Thanks for providing one! I'll need to spend some time studying how TS handles concrete type variables.

In the meantime, I've updated the PR to fix the types; I was also finally able to add that related test I was looking for.

Thanks for helping me through this type stuff! I'm learning a lot. 🙂

For example, I just learned that VS Code wasn't fully honoring the package's TypeScript configuration. 🤦

Because the TEvent parameter to Transition<…> is only used in function signatures, the test doesn't work unless "strictFunctionTypes" is enabled in the TypeScript config. Unsurprisingly, enabling it for the entire repo blows up, but enabling it for just the @xstate/fsm package works just fine, so that's what I've done.

Let me know if that isn't a desirable change.

For example, I just learned that VS Code wasn't fully honoring the package's TypeScript configuration. 🤦

Yeah, this is pretty annoying. I execute "Select TypeScript version..." command right after opening just any new project on my machine. There are ways to configure VS Code to prompt you to do this - they don't want to run the installed TS version without explicit consent for security reasons.

Because the test checks the `TEvent` parameter to `Transition`, which is only used in function signatures, the TypeScript configuration must have `strictFunctionTypes` enabled for the test to function.

Andarist · 2023-06-15T12:26:19Z

packages/xstate-fsm/test/fsm.test.ts

@@ -131,6 +131,88 @@ describe('@xstate/fsm', () => {
    expect(nextState.actions).toEqual([]);
  });

+  describe('when a wildcard transition is defined', () => {
+    type ParserContext = { entries: Array<Record<string, string>> };


I'd copy-paste this stuff into each added test and trim down redundant bits for each particular test. Don't be afraid of some repetition, embrace it! 😉

Sounds good. I was mostly trying for a setup similar to the lightFSM used in many of the tests, but it really did end up complicating things unnecessarily.

The Event and State types still ended up being the same across all five tests (but much simpler), so those are still declared just once, but the config and machine creation have been moved into each test. I think it's easier to follow what's going on in each one, now.

Andarist · 2023-06-15T12:28:54Z

packages/xstate-fsm/tsconfig.json

@@ -2,6 +2,7 @@
  "extends": "../../tsconfig.base.json",
  "include": ["src/**/*"],
  "compilerOptions": {
+    "strictFunctionTypes": true,


To avoid merge conflicts with the next branch, I'd like to leave this off - even though that in general this is a good change. On the next branch we'll work on enabling this setting globally soon.

Okay. I've reverted this change, disabled the @ts-expect-error comment as you suggested, and added a comment to the test saying that it isn't actually working at the moment.

Andarist · 2023-06-15T12:30:13Z

packages/xstate-fsm/test/types.test.ts

+              target: 'one',
+              actions: (_context: Context, _event: InitEvent | FooEvent) => {}
+            },
+            // @ts-expect-error


to accompany the other suggested change I'd change this to smth like:

Suggested change

// @ts-expect-error

// @x-ts-expect-error

We'll "turn this on" at later point in time.

Andarist · 2023-06-15T12:33:57Z

packages/xstate-fsm/src/types.ts

            Transition<
              TContext,
-              TEvent extends { type: K } ? TEvent : never,
+              K extends '*'


i think that by routing this earlier it becomes a little bit more readable, so I'd use this variant:

on?: { [K in TEvent["type"] | "*"]?: SingleOrArray< K extends "*" ? Transition<TContext, TEvent, TState["value"]> : Transition< TContext, TEvent extends { type: K } ? TEvent : never, TState["value"] > >; };

- Simplified the types used in the wildcard bevavior tests to make it easier to follow them. - Refactored `StateMachine.Config` to make it more readable. - Reverted enabling "strictFunctionTypes" and added a note to the relevant test saying that it will need updated when that option is reenabled. (Enabling "strictFunctionTypes" globally will be part of a separate effort.)

.changeset/tall-flies-occur.md

benblank added 2 commits June 8, 2023 15:20

Add support for wildcard transitions to xstate-fsm.

baeb8bd

Update docs for xstate-fsm to match its README.

15cb016

Add changeset for wildcard event types.

8c1d92e

davidkpiano approved these changes Jun 9, 2023

View reviewed changes

Andarist reviewed Jun 9, 2023

View reviewed changes

benblank added 2 commits June 14, 2023 15:46

Fix typing of wildcard transitions.

26ae801

Fix broken type test.

c58c684

Because the test checks the `TEvent` parameter to `Transition`, which is only used in function signatures, the TypeScript configuration must have `strictFunctionTypes` enabled for the test to function.

Andarist reviewed Jun 15, 2023

View reviewed changes

Andarist reviewed Jun 16, 2023

View reviewed changes

.changeset/tall-flies-occur.md Outdated Show resolved Hide resolved

Update .changeset/tall-flies-occur.md

73592a0

Andarist merged commit 3b4b130 into statelyai:main Jun 16, 2023
2 of 3 checks passed

github-actions bot mentioned this pull request Jun 16, 2023

Version Packages #4076

Merged

benblank deleted the fsm-wildcards branch June 16, 2023 14:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for wildcard transitions to xstate-fsm #4065

Add support for wildcard transitions to xstate-fsm #4065

benblank commented Jun 8, 2023

changeset-bot bot commented Jun 8, 2023 •

edited

codesandbox-ci bot commented Jun 8, 2023 •

edited

ghost commented Jun 8, 2023 •

edited by ghost

Legend

davidkpiano left a comment

Andarist Jun 9, 2023

benblank Jun 9, 2023 •

edited

Andarist Jun 9, 2023

benblank Jun 10, 2023

Andarist Jun 14, 2023

benblank Jun 14, 2023

benblank Jun 14, 2023

Andarist Jun 15, 2023

Andarist Jun 15, 2023

benblank Jun 15, 2023

Andarist Jun 15, 2023

benblank Jun 15, 2023

Andarist Jun 15, 2023

Andarist Jun 15, 2023

benblank Jun 15, 2023

	export type TransitionsConfigMap<TContext, TEvent extends EventObject> = {
	[K in TEvent['type'] \| '' \| '']?: K extends '' \| ''
	? TransitionConfigOrTarget<TContext, TEvent>
	: TransitionConfigOrTarget<TContext, ExtractEvent<TEvent, K>, TEvent>;
	};

Add support for wildcard transitions to xstate-fsm #4065

Add support for wildcard transitions to xstate-fsm #4065

Conversation

benblank commented Jun 8, 2023

changeset-bot bot commented Jun 8, 2023 • edited

🦋 Changeset detected

codesandbox-ci bot commented Jun 8, 2023 • edited

ghost commented Jun 8, 2023 • edited by ghost

Legend

davidkpiano left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benblank Jun 9, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

changeset-bot bot commented Jun 8, 2023 •

edited

codesandbox-ci bot commented Jun 8, 2023 •

edited

ghost commented Jun 8, 2023 •

edited by ghost

benblank Jun 9, 2023 •

edited