NGRX selectors updating too often

Question

I am really confused, I really thought that selectors would not run if all of his parents returned the same result.

At a time, there is 1-250 clusterMarker selector active, each with a different prop, cluster. Its execution is rather expensive. I made sure, that it needs to be reevaluated only if the result of any of its arguments changes.

Simplified example:

const offerState = createFeatureSelector<OfferState>('offer');
const currentCarrier = createSelector(offerState, state => state.currentCarrier);
const currentContext = createSelector(offerState, state => state.currentContext);
const currentPeriod = createSelector(offerState, state => state.currentPeriod);
const filteredCarriers = createSelector(offerState, state => state.filteredCarriers);
const wanted = createSelector(offerState, state => state.wanted);

const filteredCarriersSet = createSelector(
    filteredCarriers,
    carriers => new Set(carriers),
);

/**
 * Only fire if the changes to state affect this context.
 * `undefined` => `state.currentCarrier`
 * `state.currentCarrier` => `undefined`
 */
const currentCarrierInCluster = createSelector(
    currentCarrier,
    currentContext,
    (
        currentCarrier: Carrier | null,
        currentContext: AbstractMapClusterContext<Carrier> | null,
        { cluster }: { cluster: AbstractMapClusterContext<Carrier> }
    ) => currentContext == cluster ? currentCarrier : undefined,
);

export const clusterMarker = createSelector(
    filteredCarriersSet,
    currentCarrierInCluster,
    currentPeriod,
    wanted,
    (
        filteredSet,
        currentCarrier,
        currentPeriod,
        wanted,
        { cluster }: { cluster: AbstractMapClusterContext<Carrier> }
    ) => {
        // ... code ...
    },
);

Is there a part of the documentation about setting memorization options I missed? What can I do, to make this more performant?

Response to answer:

Code:

export const clusterMarkerSelectorFactory = () =>  createSelector(
    filteredCarriersSet,
    currentCarrierInCluster,
    currentPeriod,
    wanted,
    (
        filteredSet,
        currentCarrier,
        currentPeriod,
        wanted,
        { cluster }: { cluster: AbstractMapClusterContext<Carrier> }
    ) => {
        // ... code ...
    },
);

class Component {
    constructor(
        private store$: Store<OfferState>,
    ) { }

    readonly state$ = this.cluster$.pipe(
        switchMap(cluster => this.store$.select(clusterMarkerSelectorFactory(), { cluster })),
    );
}

This will still retrigger for every one of them.

Answer 1

I now know why this happens, I have also had several workarounds. It all is tied to the fact that whenever a selector is called with parameters, all of its parents will be called with parameters too! I have created a GitHub issue on this.

tldr;

Original:

// replace
const elementSelectorFactory = () => createSelector(
    listAsSet,
    (set, { element }) => set.has(element),
);

Fixed v1:

// with (This needs to be done only for the first level of selectors, more info bellow)
const elementSelectorFactory = () => createSelector(
    state => listAsSet(state as any),
    (set, { element }) => set.has(element),
);

Fixed v2:

// or with
const elementSelectorFactory = element => createSelector(
    listAsSet,
    set => set.has(element),
);

// or in case you have nested props selectors (my case)
const elementSelectorFactory = element => createSelector(
    listAsSet,
    nestedSelectorFactory(element),
    (set, nested) => set.has(element) ? nested : null,
);

---

I will walk you through a simple selector with props.

We have code bellow

interface Store {
  elements: unknown[];
}

export const offerState = createFeatureSelector<Store>('store');

const listAsSet = createSelector(
    state => state.list,
    carriers => new Set(carriers),
);

// We create a factory like they instructed us in the docs
const elementSelectorFactory = () => createSelector(
    listAsSet,
    (set, { element }) => set.has(element),
);

Now if we call this.store$.select(elementSelectorFactory(), { element }).

1. We create the elementSelector
2. This selector will ask for data from its parent with the following parameters: [state, { element }].
3. listAsSet gets called with state and props { element }, even though it does not use the props.

Now, if you were to create 2 elementSelector from the factories, step 3 will be called twice, with 2 different props! Since its arguments are not equal it must be recalculated. Set() of the same elements are not equal, thus this will invalidate arguments of our elementSelector!

What is worse, if you are using the listAsSet selector somewhere else without props, it would get invalidated too!

This was my first solution to this:

const elementSelectorFactory = () => createSelector(
    // Here we don't pass the props to the child selector, to prevent invalidating its cache
    (state, _props) => listAsSet(state as any),
    (set, { element }) => set.has(element),
);

The next solution is not to use props at all, as suggested by ngrx member...

// or with
const elementSelectorFactory = element => createSelector(
    listAsSet,
    set => set.has(element),
);

Here, the selector will never receive props, to begin with... If you want to have nested selector you will have to rewrite them all to selector factories with parameter(s) to combat this.

const elementSelectorFactory = element => createSelector(
    listAsSet,
    nestedSelectorFactory(element),
    (set, nested) => set.has(element) ? nested : null,
);

Answer 2

Memoization only remembers the single last calculated value when all inputs are the same.

In your example, currentCarrierInCluster is called with different a cluster parameter (though it is not clear how from your syntax). This renders the memoization ineffective.

Memoization works by comparing all inputs to their equivalent values the last time the selector was invoked. If they are the same, the cached value is returned, if they are different a new value is calculated via the selector's projection function.

In your case the cluster input is different, hence the projection function runs again.

You can address this by wrapping the selector inside a factory function to create different instances of the selector for each cluster input value:

const currentCarrierInCluster = (cluster: ClusterType) => createSelector(
    currentCarrier,
    currentContext,
    (
        currentCarrier: Carrier | null,
        currentContext: AbstractMapClusterContext<Carrier> | null,
        { cluster }: { cluster: AbstractMapClusterContext<Carrier> }
    ) => currentContext == cluster ? currentCarrier : undefined,
);

which can be called like:

cluster$ = this.store.select(currentCarrierInCluster(cluster));

References
https://ngrx.io/guide/store/selectors
https://stackoverflow.com/a/50375970/1188074

Further Implementation Info

NgRx Selector source code is here

Refer to the call to isArgumentsChanged on line 102 which determines if the input values have changed and, therefore whether to return the memoized result. It effectively calls the following method on each argument:

export function isEqualCheck(a: any, b: any): boolean {
  return a === b;
}

This can be overridden but would only be done in exceptional cases,

Changes required to your selector

// Note = after clusterMarker rather than after =>
export const clusterMarker = (_cluster: AbstractMapClusterContext<Carrier>) => createSelector(
    filteredCarriersSet,
    currentCarrierInCluster,
    currentPeriod,
    wanted,
    (
        filteredSet,
        currentCarrier,
        currentPeriod,
        wanted,
        /* Remove this and use _cluster instead
       { cluster }: { cluster: AbstractMapClusterContext<Carrier> } */
    ) => {
        // ... code ... 
    },
);

Call like:

this.store$.select(clusterMarker(cluster))