Entity upsert wrongly documented + typing issue

[miniplus]

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report
[ ] Feature request
[ ] Documentation issue or request

What is the current behavior?

• Using upsertOne and upsertMany from @ngrx/entity complains about typing (User vs Update<User>). See reproduction app/user.reducer.ts#14 & app/user.reducer.ts#18.
• Using upsertOne and upsertMany functions from @ngrx/entity isn't updating the store correctly when entity or entities are passed to upsertOne or upsertMany as documenten in Medium Blogpost or github docs. Instead a type of Update<Entity> is expected (which is in the form of {id: number; changes: Partial<Entity>} (See reproduction example)

Possibly related to #817.

Expected behavior:

• When entity is not in store yet, it's being added.
• When entity is already in store, it's being updated
• Typing in reducer when using adapter.upsertOne / adapter.upsertMany doesn't complain about type User vs Update<User>

Minimal reproduction of the problem with instructions:

https://stackblitz.com/edit/ngrx-upsert-bug

Version of affected browser(s),operating system(s), npm, node and ngrx:

ngrx store / entity 5.1.0

[MikeRyanDev]

This is an issue with our docs. You are correct that the type signature is Update<Entity>.

[miniplus]

Is this by design or by accident?

I figured I would just pass the entity to the API and it would be handled behind the scenes.
As the docs and medium blogpost do this as well, wouldn't it be the more logical thing to do as more people would expect it to function that way?

For now I agree it is a docs issue, however I think the API consumer shouldn't pass an Update<User> but instead a User.

#817 Seems to agree.

(Note: The Update object seems to be necessary here, but shouldn't, see #818 )

[fmalcher]

#817 Seems to agree.

I like the way the upsert is being described in the docs:

return adapter.upsertOne(myEntity, state);

IMO, upsert is an insert that silenty updates existing entities. The API for this should be closer to "add" than to "update". Using upsert with the current API will probably always look like this:

const update = { id: myEntity.id, changes: myEntity };
return adapter.upsertOne(update, state);

The Update object always has to be created manually, even though it always looks the same.
I'd really like to see the API in the first proposed way (the one that is in the docs).

Thank you!

[sandangel]

I have the same idea too. But there was a long discussion and everyone have agreed that upsert should have the same signature with update so I made PR following that. #421

[miniplus]

Docs fixed in 1ffb5a9.
I also believe just the Entity should be the arg instead of Update<Entity>, but let #817 carry on that discussion if needed, no use having 2 open issues for somewhat the same thing.

[Mathijs003]

I'm having an issue with the typing too. When an Object of get's updated in the state, the type is lost and it's just an Object

[Mathijs003]

@miniplus can you reopen this issue?