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 27, 2023
1 parent 644f0b6 commit 6e181f8
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 24 deletions.
5 changes: 5 additions & 0 deletions modules/store-devtools/spec/config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ describe('StoreDevtoolsOptions', () => {
features: defaultFeatures,
trace: false,
traceLimit: 75,
ngZone: 'enabled',
});
});

Expand Down Expand Up @@ -67,6 +68,7 @@ describe('StoreDevtoolsOptions', () => {
features: {
test: true,
},
ngZone: 'enabled',
});
});

Expand All @@ -84,6 +86,7 @@ describe('StoreDevtoolsOptions', () => {
trace: false,
traceLimit: 75,
features: defaultFeatures,
ngZone: 'enabled',
});
});

Expand All @@ -107,6 +110,7 @@ describe('StoreDevtoolsOptions', () => {
export: true,
test: true,
},
ngZone: 'enabled',
});
});

Expand All @@ -133,6 +137,7 @@ describe('StoreDevtoolsOptions', () => {
},
trace: false,
traceLimit: 75,
ngZone: 'enabled',
});
});
});
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
8 changes: 8 additions & 0 deletions modules/store-devtools/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,13 @@ export class StoreDevtoolsConfig {
* Maximum stack trace frames to be stored (in case trace option was provided as true).
*/
traceLimit?: number;

/**
* The property determines whether the extension connection is established within the
* Angular zone or not. This is set to `enabled` by default, which means the connection
* is set up within the Angular zone by default to ensure backward compatibility.
*/
ngZone?: 'enabled' | 'noop';
}

export const STORE_DEVTOOLS_CONFIG = new InjectionToken<StoreDevtoolsConfig>(
Expand Down Expand Up @@ -165,6 +172,7 @@ export function createConfig(
dispatch: true, // Dispatch custom actions or action creators
test: true, // Generate tests for the selected actions
},
ngZone: 'enabled',
};

const options =
Expand Down
53 changes: 49 additions & 4 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,23 @@ 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 ngZoneEnabled = config.ngZone === 'enabled';

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(ngZoneEnabled, ngZone),
scan<
[any, ActionReducer<LiftedState, Actions.All>],
{
Expand Down Expand Up @@ -110,9 +130,11 @@ export class StoreDevtools implements Observer<any>, OnDestroy {
}
});

this.extensionStartSubscription = extension.start$.subscribe(() => {
this.refresh();
});
this.extensionStartSubscription = extension.start$
.pipe(enterNgZone(ngZoneEnabled, ngZone))
.subscribe(() => {
this.refresh();
});

const liftedState$ =
liftedStateSubject.asObservable() as Observable<LiftedState>;
Expand Down Expand Up @@ -196,3 +218,26 @@ export class StoreDevtools implements Observer<any>, OnDestroy {
this.dispatch(new Actions.PauseRecording(status));
}
}

function enterNgZone<T>(
ngZoneEnabled: boolean,
ngZone: NgZone
): MonoTypeOperatorFunction<T> {
// This is required to return the source observable (without re-entering the
// Angular zone) to maintain backward compatibility.
if (ngZoneEnabled) {
return (source) => source;
}

// If `ngZone` is nooped by default (or explicitly), we return a
// new observable that proxies values from the source observable, but
// with `next` being called within the Angular zone.
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()),
})
);
}
22 changes: 17 additions & 5 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 @@ -167,10 +168,21 @@ export class DevtoolsExtension {
return EMPTY;
}

const ngZoneEnabled = this.config.ngZone === 'enabled';

return new Observable((subscriber) => {
const connection = this.devtoolsExtension.connect(
this.getExtensionConfig(this.config)
);
const connection = ngZoneEnabled
? 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`.
this.ngZone.runOutsideAngular(() =>
this.devtoolsExtension.connect(this.getExtensionConfig(this.config))
);

this.extensionConnection = connection;
connection.init();

Expand Down

0 comments on commit 6e181f8

Please sign in to comment.