fix: create store type differenciation #390
Conversation
WalkthroughThis pull request refines the Zustand store's type handling by introducing new imports from the "zustand" library and adjusting type assertions. The Changes
Possibly related PRs
Poem
Tip 🌐 Web search-backed reviews and chat
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
MartianGreed
force-pushed
the
fix/create-store
branch
from
e6406f2 to
395e450
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/sdk/src/react/hooks.ts (1)
16-16: Consider improving the comment and tracking the technical debt.The comment "hacktually" suggests this is a temporary solution. Consider:
- Using a more professional comment style
- Creating a TODO with more context about the proper solution
- Opening a follow-up issue to track this technical debt
- // hacktually until I find a proper type input to createDojoStoreFactory + // TODO: Implement proper type input for createDojoStoreFactory + // This is a temporary type assertion until we can implement proper generic constraints + // Follow-up issue: <issue_url>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/sdk/src/react/hooks.ts(2 hunks)packages/sdk/src/state/zustand.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/sdk/src/state/zustand.ts
🔇 Additional comments (2)
packages/sdk/src/react/hooks.ts (2)
5-5: LGTM! Import changes align with type differentiation goals.The added imports for
StoreApi,UseBoundStore, andGameStateprovide the necessary type definitions for improved store type handling.Also applies to: 7-7
17-19: Verify type compatibility between createDojoStoreFactory and UseBoundStore.While the type assertion improves type safety for consumers, using
unknowncould hide potential type incompatibilities. Please verify that the actual return type ofcreateDojoStoreFactoryis compatible withUseBoundStore<StoreApi<GameState<T>>>.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for createDojoStoreFactory implementation and its return type ast-grep --pattern 'export function createDojoStoreFactory<$_>($_) { $$$ }'Length of output: 12807
I'll now gather additional context on the underlying type of the
storeCreatorFnparameter (i.e.CreateStore) to confirm that its return type matches the expectedUseBoundStore<StoreApi<GameState<T>>>. We'll search for its definition within the repository.
🏁 Script executed:
#!/bin/bash # Search for the definition of CreateStore type in the repository rg "type CreateStore" -n .Length of output: 88
I'll gather additional context for the
CreateStoretype andUseBoundStoredefinition to confirm type compatibility. Let's examine the surrounding lines of theCreateStoredefinition.
🏁 Script executed:
#!/bin/bash # Display the context around the CreateStore definition. sed -n '1,50p' packages/sdk/src/state/zustand.tsLength of output: 1973
Type compatibility between createDojoStoreFactory and UseBoundStore is valid.
- The function receives a store creator (i.e. Zustand's
create) and calls it with the appropriate initializer.- The custom
CreateStoretype specifies a return ofStoreApi<T>, but Zustand’s actualcreatereturns a store hook that augments this with a call signature (i.e. aUseBoundStore<StoreApi<T>>).- Casting via
as unknown as UseBoundStore<StoreApi<GameState<T>>>safely converts the narrower declared type into the actual type returned by Zustand’screate.- In short, provided that the underlying
createfollows Zustand’s implementation, the asserted type is compatible with the returned value.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/friendly-emus-beam.md (1)
16-17: Improve Commit Message GrammarThe commit message currently reads "fix: createStore now fit to either vanilla or react store". To ensure proper subject–verb agreement, consider changing "fit" to "fits". This small improvement enhances clarity and professionalism.
Suggested diff:
- fix: createStore now fit to either vanilla or react store + fix: createStore now fits to either vanilla or react store🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...-wasm": patch --- fix: createStore now fit to either vanilla or react store(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/friendly-emus-beam.md(1 hunks)packages/sdk/src/react/hooks.ts(2 hunks)packages/sdk/src/state/zustand.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/sdk/src/state/zustand.ts
- packages/sdk/src/react/hooks.ts
🧰 Additional context used
🪛 LanguageTool
.changeset/friendly-emus-beam.md
[uncategorized] ~16-~16: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...-wasm": patch --- fix: createStore now fit to either vanilla or react store
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: check
- GitHub Check: build
🔇 Additional comments (1)
.changeset/friendly-emus-beam.md (1)
1-14: Approved Dependency Patch UpdatesThe list of dependency updates is clear and correctly formatted for patch level changes. Ensure that any related documentation or changelog entries are updated accordingly.
Closes #
Introduced changes
Checklist
Summary by CodeRabbit
createStorefunction to support both vanilla and React store implementations.