-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
setup().extend()
#4703
base: main
Are you sure you want to change the base?
setup().extend()
#4703
Conversation
|
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 eae625c:
|
@@ -203,17 +163,171 @@ export function setup< | |||
TTag | |||
> | |||
>; | |||
} { | |||
|
|||
extend: < |
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.
extend
can't be used to add event types, context types, etc. This should only support actions, actors, guards and delays
The problem is that this would become possible and incorrect:
setup({
types: {} as { events: { type: 'FOO' } },
actions: {
doStuff: ({ event }) => {} // sees `{ type: 'FOO' }`
}
}).extend({
types: {} as { events: { type: 'BAR' } },
}).createMachine({
initial: 'a',
states: {
a: {
on: { BAR: 'b' }
},
b: {
entry: 'doStuff' // oops, it got called with `{ type: 'BAR' }`
}
}
})
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.
Okay, that's fair
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.
setup({})
.extend({
actions: {
doOneStuff: () => {},
},
})
.createMachine({
entry: "doOtherStuff", // this should be an error
});
That's why I've mentioned on Discord that before doing this change we need to make it impossible to use non-implemented things when nothing was implemented in that "group".
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.
I have a draft for this change here but I didn't have time to finish it
ToProvidedActor<TExtChildrenMap, TExtActors>, | ||
ToParameterizedObject<TExtActions>, | ||
ToParameterizedObject<TExtGuards>, | ||
TExtDelay |
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.
all those locations should be unions with their respective "outer" parameters, the same goes for guars
and delays
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.
and in a similar manner, it's very likely that those never
s should be converted to their respective "outer" type parameters:
xstate/packages/core/src/setup.ts
Line 32 in 39702e6
? never |
xstate/packages/core/src/setup.ts
Line 55 in 39702e6
? never |
the defaults for those should still be never
- the first call in a chain doesn't have any outer type parameters after all
No description provided.