diff --git a/.bundlemonrc.json b/.bundlemonrc.json index e80a03937..0833cf7b3 100644 --- a/.bundlemonrc.json +++ b/.bundlemonrc.json @@ -30,7 +30,7 @@ }, { "path": "./fesm2022/ngxs-store.mjs", - "maxSize": "103kB", + "maxSize": "105kB", "maxPercentIncrease": 0.5 } ], diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b867984c..2a5a7d953 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) - Fix(store): Complete action results on destroy [#2282](https://github.com/ngxs/store/pull/2282) - Refactor(form-plugin): Replace `takeUntil` with `takeUntilDestroyed` [#2283](https://github.com/ngxs/store/pull/2283) diff --git a/packages/store/src/pending-tasks.ts b/packages/store/src/pending-tasks.ts index f9e6c320d..0dc50c5ca 100644 --- a/packages/store/src/pending-tasks.ts +++ b/packages/store/src/pending-tasks.ts @@ -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'; @@ -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(); + + // 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 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; + } } - } - }); + }); }); } diff --git a/packages/store/tests/issues/pending-task-notifications.spec.ts b/packages/store/tests/issues/pending-task-notifications.spec.ts index 86657bdb0..1a983c7d6 100644 --- a/packages/store/tests/issues/pending-task-notifications.spec.ts +++ b/packages/store/tests/issues/pending-task-notifications.spec.ts @@ -45,7 +45,7 @@ describe('Pending task notifications', () => { }; it( - 'should call `scheduler.notify()` more than 20 times', + 'should NOT call `scheduler.notify()` more than 20 times (because of 20 actions)', freshPlatform(async () => { // Arrange const notifySpy = jest.spyOn(ɵChangeDetectionSchedulerImpl.prototype, 'notify'); @@ -53,8 +53,7 @@ describe('Pending task notifications', () => { await skipConsoleLogging(() => bootstrapApplication(TestComponent, appConfig)); try { // Assert - // `notify` has been called more than 20 times because we dispatched 20 times. - expect(notifySpy.mock.calls.length > 20).toBeTruthy(); + expect(notifySpy).toHaveBeenCalledTimes(2); } finally { notifySpy.mockRestore(); }