Skip to content

Drop support for TransitionsConfigArray#4123

Merged
Andarist merged 4 commits intonextfrom
support-transition-maps-exclusively
Jul 7, 2023
Merged

Drop support for TransitionsConfigArray#4123
Andarist merged 4 commits intonextfrom
support-transition-maps-exclusively

Conversation

@Andarist
Copy link
Collaborator

@Andarist Andarist commented Jul 6, 2023

I think that:

  1. TransitionsConfigArray complicated our types
  2. it also complicated our implementation and slowed down the most basic scenario (we were searching for candidates with explicit descriptors in the array instead of just looking it up in a map)
  3. it's rarely used feature (if used at all)
  4. its existence it makes vizualization harder/less obvious. We don't visually order (nor denote order) anyhow. By removing the ability to order transitions arbitrarily in an array we make it more intuitive to interpret the visualization: explicit descriptions > longest-to-shortest partial descriptors > wildcard

This PR is not meant to refactor all inner bits. The main goal here was to remove this from the types and the public API, the internals could still use some further cleanups but that can be done at any time.

@Andarist Andarist requested a review from davidkpiano July 6, 2023 15:32
@changeset-bot
Copy link

changeset-bot bot commented Jul 6, 2023

🦋 Changeset detected

Latest commit: 822fe64

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

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 6, 2023

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 822fe64:

Sandbox Source
XState Example Template Configuration
XState React Template Configuration

@ghost
Copy link

ghost commented Jul 6, 2023

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

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

// 'script-src': ['test0', 'test1', 'test2', 'test3'], // <script/> conversion not implemented
'scxml-prefix-event-name-matching': [
'star0'
// 'star0' // this relies on the source order of transitions where * is first and it's supposed to get macthed over an explicit descriptor
Copy link
Member

Choose a reason for hiding this comment

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

Do you think there is value in supporting this use-case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think so - it makes visualization less obvious. If somebody really needs smth like this they can always divide their state into 2 nested ones and put "higher priority" transitions in the inner one.

@Andarist Andarist merged commit b13bfcb into next Jul 7, 2023
@Andarist Andarist deleted the support-transition-maps-exclusively branch July 7, 2023 07:46
@github-actions github-actions bot mentioned this pull request Jul 7, 2023
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.

2 participants