Reducers can occasionally receive actions without a type #44
Conversation
|
Can you please target |
src/actions.spec.ts
Outdated
| describe('action', () => { | ||
| it('should return true if type is ngrx/forms/', () => { | ||
| expect(isNgrxFormsAction({ type: 'ngrx/forms/' })).toBeTruthy(); | ||
| }); |
There was a problem hiding this comment.
Please try to be consistent with the formatting of the rest of the codebase, i.e. leave a blank line between each test.
src/actions.spec.ts
Outdated
|
|
||
| describe('action', () => { | ||
| it('should return true if type is ngrx/forms/', () => { | ||
| expect(isNgrxFormsAction({ type: 'ngrx/forms/' })).toBeTruthy(); |
There was a problem hiding this comment.
You expect the return value to be true, not just any truthy value, therefore you should assert on the concrete value (same for the other tests).
src/actions.spec.ts
Outdated
| @@ -0,0 +1,16 @@ | |||
| import { isNgrxFormsAction } from './actions'; | |||
|
|
|||
| describe('action', () => { | |||
There was a problem hiding this comment.
You are testing a concrete function, so you should use its name for the describe. Also, don't write the name as a string but instead access the name property of the function.
src/actions.ts
Outdated
| @@ -334,5 +334,5 @@ export type Actions<TValue> = | |||
| ; | |||
|
|
|||
| export function isNgrxFormsAction(action: Action) { | |||
| return action.type.startsWith('ngrx/forms/'); | |||
| return action.type && action.type.startsWith('ngrx/forms/'); | |||
There was a problem hiding this comment.
This PR would be the perfect chance to extract the prefix into a constant and use that everywhere. However, this is optional.
|
That's perfectly understandable and instead means that you care of the repository ;) will do it soon. |
|
should be fine now. ps: I extracted the string to a constant, but I'm not using it in the tests, because it can lead to false positive, I would leave it to be explicit. |
|
@MrWolfZ Can you review it? we need it as this is currently breaking our tests :( |
|
Sorry for the delay. Will merge and release the fix this evening.
|
|
I have just released version 2.3.1 which contains this fix. |
it happens only on specs, when you are testing for empty states to return the default state
(example: https://github.com/ngrx/platform/blob/master/example-app/app/books/reducers/books.spec.ts#L22)