feat(Entity): log a message when selectId returns undefined in dev mode #1169
Conversation
| }); | ||
|
|
||
| it('should not warn when key does not exist in prod mode', () => { | ||
| spyOn(ngCore, 'isDevMode').and.returnValue(false); |
There was a problem hiding this comment.
You think it would be better to just set prod mode here and reset it after the selectIdValue function call?
There was a problem hiding this comment.
You mean with enableProdMode()? Because this threw an error because the platform was already set.
There was a problem hiding this comment.
Ok. Just didn't want to use the wildcard import if we didn't have to.
modules/entity/src/utils.ts
Outdated
| if (isDevMode() && key === undefined) { | ||
| console.warn( | ||
| '@ngrx/entity: The entity passed to the `selectId` implementation returned undefined. You should probably provide your own `selectId` implementation.', | ||
| 'The entity that was passed:', |
There was a problem hiding this comment.
Split up this sentence more.
| const key = selectId(entity); | ||
|
|
||
| if (isDevMode() && key === undefined) { | ||
| console.warn( |
There was a problem hiding this comment.
What do you mean by that? You would also log this when the key would be null, false?
There was a problem hiding this comment.
Yes. We also want to warn if the selected id value is false, correct?
There was a problem hiding this comment.
My initial thinking was to only check on undefined because the id property wouldn't exist on the entity, meaning you probably did something wrong. I'm not sure how this is implemented by other devs, because if one would define an entity as null, this is "hes choice" and he should know what he is doing.
Therefor I'm only checking on undefined.
For me it would also make more sense to check if the id value is false, but as I said I'm not sure how this is used.
There was a problem hiding this comment.
Ok, that makes sense. My rationale was that we're checking for values that aren't valid ids.
| import { BookModel, AClockworkOrange } from './fixtures/book'; | ||
|
|
||
| describe('Entity utils', () => { | ||
| describe(`selectIdValue()`, () => { |
There was a problem hiding this comment.
Need an additional test with a valid key that's undefined.
modules/entity/spec/utils.spec.ts
Outdated
| }); | ||
|
|
||
| it('should not warn when key is undefined in dev mode', () => { | ||
| spyOn(ngCore, 'isDevMode').and.returnValue(false); |
Closes #1133
Example of the message: