Skip to content

fix(signals): preserve entity when id changes number to equivalent string - #5219

Merged
timdeschryver merged 1 commit into
ngrx:mainfrom
adxdits:fix/signals-entity-id-string-number
Aug 31, 2026
Merged

timdeschryver merged 1 commit into
ngrx:mainfrom
adxdits:fix/signals-entity-id-string-number

Conversation

@adxdits

@adxdits adxdits commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

PR Checklist

#5218

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • 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?

In updateEntitiesMutably (used by updateEntity, updateEntities, and updateAllEntities), when an entity's ID is changed to a value that is a different type but the same object key (e.g. numeric 1 to string '1'), the entity is deleted instead of updated. This happens because the new key is written before the old key is deleted, and JavaScript treats 1 and '1' as the same object property, so the delete step removes the just-written entity.

Reproduction:

const Store = signalStore({ protectedState: false }, withEntities<Entity>());
const store = new Store();

patchState(store, addEntity({ id: 1, name: 'Ada' }));
patchState(store, updateEntity({ id: 1, changes: { id: '1' } }));

store.entityMap(); // {} entity is lost
store.entities();  // [undefined]

What is the new behavior?

The old entity key is deleted before the updated entity is assigned to the new key, so entities are preserved when the ID's string/number representation changes. entityMap, ids, and entities all stay consistent.

A regression test was added in update-entity.spec.ts covering the numeric-to-string ID update case.

Does this PR introduce a breaking change?

No

@netlify

netlify Bot commented Aug 31, 2026

Copy link
Copy Markdown

Deploy Preview for ngrx-io ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 63f156f
🔍 Latest deploy log https://app.netlify.com/projects/ngrx-io/deploys/6a956d6710b9ec0008e79696
😎 Deploy Preview https://deploy-preview-5219--ngrx-io.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@timdeschryver timdeschryver left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @adxdits

@timdeschryver
timdeschryver merged commit a797e4b into ngrx:main Aug 31, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants