-
-
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 option to mock selectors in MockStoreConfig #1836
feat(store): add option to mock selectors in MockStoreConfig #1836
Conversation
Preview docs changes for e009b2f at https://previews.ngrx.io/pr1836-e009b2f/ |
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, left some minor comments.
if (mockSelectors) { | ||
mockSelectors.forEach(mockSelector => { | ||
const selector = mockSelector.selector; | ||
if (typeof selector === 'string') { |
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.
Any reason why this if
statement is here? I think the overrideSelector
is the same 😃
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.
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.
What if we would tweak the overrideSelector
type?
overrideSelector<
T,
Result,
SelectorKind =
| string
| MemoizedSelector<T, Result>
| MemoizedSelectorWithProps<T, any, Result>
>(selector: SelectorKind, value: Result): SelectorKind;
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.
Preview docs changes for ed7cc49 at https://previews.ngrx.io/pr1836-ed7cc49/ |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Closes #1827
What is the new behavior?
Adds a mock selectors option to the MockStoreConfig. This will enable developers to mock selectors up front in the testing module configuration, similar to how you can provide an initial state.
This enables developers who do not need to update the state or selector to omit creating a MockStore instance.
Does this PR introduce a breaking change?
Other information
Should I add documentation in this PR or a follow up PR? Seems to usually be done as a follow up.