-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Passed state type generic to the Config interface #1122
Conversation
🦋 Changeset is good to goLatest commit: c745df0 We got this. This PR includes changesets to release 1 package
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 |
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 c745df0:
|
@@ -81,7 +81,7 @@ export function createMachine< | |||
TEvent extends EventObject = EventObject, | |||
TState extends Typestate<TContext> = any | |||
>( | |||
fsmConfig: StateMachine.Config<TContext, TEvent>, | |||
fsmConfig: StateMachine.Config<TContext, TEvent, TState>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to open a discussion: XState core always uses <Context, State, Event>
so I'm not sure if @davidkpiano and @Andarist would be okay with the xstate-fsm
API to differ. In this case, it would obviously be a breaking change to decide in favor of consistency by having the type arguments in the same order as within the core.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TState
is already the third argument in the State
and Machine
interfaces in fsm
, so this PR follows those patterns. Since TState
defaults to any
this shouldn't be a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CodingDive Moving forward in V5, the generic arguments will always follow <TContext, TEvent, ...>
in that order.
Co-Authored-By: Mateusz Burzyński <mateuszburzynski@gmail.com>
The
StateMachine.Config
interface should accept a third generic for our state type, which should then be provided when used in thecreateMachine
function. I think. I have declared my own module and haven't had any issues, but if there are any reasons this wasn't set up this way originally let me know.Using the state type provides helpful intellisense when defining the state chart transitions!
NOTE: I ran tests locally but a bunch of them failed because local modules weren't resolved. Not sure if there is a step missing in
CONTRIBUTING.md
to link these packages.