-
Notifications
You must be signed in to change notification settings - Fork 34
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
Introduce TypedStore<State,Action>! #125
Conversation
Some additional tests are still missing but public API is final. |
I'm not following why adding a type for action to Store. What is the need this is fulfilling? To require a common subclass for all actions? I'm in favor of not requiring a type for actions. Most stores would use Any as action type and requiring a common subclass is just adding complexity & confusion imo. The |
To be clear, this feature set is entirely opt in. The benefits are typesafe dispatchers and reducers. The downsides are that some enchancers might become unavailable (e.g. thunk). In any case, traditional stores and reducers remain exactly the same. I'll bring back removed typealiases with deprecation in the next patch to this. I personally missed typesafe store in some of my projects as well. |
d304b15
to
9844d5b
Compare
@patjackson52 PR is ready for review. Here are a few things I'd like you to double-check while reviewing:
|
redux-kotlin/src/commonMain/kotlin/org/reduxkotlin/CreateStore.kt
Outdated
Show resolved
Hide resolved
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.
Lgtm, thanks for reworking and keeping original api unchanged. Addition of TypedStore seems to be requested enough that should be included in the lib.
Thanks for putting this up!
440bd7e
to
82d3ade
Compare
82d3ade
to
0a6c130
Compare
Closes #92
Closes #46