[@xstate/store] v3 #5175
[@xstate/store] v3 #5175
Conversation
🦋 Changeset detectedLatest commit: 80fe593 The changes in this PR will be included in the next version bump. 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 |
- Added `StoreEffect` type to support both emitted events and side effects. - Updated `createStoreTransition` to return effects instead of emitted events. - Modified `receive` function to handle effects, executing functions or emitting events accordingly. - Added a test case to verify that effects can be enqueued and executed after state updates. This change improves the flexibility of the store's event handling mechanism.
This reverts commit cfe28e6.
- Updated `createStore` and `createStoreWithProducer` to use more explicit type parameters
- Replaced `types: { emitted }` with separate type parameters for context, event payloads, and emitted events
- Removed `Cast` import and simplified type definitions
- Updated test cases to use new type parameter approach
- Added `EventMap` type to support event type mapping
.changeset/big-schools-hang.md
Outdated
| // Sent events | ||
| { | ||
| inc: { | ||
| by: number; | ||
| }; | ||
| }, |
There was a problem hiding this comment.
I think the need for this explicit slot is a significant DX downgrade, but I guess you have already made up your mind on this. It feels like this now uses different set of tradeoffs than setup from xstate which is somewhat weird, even if both libraries are not strictly related.
There was a problem hiding this comment.
You don't need this if you're not using emitted events; it's still inferred. But if you are, this:
createStore({
types: {
emitted: {} as ...
},
// …
});is also not really a good DX.
There was a problem hiding this comment.
I'll change this since this is now inferred
| } | ||
|
|
||
| export const createStore: { |
There was a problem hiding this comment.
If you are going with "Type parameters should now be explicitly provided to createStore and createStoreWithProducer" then those overloads are not needed at all here and this could be just simplified/inlined
.changeset/big-schools-hang.md
Outdated
| { | ||
| inc: { | ||
| by: number; | ||
| }; | ||
| }, |
There was a problem hiding this comment.
This payload map doesn't feel like an ergonomic explicit type argument - you won't need it anywhere else than at the createStore call. A union plays better with other utils you might want to build around this API as a user. Like, even a logEvent becomes much more cumbersome to type because you need to somehow extract the event type from your store instead of just using the same union type that you could use here
There was a problem hiding this comment.
The payload map is mostly to still allow for inference when the type parameters are not provided.
|
|
||
| expect(store.getSnapshot().context.count).toEqual(1); | ||
|
|
||
| await new Promise((resolve) => setTimeout(resolve, 10)); |
There was a problem hiding this comment.
you can import a shared util for this instead:
import { sleep } from '@xstate-repo/jest-utils';
| @@ -144,17 +147,16 @@ it('createStoreWithProducer(…) works with an immer producer (object API)', () | |||
| }); | |||
|
|
|||
| it('createStoreWithProducer(…) infers the context type properly with a producer', () => { | |||
There was a problem hiding this comment.
this test should be removed now, it was specifically testing the non-object API. A test for the object API is directly below this one an it's the only one that you need after deprecating and removing the previous API
|
Oh no... 😭 #5183 Figuring out how to pass the types around was difficult. But I think I made some improvements, even beyond the new getter API. But when I saw how many changes where in this PR... I am going to get a beer. |
Hey @expelledboy – computed properties are a great idea! I think you'll find the types even simpler to work with in this PR. Let's make it work. 🛠️ |
No description provided.