@ngrx/data: Changing an entity causes other entities' selectors to emit

ngrx / platform

Reactive State for Angular

Other

8.04k stars 1.97k forks source link

Which @ngrx/* package(s) are the source of the bug?

data, entity

Minimal reproduction of the bug/regression with instructions

https://stackblitz.com/edit/stackblitz-starters-qfsbzb?file=src%2Fmain.ts

There are 2 entities: Test, Other.

Clicking "Add to other" adds another item to the "Other" entities. However, it also causes any selectors created with new EntitySelectorsFactory().create('Test') to emit changes, even though there are no changes in that entity. The changelist is even empty.

Subscriptions of this.entityServices.getEntityCollectionService('Test').entities$ do not emit the change.

Expected behavior

I am using custom entity selectors extensively, and this causes many unnecessary selector emissions that are hard to catch later on. It is expected that using this.store.select in a native entity selector should prevent emissions with same object references (since it uses distinctUntilChanged().

When an entity changes, other entity selectors should not reflect any changes (they should return the same object reference), since their state slice has not changed at all.

Versions of NgRx, Angular, Node, affected browser(s) and operating system(s)

NgRx: 16.1.0 Angular: 16.1.4 Node: 18.16.0 Browser: Chrome 115 Operating System: Windows 11

Other information

I have tried to trace the issue but I cannot find the cause of it. I suspect the entity state adapter (https://github.com/ngrx/platform/blob/main/modules/entity/src/state_adapter.ts#L12), although I am not sure how it affects other entities.

I would be willing to submit a PR to fix this issue

[X] Yes
[ ] No

I've noticed the following: unexpected emits for Test fire until first Test is actually created. This allowed me to track the problem down to this:

  createCollectionSelector<
    T = any,
    C extends EntityCollection<T> = EntityCollection<T>
  >(entityName: string) {
    const getCollection = (cache: EntityCache = {}) =>
      <C>(
        (cache[entityName] ||
          this.entityCollectionCreator.create<T>(entityName))
      );
    return createSelector(this.selectEntityCache, getCollection);
  }

This is a factory function for collection selector which is used as a base for everything else, including selectEntities, selectChangeState, etc.

As you can see, if there is initialized cache for the specified entity, it is used as is. But if no cache is initialized yet, new one is created -- and not saved anywhere, which leads to creation of new cache object every time (and consequentially it looks brand new).

A quick fix would look like this:

        (cache[entityName] ||
          (cache[entityName] = this.entityCollectionCreator.create<T>(entityName)))

But I'm not entirely sure how robust this solution is, because cache is mutated, not recreated which might mess things up. To properly recreate it, we need a reference to parent object (which seems to be <state-root>.entityCache).

On the other hand, this would be a huge side effect in a selector, so, maybe pre-creating all caches would be a better solution? Not sure how feasible this change would be for NgRx code, but at least you can implement it in your code by modifying provideStore(...) call like this:

    provideStore(reducers, {
      metaReducers: metaReducers,
      initialState: {
        top: 0,
        [ENTITY_CACHE_NAME]: {
          Test: new EntityCollectionCreator().create('Test'),
          Other: new EntityCollectionCreator().create('Other'),
        }
      } as any,
    }),

ngrx / platform