Skip to content

Conversation

@davidkpiano
Copy link
Member

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented May 25, 2023

⚠️ No Changeset found

Latest commit: c959a75

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

@davidkpiano davidkpiano requested a review from Andarist May 26, 2023 09:32
Copy link
Collaborator

@Andarist Andarist left a comment

Choose a reason for hiding this comment

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

We need to also adjust the logic to handle reenter over internal transition property.

@Andarist
Copy link
Collaborator

Andarist commented Jun 2, 2023

and, or, etc guards are not handled correctly here - we need to handle those with a logic similar to choose (we need to look into arguments passed to those and extract the used strings)

@Andarist
Copy link
Collaborator

Andarist commented Jun 6, 2023

Prepared a quick list of things that we should implement/change/consider before landing this:

  1. change tsTypes -> types.typegen, this gets slightly more complex because we might need to handle absent types as well as existing one, previously it was always just a top-level key and not mixed with other type-related things. Should we also have a codemod for this in the extension?
  2. version detection (or just a config option for starters?)
  3. default to the .d.ts strategy in v5 that is currently opt-in
  4. fix actor types, David has a PR for this -> review
  5. handle guard names recursvely in composite guards (like and(['isAdmin', 'hasPaidSubscription'])). We have limited analysis capabilities to only things that are: a) inline b) provided at createMachine call site. A isn't really new but I wonder if composite guards won't make it more apparent. B also isn't new but it's worth pointing out here that being able to .provide({ guards }) inherently makes our analysis flawed (!) and unsound
  6. analysis for event types being "propagated" through always transitions isn't implemented (since it didn't work like that in v4) -> we should implement this
  7. recheck how reenter works, it's not exactly a direct replacement for !internal - although, IIRC, we didn't yet make changes to the behavior so maybe it's enough right now. Since currently we are piggybacking on the transition's type resolved by createMachine itself... we might not need a lot of changes here.
  8. we were talking about changes to stopped actors (and not done ones!), those shouldn't be called with xstate.stop and the related change should be implemented in typegen
  9. currently we don't support partial event descriptors in typegen and we can't quite support them right now because we'd have to integrate with the TS language server to learn what are all of the available event types to relate them to partial descriptors
  10. "Make it impossible to exit a root state". IIRC we have reverted this change after making all transitions "internal" by default - it's worth rechecking but if we reverted it we don't have to implement any changes.
  11. Add support for: initial: { target: 'child', actions: 'doSmth' }
  12. onSnapshot support (?)

@davidkpiano davidkpiano marked this pull request as ready for review June 6, 2023 14:46
@davidkpiano
Copy link
Member Author

Prepared a quick list of things that we should implement/change/consider before landing this:

  1. 🟢 change tsTypes -> types.typegen, this gets slightly more complex because we might need to handle absent types as well as existing one, previously it was always just a top-level key and not mixed with other type-related things. Should we also have a codemod for this in the extension?
  2. version detection (🟢 or just a config option for starters?)
  3. 🟢 default to the .d.ts strategy in v5 that is currently opt-in
  4. 🟢 fix actor types, David has a PR for this -> review
  5. handle guard names recursvely in composite guards (like and(['isAdmin', 'hasPaidSubscription'])). We have limited analysis capabilities to only things that are: a) inline b) provided at createMachine call site. A isn't really new but I wonder if composite guards won't make it more apparent. B also isn't new but it's worth pointing out here that being able to .provide({ guards }) inherently makes our analysis flawed (!) and unsound
  6. analysis for event types being "propagated" through always transitions isn't implemented (since it didn't work like that in v4) -> we should implement this
  7. recheck how reenter works, it's not exactly a direct replacement for !internal - although, IIRC, we didn't yet make changes to the behavior so maybe it's enough right now. Since currently we are piggybacking on the transition's type resolved by createMachine itself... we might not need a lot of changes here.
  8. we were talking about changes to stopped actors (and not done ones!), those shouldn't be called with xstate.stop and the related change should be implemented in typegen
  9. currently we don't support partial event descriptors in typegen and we can't quite support them right now because we'd have to integrate with the TS language server to learn what are all of the available event types to relate them to partial descriptors
  10. "Make it impossible to exit a root state". IIRC we have reverted this change after making all transitions "internal" by default - it's worth rechecking but if we reverted it we don't have to implement any changes.
  11. Add support for: initial: { target: 'child', actions: 'doSmth' }
  12. onSnapshot support (?)

Marked the ones that I think should go in for initial release; only the first 4 are important IMO for most common use-cases.

@zgotsch
Copy link

zgotsch commented Jun 15, 2023

Is this blocked by statelyai/xstate#4036?

davidkpiano and others added 2 commits September 8, 2023 10:41
@dmnd
Copy link

dmnd commented Sep 29, 2023

I've been putting off upgrading waiting for this, is it coming soon?

@a-mlo
Copy link

a-mlo commented Dec 6, 2023

Hi @davidkpiano! Hope you're well and thanks for the great tools! Just wondering if this PR is still relevant or whether v5 is supported by the VSCode extension through:

On the XState v4 to v5 migration guide it also states that VSCode extension is not yet supported (https://stately.ai/docs/migration#when-will-the-xstate-vs-code-extension-be-compatible-with-xstate-v5) but not too sure if that is still accurate given the above merged PRs that indicate v5 support, and this WIP PR hasn't been updated in several months. 😅

If it's not ready, is there a timeline where we can track the progress? Or is it dependant on the XState v5 Essential issues being resolved? (https://github.com/statelyai/xstate/labels/XState%20v5%20essential) Our team has been waiting to migrate to XState v5 but we actively use the VSCode extension when writing and reviewing the code.

Thank you! 🙇

@davidkpiano
Copy link
Member Author

Hi @davidkpiano! Hope you're well and thanks for the great tools! Just wondering if this PR is still relevant or whether v5 is supported by the VSCode extension through:

On the XState v4 to v5 migration guide it also states that VSCode extension is not yet supported (https://stately.ai/docs/migration#when-will-the-xstate-vs-code-extension-be-compatible-with-xstate-v5) but not too sure if that is still accurate given the above merged PRs that indicate v5 support, and this WIP PR hasn't been updated in several months. 😅

If it's not ready, is there a timeline where we can track the progress? Or is it dependant on the XState v5 Essential issues being resolved? (https://github.com/statelyai/xstate/labels/XState%20v5%20essential) Our team has been waiting to migrate to XState v5 but we actively use the VSCode extension when writing and reviewing the code.

Thank you! 🙇

Hey, this is still in progress. We are actually redoing the machine extractor devtools because we need to move away from recast and use typescript (as a library) instead. This will give us more flexibility, better performance and safety, and the ability to make more granular updates that preserves as much of the original code as much as possible for bidirectional editing.

The timeline for this is early 2024, if all goes well.

@a-mlo
Copy link

a-mlo commented Dec 6, 2023

@davidkpiano Thanks for the quick reply! Fingers crossed 🤞

@Andarist Andarist closed this Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants