[@xstate/store] createStoreLogic and selectors#5490
[@xstate/store] createStoreLogic and selectors#5490davidkpiano wants to merge 6 commits intomainfrom
createStoreLogic and selectors#5490Conversation
🦋 Changeset detectedLatest commit: 81648e8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
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 |
Andarist
left a comment
There was a problem hiding this comment.
note that all of my comments are just questions, not suggestions
| storeWithSelectors.with = ((extension: any) => { | ||
| const extended = originalWith(extension); | ||
| return attachSelectors(extended, selectorsConfig); | ||
| }) as any; |
There was a problem hiding this comment.
q: will this work properly with multi-layer .with calls? like x.with().with().with(). It feels like it will lead to calling attachSelectors for each of those layers and I wonder if that's a) optimal b) correct
There was a problem hiding this comment.
on that point, why emits and selectors are threaded through things differently? emits go through createStoreCore and selectors go through attachSelectors
There was a problem hiding this comment.
q: will this work properly with multi-layer
.withcalls? likex.with().with().with(). It feels like it will lead to callingattachSelectorsfor each of those layers and I wonder if that's a) optimal b) correct
It's correct, and yeah the intermediate selector atoms get GC'd which is slightly wasteful but correct & simple. Also selectors are after-the-fact (they observe the store; they're not part of the store), and we shouldn't add them to storeLogic
| | (StoreConfig<any, any, any> & { | ||
| selectors?: Record<string, (context: any) => any>; | ||
| }) |
There was a problem hiding this comment.
q: related to some of the other questions - why selectors are not part of the StoreConfig?
There was a problem hiding this comment.
Keeps it additive so we don't pollute the StoreConfig interface- createStoreConfig and fromStore don't need it at the moment. We can move it in later if we decide to make selectors a broader XState concept maybe?
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
- Remove unnecessary .bind(store) on .with() (uses closures, not this) - Remove redundant intersection on selectors type - Split createStoreLogic into 4 overloads to fix TContext inference in selectors for both static context and input-function cases - Keep selectors outside StoreConfig (correct separation) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…thub.com/statelyai/xstate into davidkpiano/create-store-logic-selectors
|
Thank you for implementing this feature. I am a primary user of the ability to create multiple store instances from a store definition. Could you confirm if we are planning to use the value returned by |
Good question; not yet but it should. |
| createStore: [TInput] extends [void] | ||
| ? () => StoreWithSelectors<TContext, TEventPayloadMap, TEmitted, TSelectors> | ||
| : ( | ||
| input: TInput | ||
| ) => StoreWithSelectors<TContext, TEventPayloadMap, TEmitted, TSelectors>; |
There was a problem hiding this comment.
we'd usually only allow a dynamic context to be created based on the provided input. Creating the whole store feels like a divergence from the other APIs
Added
createStoreLogicfor reusable store definitions with input andselectorssupport for bothcreateStoreandcreateStoreLogic.Selectors are reactive
ReadonlyAtoms (powered bystore.select()), composable withcreateAtom, and preserved through.with()extensions.