fix(signals): allow modifying entity id on update

[markostanimirovic]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/ngrx/platform/blob/main/CONTRIBUTING.md#commit
• Tests for the changes have been added (for bug fixes / features)
• Documentation has been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

It's not possible to modify entity ID via update* functions. Instead, it's necessary to remove the existing one and add a new entity with an updated ID:

const myEntity = { ...entityMap()['tmp_id'], id: 'real_id' };

patchState(store, removeEntity('tmp_id'), addEntity(myEntity));

Closes #4235

What is the new behavior?

The update* updaters can be used to modify entity ID:

patchState(store, updateEntity({ id: 'tmp_id', changes: { id: 'real_id' } }));

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

After this change, it's necessary to provide selectId to all update* updaters if an entity has a custom ID.

type Todo = { _id: string; text: string };

+const selectId: SelectEntityId<Todo> = (todo) => todo._id;

const TodosStore = signalStore(
  withEntities<Todo>(),
  withMethods((store) => ({
    updateTodo(id: string, changes: Partial<Todo>): void {
-      patchState(store, updateEntity({ id, changes }));
+      patchState(store, updateEntity({ id, changes }, { selectId }));
    },
  }))
);

This is not considered as a breaking change because the @ngrx/signals package is in developer preview.

[netlify]

✅ Deploy Preview for ngrx-io canceled.

Name	Link
🔨 Latest commit	9990abd
🔍 Latest deploy log	https://app.netlify.com/sites/ngrx-io/deploys/668043d1d1f6b00009655a6d

[timdeschryver]

With the current implementation, the ids can get out of sync with the entity keys. This happens when we update an entity with an existing id.
Is this expected? If so, I think adding a warning can be useful.

  it('updates entity ids', () => {
    const Store = signalStore(withEntities<User>());
    const store = new Store();

    patchState(
      store,
      addEntities([user1, user2]),
      updateEntities({
        ids: [user1.id],
        changes: ({ id }) => ({ id: user2.id, firstName: `Updated Jimmy` }),
      }),
    );

    // 👇 only has user 2
    expect(store.entityMap()).toEqual({
      [user2.id]: { ...user1, id: user2.id, firstName: 'Updated Jimmy' },
    });

    // 👇 user id 2 is added twice
    expect(store.ids()).toEqual([user2.id, user2.id]);
  });

[markostanimirovic]

@timdeschryver With allowing ID changes there are several ways to break/move entity collection to the inconsistent state. That's the reason why I was against this change initially. Users now have better control, but also responsibility.

Let's check how @ngrx/entity handles this case. It probably works the same.

I agree that warning will be useful 👍

[markostanimirovic]

I just checked @ngrx/entity - the behavior is the same. Let's add a warning then.