-
-
Notifications
You must be signed in to change notification settings - Fork 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
feat(Store): add an overload to createFeatureSelector to provide better type checking #1171
feat(Store): add an overload to createFeatureSelector to provide better type checking #1171
Conversation
docs/store/selectors.md
Outdated
@@ -104,6 +104,24 @@ export const selectFeatureCount = createSelector( | |||
); | |||
``` | |||
|
|||
There is also an overload on the `createFeatureSelector` method, which allows a better typechecking. |
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.
Instead of adding this as an additional section, let's just make this the default in the example above.
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.
Yea I wasn't sure about this, but you just confirmed my thinking.
Should I also update the example app?
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.
Yes
docs/store/selectors.md
Outdated
|
||
Where `AppState` is the type from the top level feature state and `FeatureState` is the type from the feature slice to select. | ||
Meaning that the following would not compile because `foo` is not a feature slice of `AppState`. | ||
|
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.
Keep the section starting with Meaning
as a note.
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.
Minor nit. Everything else LGTM
docs/store/selectors.md
Outdated
export const selectFeatureCount = createSelector( | ||
selectFeature, | ||
(state: FeatureState) => state.counter | ||
); | ||
``` | ||
|
||
Meaning that the following would not compile because `foo` is not a feature slice of `AppState`. | ||
|
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.
Change to The following selector below would not ...
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.
No problem! Thanks for the reviews!
Closes #1136