Introduce `TExpressionEvent` generic #3694

Andarist · 2022-12-07T11:20:31Z

With this change, the inferred types for raise are being improved. We can see here that both TExpressionEvent and TEvent are inferred correctly here - to { type: 'DECIDE'; aloha?: boolean | undefined; } and GreetingEvent, respectively.

In here we can see that this improves autocomplete capabilities in the IDE because the contextual type is restricted to what can actually be sent to this machine:

And in here we can see that a type error is raised (huehue) correctly when we try to use an invalid event:

⚠️ there are some type failures here as I didn't yet adjust everything around the implementation etc. This only touches public-facing APIs etc - as only those had to be adjusted to validate the idea.

I'm not super keen on touching types in v4 as I'd really like to completely rewrite them in v5 (IMHO it's the easiest way to clean them up). OTOH, this brings some very nice improvements to the table.

Our types are so complex that it's quite hard to predict all potential problems with this but, dunno, perhaps it's worth shipping this?

fixes #2897
fixes #1414
fixes #2994

changeset-bot · 2022-12-07T11:20:34Z

🦋 Changeset detected

Latest commit: a758eee

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

This PR includes changesets to release 1 package

Name	Type
xstate	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

ghost · 2022-12-07T11:21:29Z

👇 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 · 2022-12-07T11:25:44Z

packages/core/src/actions.ts

+  TExpressionEvent extends EventObject,
+  TEvent extends EventObject = TExpressionEvent
+>(
+  event: NoInfer<TEvent>,


previously this was typed as Event<TEvent>:

xstate/packages/core/src/types.ts

Line 90 in 70f426a

export type Event<TEvent extends EventObject> = TEvent['type'] | TEvent;

This wasn't correct for events with payload. IIRC we've decided to ditch the idea of extracting "simple" events and allowing them to be used as strings. I think it would be OK-ish to introduce such a "breaking change" (type-level only) in a minor version. Let me know what do you think.

Yeah, that's fine. Worst-case scenario, it would only "break" code at the type-level (ignorable) but still work exactly the same.

Andarist · 2022-12-07T11:26:50Z

packages/core/src/types.ts

+  TContext,
+  TExpressionEvent extends EventObject,
+  TEvent extends EventObject = TExpressionEvent
+> extends ActionObject<TContext, TExpressionEvent, TEvent> {


Adding this extends was one of the crucial changes here.

davidkpiano · 2022-12-11T05:51:29Z

Yeah, this might be too big of a change to introduce to v4. Is there a more limited change that we can make to improve the raise typings for now, like I introduced in the linked PR?

Andarist · 2022-12-11T09:49:32Z

Yeah, this might be too big of a change to introduce to v4.

Note that I've tried to ensure maximum (and reasonable) backward compatibility here. That's why I didn't reorder some type parameters and why I put default type arguments in all places that I touched.

The only unknown, from my PoV, is how this might impact projects "in the wild". Our types are so complex that it's super hard to predict this. I plan to add a lot of type-related tests in v5 but that's a future talk.

The shape of the PR in itself - I think it's good. That doesn't mean that we have to ship this though. I'm really tempted though, just take a look at those DX improvements and correctness! 😅

Is there a more limited change that we can make to improve the raise typings for now, like I introduced in the linked PR?

The question here is - what is our aim with the type-related changes? If I'd know how to change this in a simpler way in v4 then I would do it differently right from the start. The changes in the linked PR are not correct - they just allow things in an unsafe way, so "accidentally" the errors go away. Arguably... the old types weren't that much better (I'd have to take another look to confirm that) so it's not necessarily like those changes introduce a regression (but again, I'd have to recheck that), maybe they just trade one incorrect/unsound thing for another incorrect/unsound thing.

davidkpiano · 2022-12-11T16:38:28Z

@Andarist I'll take a deeper look at this. My main goal is to make the normal usage of raise(...) work instead of incorrectly causing type errors.

codesandbox-ci · 2023-01-18T12:13:27Z

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

Sandbox	Source
XState Example Template	Configuration
XState React Template	Configuration
cool-jones-1opd0y	Issue #2994

davidkpiano

Looks good, but can we add a // @ts-expect-error test?

The base branch was changed.

packages/core/src/interpreter.ts

packages/core/test/types.test.ts

davidkpiano · 2023-01-31T20:26:28Z

packages/xstate-react/test/useActor.test.tsx

-      context: {
-        actorRef: undefined
-      },
+      context: {} as Ctx,


Why wouldn't actorRef: undefined work?

packages/xstate-react/test/useSelector.test.tsx

# Clean up # fixed types # fix more problems manifested in tests # fixed an immer problem

Andarist requested a review from davidkpiano December 7, 2022 11:22

Andarist commented Dec 7, 2022

View reviewed changes

Andarist mentioned this pull request Dec 7, 2022

Deprecate send() action creator #3393

Merged

Andarist marked this pull request as ready for review January 18, 2023 12:55

davidkpiano previously approved these changes Jan 18, 2023

View reviewed changes

davidkpiano mentioned this pull request Jan 23, 2023

raise actions cause TypeScript errors #1414

Closed

Andarist force-pushed the davidkpiano/deprecate-send branch 4 times, most recently from b0bb50d to 1711278 Compare January 27, 2023 13:10

Andarist force-pushed the andarist/texpressionevent-spike branch from c09b713 to 55e2143 Compare January 27, 2023 13:44

Andarist changed the base branch from davidkpiano/deprecate-send to stop-reusing-send-in-raise January 27, 2023 13:45

Base automatically changed from stop-reusing-send-in-raise to davidkpiano/deprecate-send January 27, 2023 20:03

Andarist force-pushed the andarist/texpressionevent-spike branch from 941c4fe to c6a9dc7 Compare January 27, 2023 20:08

davidkpiano reviewed Jan 31, 2023

View reviewed changes

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

davidkpiano reviewed Jan 31, 2023

View reviewed changes

packages/core/test/types.test.ts Outdated Show resolved Hide resolved

davidkpiano reviewed Jan 31, 2023

View reviewed changes

packages/core/test/types.test.ts Show resolved Hide resolved

davidkpiano reviewed Jan 31, 2023

View reviewed changes

packages/xstate-react/test/useSelector.test.tsx Outdated Show resolved Hide resolved

Andarist force-pushed the andarist/texpressionevent-spike branch 3 times, most recently from 27528ad to 5c313ba Compare February 10, 2023 11:34

Base automatically changed from davidkpiano/deprecate-send to main February 12, 2023 07:57

Andarist force-pushed the andarist/texpressionevent-spike branch from 2c6d655 to 0c335e5 Compare February 12, 2023 08:05

Introduce TExpressionEvent generic

d7c99a3

# Clean up # fixed types # fix more problems manifested in tests # fixed an immer problem

Andarist force-pushed the andarist/texpressionevent-spike branch from 0c335e5 to 072e6bb Compare February 12, 2023 10:07

Fix things

f732c0e

Andarist force-pushed the andarist/texpressionevent-spike branch from 072e6bb to f732c0e Compare February 12, 2023 10:11

Andarist added 2 commits February 12, 2023 14:21

cleanup

a543254

add changeset

a758eee

Andarist merged commit fd58905 into main Feb 13, 2023

Andarist deleted the andarist/texpressionevent-spike branch February 13, 2023 08:59

This was referenced Feb 13, 2023

Version Packages #3813

Merged

Version Packages #3827

Closed

Andarist mentioned this pull request Jun 15, 2023

[core] Improve action types with specific event generics #2690

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce `TExpressionEvent` generic #3694

Introduce `TExpressionEvent` generic #3694

Andarist commented Dec 7, 2022 •

edited

changeset-bot bot commented Dec 7, 2022 •

edited

ghost commented Dec 7, 2022 •

edited by ghost

Andarist Dec 7, 2022

davidkpiano Dec 8, 2022

Andarist Dec 7, 2022

davidkpiano commented Dec 11, 2022

Andarist commented Dec 11, 2022

davidkpiano commented Dec 11, 2022

codesandbox-ci bot commented Jan 18, 2023 •

edited

davidkpiano left a comment

davidkpiano Jan 31, 2023

Introduce TExpressionEvent generic #3694

Introduce TExpressionEvent generic #3694

Conversation

Andarist commented Dec 7, 2022 • edited

changeset-bot bot commented Dec 7, 2022 • edited

🦋 Changeset detected

ghost commented Dec 7, 2022 • edited by ghost

Legend

Andarist Dec 7, 2022

Choose a reason for hiding this comment

davidkpiano Dec 8, 2022

Choose a reason for hiding this comment

Andarist Dec 7, 2022

Choose a reason for hiding this comment

davidkpiano commented Dec 11, 2022

Andarist commented Dec 11, 2022

davidkpiano commented Dec 11, 2022

codesandbox-ci bot commented Jan 18, 2023 • edited

davidkpiano left a comment

Choose a reason for hiding this comment

davidkpiano Jan 31, 2023

Choose a reason for hiding this comment

Introduce `TExpressionEvent` generic #3694

Introduce `TExpressionEvent` generic #3694

Andarist commented Dec 7, 2022 •

edited

changeset-bot bot commented Dec 7, 2022 •

edited

ghost commented Dec 7, 2022 •

edited by ghost

codesandbox-ci bot commented Jan 18, 2023 •

edited