Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core] Ensure root node gets included in configuration for entry/exit actions #2149

Merged
merged 8 commits into from
May 10, 2021

Conversation

davidkpiano
Copy link
Member

@davidkpiano davidkpiano commented Apr 26, 2021

Fixes #2147, fixes #2176

@changeset-bot
Copy link

changeset-bot bot commented Apr 26, 2021

🦋 Changeset detected

Latest commit: 47b9af5

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

This PR includes changesets to release 1 package
Name Type
xstate Patch

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

@davidkpiano
Copy link
Member Author

@Andarist Ready for final review

@Andarist
Copy link
Member

Observation: before investigating #2176 I had a mental model of us adding an "implied" root node but I've realized here that it cannot be seen as such without overcomplicating abstract logic for final states. What we do (in contrast to SCXML) is that we allow <scxml> to work more closely to <state>/<parallel> - by allowing, for example, transitions to be defined for it. If we plan to support XState -> SCXML conversion then this should be accounted for as wrapping the whole thing in that extra "implied" root node would just not cut it.

Question: what should be the value of a combinatorial machine?

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
@davidkpiano
Copy link
Member Author

Question: what should be the value of a combinatorial machine?

Just an object: {}

@davidkpiano davidkpiano merged commit 89a3880 into master May 10, 2021
@github-actions github-actions bot mentioned this pull request May 10, 2021
@Andarist
Copy link
Member

Isn't this looking slighty weird?

// value: {}
createmachine({})

// value: 'a'
createMachine({
  initial: 'a',
  states: {
    a: {}
  }
})

// value: { a: 'b' }
createMachine({
  initial: 'a',
  states: {
    a: {
      initial: 'b',
      states: {
        b: {}
      }
    }
  }
})

@VanTanev
Copy link
Contributor

Do you think the second one should be: { a: {} }?

@davidkpiano
Copy link
Member Author

Do you think the second one should be: { a: {} }?

Maybe in version 5 🤔 that's not a bad idea.

@Andarist Andarist deleted the davidkpiano/2147 branch March 22, 2023 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants