Skip to content

Commit

Permalink
fix(store-devtools): reduce CD cycles by listening message outside …
Browse files Browse the repository at this point in the history
…of Angular

This commit updates the extension connection setup and wraps it with `runOutsideAngular`
to prevent running change detection too frequently. This ensures that change detection is
only triggered when all asynchronous actions have been processed.

Closes #3839
  • Loading branch information
arturovt committed Jul 24, 2023
1 parent 644f0b6 commit 9a9d966
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 21 deletions.
51 changes: 36 additions & 15 deletions modules/store-devtools/spec/extension.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ function createState(
};
}

const ngZone = {
runOutsideAngular<T>(fn: () => T) {
return fn();
},
} as any;

describe('DevtoolsExtension', () => {
let reduxDevtoolsExtension: ReduxDevtoolsExtension;
let extensionConnection: ReduxDevtoolsExtensionConnection;
Expand Down Expand Up @@ -106,7 +112,8 @@ describe('DevtoolsExtension', () => {
devtoolsExtension = new DevtoolsExtension(
reduxDevtoolsExtension,
createConfig({}),
<any>null
<any>null,
ngZone
);
// Subscription needed or else extension connection will not be established.
devtoolsExtension.actions$.subscribe(() => null);
Expand All @@ -128,7 +135,8 @@ describe('DevtoolsExtension', () => {
trace: true,
traceLimit: 20,
}),
<any>null
<any>null,
ngZone
);
// Subscription needed or else extension connection will not be established.
devtoolsExtension.actions$.subscribe(() => null);
Expand All @@ -155,7 +163,8 @@ describe('DevtoolsExtension', () => {
name: 'ngrx-store-devtool-todolist',
serialize: customSerializer,
}),
<any>null
<any>null,
ngZone
);
// Subscription needed or else extension connection will not be established.
devtoolsExtension.actions$.subscribe(() => null);
Expand All @@ -169,7 +178,8 @@ describe('DevtoolsExtension', () => {
devtoolsExtension = new DevtoolsExtension(
reduxDevtoolsExtension,
createConfig({}),
<any>null
<any>null,
ngZone
);
const defaultOptions = createOptions();
const action = {} as LiftedAction;
Expand All @@ -193,7 +203,8 @@ describe('DevtoolsExtension', () => {
actionSanitizer: myActionSanitizer,
stateSanitizer: myStateSanitizer,
}),
<any>null
<any>null,
ngZone
);

const options = createOptions(
Expand Down Expand Up @@ -234,7 +245,8 @@ describe('DevtoolsExtension', () => {
devtoolsExtension = new DevtoolsExtension(
reduxDevtoolsExtension,
createConfig({}),
<any>null
<any>null,
ngZone
);
// Subscription needed or else extension connection will not be established.
devtoolsExtension.actions$.subscribe(() => null);
Expand Down Expand Up @@ -272,7 +284,8 @@ describe('DevtoolsExtension', () => {
createConfig({
actionSanitizer: testActionSanitizer,
}),
<any>null
<any>null,
ngZone
);
// Subscription needed or else extension connection will not be established.
devtoolsExtension.actions$.subscribe(() => null);
Expand Down Expand Up @@ -318,7 +331,8 @@ describe('DevtoolsExtension', () => {
createConfig({
stateSanitizer: testStateSanitizer,
}),
<any>null
<any>null,
ngZone
);
// Subscription needed or else extension connection will not be established.
devtoolsExtension.actions$.subscribe(() => null);
Expand Down Expand Up @@ -363,7 +377,8 @@ describe('DevtoolsExtension', () => {
actionSanitizer: testActionSanitizer,
stateSanitizer: testStateSanitizer,
}),
<any>null
<any>null,
ngZone
);
// Subscription needed or else extension connection will not be established.
devtoolsExtension.actions$.subscribe(() => null);
Expand Down Expand Up @@ -400,7 +415,8 @@ describe('DevtoolsExtension', () => {
createConfig({
actionsBlocklist: [BLOCKED_ACTION_1, BLOCKED_ACTION_2],
}),
<any>null
<any>null,
ngZone
);
// Subscription needed or else extension connection will not be established.
devtoolsExtension.actions$.subscribe(() => null);
Expand Down Expand Up @@ -442,7 +458,8 @@ describe('DevtoolsExtension', () => {
createConfig({
actionsSafelist: [SAFE_ACTION_1, SAFE_ACTION_2],
}),
<any>null
<any>null,
ngZone
);
// Subscription needed or else extension connection will not be established.
devtoolsExtension.actions$.subscribe(() => null);
Expand Down Expand Up @@ -492,7 +509,8 @@ describe('DevtoolsExtension', () => {
createConfig({
predicate,
}),
<any>null
<any>null,
ngZone
);
// Subscription needed or else extension connection will not be established.
devtoolsExtension.actions$.subscribe(() => null);
Expand Down Expand Up @@ -528,7 +546,8 @@ describe('DevtoolsExtension', () => {
devtoolsExtension = new DevtoolsExtension(
reduxDevtoolsExtension,
createConfig({}),
<any>null
<any>null,
ngZone
);
// Subscription needed or else extension connection will not be established.
devtoolsExtension.actions$.subscribe(() => null);
Expand Down Expand Up @@ -563,7 +582,8 @@ describe('DevtoolsExtension', () => {
devtoolsExtension = new DevtoolsExtension(
reduxDevtoolsExtension,
createConfig({}),
<any>null
<any>null,
ngZone
);
// Subscription needed or else extension connection will not be established.
devtoolsExtension.actions$.subscribe(() => null);
Expand Down Expand Up @@ -600,7 +620,8 @@ describe('DevtoolsExtension', () => {
devtoolsExtension = new DevtoolsExtension(
reduxDevtoolsExtension,
createConfig({}),
<any>null
<any>null,
ngZone
);
// Subscription needed or else extension connection will not be established.
devtoolsExtension.actions$.subscribe();
Expand Down
34 changes: 32 additions & 2 deletions modules/store-devtools/src/devtools.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import { Injectable, Inject, ErrorHandler, OnDestroy } from '@angular/core';
import {
Injectable,
Inject,
ErrorHandler,
NgZone,
OnDestroy,
inject,
} from '@angular/core';
import { toSignal } from '@angular/core/rxjs-interop';
import {
Action,
Expand All @@ -11,6 +18,7 @@ import {
} from '@ngrx/store';
import {
merge,
MonoTypeOperatorFunction,
Observable,
Observer,
queueScheduler,
Expand Down Expand Up @@ -69,11 +77,22 @@ export class StoreDevtools implements Observer<any>, OnDestroy {

const liftedReducer$ = reducers$.pipe(map(liftReducer));

// Using `inject` function instead of the constructor parameter to prevent breaking
// changes if the `StoreDevtools` is being extended by consumers.
const ngZone = inject(NgZone);

const liftedStateSubject = new ReplaySubject<LiftedState>(1);

this.liftedStateSubscription = liftedAction$
.pipe(
withLatestFrom(liftedReducer$),
// The extension would post messages back outside of the Angular zone
// because we call `connect()` wrapped with `runOutsideAngular`. We run change
// detection only once at the end after all the required asynchronous tasks have
// been processed (for instance, `setInterval` scheduled by the `timeout` operator).
// We have to re-enter the Angular zone before the `scan` since it runs the reducer
// which must be run within the Angular zone.
enterNgZone(ngZone),
scan<
[any, ActionReducer<LiftedState, Actions.All>],
{
Expand Down Expand Up @@ -111,7 +130,7 @@ export class StoreDevtools implements Observer<any>, OnDestroy {
});

this.extensionStartSubscription = extension.start$.subscribe(() => {
this.refresh();
ngZone.run(() => this.refresh());
});

const liftedState$ =
Expand Down Expand Up @@ -196,3 +215,14 @@ export class StoreDevtools implements Observer<any>, OnDestroy {
this.dispatch(new Actions.PauseRecording(status));
}
}

function enterNgZone<T>(ngZone: NgZone): MonoTypeOperatorFunction<T> {
return (source: Observable<T>) =>
new Observable<T>((subscriber) =>
source.subscribe({
next: (value) => ngZone.run(() => subscriber.next(value)),
error: (error) => ngZone.run(() => subscriber.error(error)),
complete: () => ngZone.run(() => subscriber.complete()),
})
);
}
15 changes: 11 additions & 4 deletions modules/store-devtools/src/extension.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Inject, Injectable, InjectionToken } from '@angular/core';
import { Inject, Injectable, InjectionToken, NgZone } from '@angular/core';
import { Action, UPDATE } from '@ngrx/store';
import { EMPTY, Observable, of } from 'rxjs';
import {
Expand Down Expand Up @@ -80,7 +80,8 @@ export class DevtoolsExtension {
constructor(
@Inject(REDUX_DEVTOOLS_EXTENSION) devtoolsExtension: ReduxDevtoolsExtension,
@Inject(STORE_DEVTOOLS_CONFIG) private config: StoreDevtoolsConfig,
private dispatcher: DevtoolsDispatcher
private dispatcher: DevtoolsDispatcher,
private ngZone: NgZone
) {
this.devtoolsExtension = devtoolsExtension;
this.createActionStreams();
Expand Down Expand Up @@ -168,8 +169,14 @@ export class DevtoolsExtension {
}

return new Observable((subscriber) => {
const connection = this.devtoolsExtension.connect(
this.getExtensionConfig(this.config)
// To reduce change detection cycles, we need to run the `connect` method
// outside of the Angular zone. The `connect` method adds a `message`
// event listener to communicate with an extension using `window.postMessage`
// and handle message events.
// We will re-enter the Angular zone when handling specific actions from the
// extension and when dispatching actions through `store.dispatch`.
const connection = this.ngZone.runOutsideAngular(() =>
this.devtoolsExtension.connect(this.getExtensionConfig(this.config))
);
this.extensionConnection = connection;
connection.init();
Expand Down

0 comments on commit 9a9d966

Please sign in to comment.