xstate icon indicating copy to clipboard operation
xstate copied to clipboard

refactor: :label: stronger types for `StateValue` when using Typegen

Open brunocrosier opened this issue 3 years ago • 11 comments

Demo of state.value having loose types while using Typegen: https://codesandbox.io/s/github/brunocrosier/test-xstate-typegen

image

Borrowing the type from: https://github.com/statelyai/xstate/blob/main/packages/core/src/State.ts#L321

brunocrosier avatar May 25 '22 20:05 brunocrosier

⚠️ No Changeset found

Latest commit: b2a466f2ba19eef4a0a26c31a3897423fd23a4e2

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

changeset-bot[bot] avatar May 25 '22 20:05 changeset-bot[bot]

👇 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

ghost avatar May 25 '22 21:05 ghost

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

Sandbox Source
XState Example Template Configuration
XState React Template Configuration
test-xstate-typegen PR

codesandbox-ci[bot] avatar May 25 '22 21:05 codesandbox-ci[bot]

lol @erikras just literally told me he wanted this when we went out for sushi

mattpocock avatar May 25 '22 21:05 mattpocock

lol @erikras just literally told me he wanted this when we went out for sushi

jealous! 🍣

I'm hoping this will work..!

brunocrosier avatar May 25 '22 21:05 brunocrosier

I think, but that would have to be verified, that reusing matchesStates is not good enough here because it accepts some object~ flavors that are not correct for the state.value.

q: why do you want to use state.value in the first place? I'm not saying that it's not OK - I just want to understand the context behind the request etc. I think that state.value & state.matches are so close to each other that I wonder if there is a value in having both of them in the API.

Andarist avatar Jun 01 '22 14:06 Andarist

I think, but that would have to be verified, that reusing matchesStates is not good enough here because it accepts some object~ flavors that are not correct for the state.value.

Sure thing! I could create a PR to add something like stateValues here: https://github.com/statelyai/xstate-tools/blob/e2d7a2b498fb4d3a666c434748d678ce6ca5f8e7/packages/shared/src/getTypegenOutput.ts#L164 ?

What would that look like though? How would it be different to matchesStates ?

q: why do you want to use state.value in the first place? I'm not saying that it's not OK - I just want to understand the context behind the request etc. I think that state.value & state.matches are so close to each other that I wonder if there is a value in having both of them in the API.

Basically, just for better type safety. I created a small repro here to show an example of when we get a type error when we should not: https://codesandbox.io/s/github/brunocrosier/test-xstate-typegen

Happy to explain in more detail if that's unclear!

brunocrosier avatar Jun 01 '22 14:06 brunocrosier

What would that look like though? How would it be different to matchesStates ?

One important difference is that only the "object syntax" is valid for state.value (+ string values for top-level atomic state) but a "dotted" notation is not valid: https://github.com/statelyai/xstate-tools/blob/041846a873d58247fe17206f1e6149916203b449/packages/shared/src/getTypegenOutput.ts#L93-L97

Another one is that .matches always mark properties as optional: https://github.com/statelyai/xstate-tools/blob/041846a873d58247fe17206f1e6149916203b449/packages/shared/src/getStateMatchesObjectSyntax.ts#L24 but for parallel regions in the .value this probably wouldn't be true, those should actually always be required.

Andarist avatar Jun 07 '22 13:06 Andarist

OK, I will see if I can create a PR in xstate-tools to add stateValues with the requirements that you mentioned 🙂

brunocrosier avatar Jun 07 '22 16:06 brunocrosier

It would still be pretty interesting to see some real-life examples of how you are using state.value in your code.

Andarist avatar Jun 08 '22 09:06 Andarist

It would still be pretty interesting to see some real-life examples of how you are using state.value in your code.

Unfortunately I don't have any examples apart from that minimal CodeSandbox, sorry! So far I've only played around with XState in personal projects - but I guess that stronger types are always better, right?

Maybe @erikras has some examples of potential use-cases?

Added PR here: https://github.com/statelyai/xstate-tools/pull/160

brunocrosier avatar Jun 08 '22 21:06 brunocrosier

Superseded by #4498

davidkpiano avatar Nov 29 '23 15:11 davidkpiano