Skip to content

Commit

Permalink
fix(store): reduce change detection cycles with pending tasks
Browse files Browse the repository at this point in the history
An `INFINITE_CHANGE_DETECTION` error has been logged to the error service multiple times,
occurring randomly depending on the number of actions dispatched in a row. This happens
because a pending task is added every time an action is dispatched. Removing a pending task
via the public API forces a scheduled tick, ensuring that stability is asynchronous and
delayed until there has been at least an opportunity to run app synchronization.

This change reduces the number of change detection cycles. For example, if 10 synchronous
actions are dispatched in a row, it may previously trigger 10 change detection cycles, as
tasks would be removed 10 times.

We listen to the actions stream, and every time a context with a `dispatched` status is
generated, we add a pending task only once and keep it until the action is completed.
If multiple actions are dispatched at the same time, we debounce them and use `buffer`
to collect them into a single list.
  • Loading branch information
arturovt committed Dec 20, 2024
1 parent 4499a53 commit 477e98e
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 16 deletions.
2 changes: 1 addition & 1 deletion .bundlemonrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
},
{
"path": "./fesm2022/ngxs-store.mjs",
"maxSize": "103kB",
"maxSize": "105kB",
"maxPercentIncrease": 0.5
}
],
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ $ npm install @ngxs/store@dev
### To become next patch version

- Fix(store): Add root store initializer guard [#2278](https://github.com/ngxs/store/pull/2278)
- Fix(store): Reduce change detection cycles with pending tasks [#2280](https://github.com/ngxs/store/pull/2280)

### 19.0.0 2024-12-3

Expand Down
68 changes: 53 additions & 15 deletions packages/store/src/pending-tasks.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { inject, PendingTasks } from '@angular/core';
import { DestroyRef, inject, PendingTasks } from '@angular/core';
import { buffer, debounceTime, filter } from 'rxjs';

import { Actions, ActionStatus } from './actions-stream';
import { withNgxsPreboot } from './standalone-features/preboot';
Expand All @@ -13,22 +14,59 @@ import { withNgxsPreboot } from './standalone-features/preboot';
*/
export function withNgxsPendingTasks() {
return withNgxsPreboot(() => {
const pendingTasks = inject(PendingTasks);
const actions$ = inject(Actions);
const destroyRef = inject(DestroyRef);
const pendingTasks = inject(PendingTasks);

// Removing a pending task via the public API forces a scheduled tick, ensuring that
// stability is async and delayed until there was at least an opportunity to run
// app synchronization.
// Adding a new task every time an action is dispatched drastically increases the
// number of change detection cycles because removing a task schedules a new change
// detection cycle.
// If 10 actions are dispatched with synchronous action handlers, this would trigger
// 10 change detection cycles in a row, potentially leading to an
// `INFINITE_CHANGE_DETECTION` error.
let removeTask: VoidFunction | null = null;

const executedActions = new Set<unknown>();

// If the app is forcely destroyed before all actions are completed,
// we clean up the set of actions being executed to prevent memory leaks
// and remove the pending task to stabilize the app.
destroyRef.onDestroy(() => executedActions.clear());

actions$
.pipe(
filter(context => {
if (context.status === ActionStatus.Dispatched) {
executedActions.add(context.action);
removeTask ||= pendingTasks.add();
return false;
} else {
return true;
}
}),
// Every time an action is completed, we debounce the stream to ensure only one
// task is removed, even if multiple synchronous actions are completed in a row.
// We use `buffer` to collect action contexts because, if we only use
// `debounceTime(0)`, we may lose action contexts that are never removed from the set.
buffer(actions$.pipe(debounceTime(0)))
)
.subscribe(contexts => {
for (const context of contexts) {
if (!executedActions.has(context.action)) {
continue;
}

executedActions.delete(context.action);

const actionToRemoveTaskFnMap = new Map<any, () => void>();

actions$.subscribe(ctx => {
if (ctx.status === ActionStatus.Dispatched) {
const removeTaskFn = pendingTasks.add();
actionToRemoveTaskFnMap.set(ctx.action, removeTaskFn);
} else {
const removeTaskFn = actionToRemoveTaskFnMap.get(ctx.action);
if (typeof removeTaskFn === 'function') {
actionToRemoveTaskFnMap.delete(ctx.action);
removeTaskFn();
// Mark app as stable once all of the debounced actions have completed.
if (executedActions.size === 0) {
removeTask?.();
removeTask = null;
}
}
}
});
});
});
}

0 comments on commit 477e98e

Please sign in to comment.