From 9a8bd8e35a951100fb0d1e943a86a4d48a0ba86b Mon Sep 17 00:00:00 2001 From: arturovt Date: Sun, 22 Dec 2024 14:35:06 +0200 Subject: [PATCH] fix(store): complete action results on destroy In this commit, we perform some minor refactoring by replacing the implementation of the `OnDestroy` interface with the use of `ApplicationRef` in the constructor to avoid creating redundant methods. Additionally, we complete the `InternalDispatchedActionResults` once the application is destroyed to ensure there are no active subscribers after resources have been cleaned up. --- CHANGELOG.md | 1 + packages/store/internals/src/state-stream.ts | 18 ++++++++------- packages/store/src/actions-stream.ts | 12 ++++++---- packages/store/src/internal/action-results.ts | 23 +++++++++++++++++++ packages/store/src/internal/dispatcher.ts | 16 ++++--------- packages/store/src/internal/state-factory.ts | 2 +- 6 files changed, 46 insertions(+), 26 deletions(-) create mode 100644 packages/store/src/internal/action-results.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ec8af6a5..72a6946a8 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): Complete action results on destroy [#2282](https://github.com/ngxs/store/pull/2282) ### 19.0.0 2024-12-3 diff --git a/packages/store/internals/src/state-stream.ts b/packages/store/internals/src/state-stream.ts index 39da83a1f..2f6e9242d 100644 --- a/packages/store/internals/src/state-stream.ts +++ b/packages/store/internals/src/state-stream.ts @@ -1,4 +1,4 @@ -import { Injectable, OnDestroy, Signal, untracked } from '@angular/core'; +import { ApplicationRef, inject, Injectable, Signal, untracked } from '@angular/core'; import { toSignal } from '@angular/core/rxjs-interop'; import { ɵwrapObserverCalls } from './custom-rxjs-operators'; @@ -10,7 +10,7 @@ import { ɵPlainObject } from './symbols'; * @ignore */ @Injectable({ providedIn: 'root' }) -export class ɵStateStream extends ɵOrderedBehaviorSubject<ɵPlainObject> implements OnDestroy { +export class ɵStateStream extends ɵOrderedBehaviorSubject<ɵPlainObject> { readonly state: Signal<ɵPlainObject> = toSignal(this.pipe(ɵwrapObserverCalls(untracked)), { manualCleanup: true, requireSync: true @@ -18,14 +18,16 @@ export class ɵStateStream extends ɵOrderedBehaviorSubject<ɵPlainObject> imple constructor() { super({}); - } - ngOnDestroy(): void { - // The StateStream should never emit values once the root view is removed, - // such as when the `NgModuleRef.destroy()` method is called. This is crucial - // for preventing memory leaks in server-side rendered apps, where a new StateStream + // Complete the subject once the root injector is destroyed to ensure + // there are no active subscribers that would receive events or perform + // any actions after the application is destroyed. + // The `StateStream` should never emit values once the root view is removed, + // such as when the `ApplicationRef.destroy()` method is called. This is crucial + // for preventing memory leaks in server-side rendered apps, where a new `StateStream` // is created for each HTTP request. If users forget to unsubscribe from `store.select` // or `store.subscribe`, it can result in significant memory leaks in SSR apps. - this.complete(); + const appRef = inject(ApplicationRef); + appRef.onDestroy(() => this.complete()); } } diff --git a/packages/store/src/actions-stream.ts b/packages/store/src/actions-stream.ts index b33cbace5..48bb5c526 100644 --- a/packages/store/src/actions-stream.ts +++ b/packages/store/src/actions-stream.ts @@ -1,4 +1,4 @@ -import { inject, Injectable, OnDestroy } from '@angular/core'; +import { ApplicationRef, inject, Injectable } from '@angular/core'; import { ɵOrderedSubject } from '@ngxs/store/internals'; import { Observable, Subject, filter, share } from 'rxjs'; @@ -25,7 +25,7 @@ export interface ActionContext { * Internal Action stream that is emitted anytime an action is dispatched. */ @Injectable({ providedIn: 'root' }) -export class InternalActions extends ɵOrderedSubject implements OnDestroy { +export class InternalActions extends ɵOrderedSubject { // This subject will be the first to know about the dispatched action, its purpose is for // any logic that must be executed before action handlers are invoked (i.e., cancelation). readonly dispatched$ = new Subject(); @@ -36,10 +36,12 @@ export class InternalActions extends ɵOrderedSubject implements this.pipe(filter(ctx => ctx.status === ActionStatus.Dispatched)).subscribe(ctx => { this.dispatched$.next(ctx); }); - } - ngOnDestroy(): void { - this.complete(); + // Complete the subject once the root injector is destroyed to ensure + // there are no active subscribers that would receive events or perform + // any actions after the application is destroyed. + const appRef = inject(ApplicationRef); + appRef.onDestroy(() => this.complete()); } } diff --git a/packages/store/src/internal/action-results.ts b/packages/store/src/internal/action-results.ts new file mode 100644 index 000000000..4e5732e85 --- /dev/null +++ b/packages/store/src/internal/action-results.ts @@ -0,0 +1,23 @@ +import { ApplicationRef, inject, Injectable } from '@angular/core'; +import { Subject } from 'rxjs'; + +import type { ActionContext } from '../actions-stream'; + +/** + * Internal Action result stream that is emitted when an action is completed. + * This is used as a method of returning the action result to the dispatcher + * for the observable returned by the dispatch(...) call. + * The dispatcher then asynchronously pushes the result from this stream onto the main action stream as a result. + */ +@Injectable({ providedIn: 'root' }) +export class InternalDispatchedActionResults extends Subject { + constructor() { + super(); + + // Complete the subject once the root injector is destroyed to ensure + // there are no active subscribers that would receive events or perform + // any actions after the application is destroyed. + const appRef = inject(ApplicationRef); + appRef.onDestroy(() => this.complete()); + } +} diff --git a/packages/store/src/internal/dispatcher.ts b/packages/store/src/internal/dispatcher.ts index 7ea1edfbf..15c578a24 100644 --- a/packages/store/src/internal/dispatcher.ts +++ b/packages/store/src/internal/dispatcher.ts @@ -1,24 +1,16 @@ import { inject, Injectable, Injector, NgZone, runInInjectionContext } from '@angular/core'; -import { forkJoin, Observable, of, Subject, throwError } from 'rxjs'; +import { forkJoin, Observable, of, throwError } from 'rxjs'; import { filter, map, mergeMap, shareReplay, take } from 'rxjs/operators'; import { getActionTypeFromInstance } from '@ngxs/store/plugins'; import { ɵPlainObject, ɵStateStream } from '@ngxs/store/internals'; -import { ActionContext, ActionStatus, InternalActions } from '../actions-stream'; import { PluginManager } from '../plugin-manager'; -import { InternalNgxsExecutionStrategy } from '../execution/internal-ngxs-execution-strategy'; import { leaveNgxs } from '../operators/leave-ngxs'; import { fallbackSubscriber } from './fallback-subscriber'; - -/** - * Internal Action result stream that is emitted when an action is completed. - * This is used as a method of returning the action result to the dispatcher - * for the observable returned by the dispatch(...) call. - * The dispatcher then asynchronously pushes the result from this stream onto the main action stream as a result. - */ -@Injectable({ providedIn: 'root' }) -export class InternalDispatchedActionResults extends Subject {} +import { InternalDispatchedActionResults } from './action-results'; +import { ActionContext, ActionStatus, InternalActions } from '../actions-stream'; +import { InternalNgxsExecutionStrategy } from '../execution/internal-ngxs-execution-strategy'; @Injectable({ providedIn: 'root' }) export class InternalDispatcher { diff --git a/packages/store/src/internal/state-factory.ts b/packages/store/src/internal/state-factory.ts index 7da192221..1629c61af 100644 --- a/packages/store/src/internal/state-factory.ts +++ b/packages/store/src/internal/state-factory.ts @@ -42,7 +42,7 @@ import { } from './internals'; import { NgxsActionRegistry } from '../actions/action-registry'; import { ActionContext, ActionStatus, InternalActions } from '../actions-stream'; -import { InternalDispatchedActionResults } from '../internal/dispatcher'; +import { InternalDispatchedActionResults } from '../internal/action-results'; import { ensureStateNameIsUnique, ensureStatesAreDecorated } from '../utils/store-validators'; import { ensureStateClassIsInjectable } from '../ivy/ivy-enabled-in-dev-mode'; import { NgxsUnhandledActionsLogger } from '../dev-features/ngxs-unhandled-actions-logger';