Skip to content

Commit d82c600

Browse files
committed
split batchCD into 2 functions, parallel and noAutoChangeDetection
1 parent f000788 commit d82c600

File tree

6 files changed

+240
-89
lines changed

6 files changed

+240
-89
lines changed

src/cdk/testing/BUILD.bazel

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ ts_library(
1111
"**/*.spec.ts",
1212
],
1313
),
14+
deps = [
15+
"@npm//rxjs",
16+
],
1417
module_name = "@angular/cdk/testing",
1518
)
1619

src/cdk/testing/component-harness.ts

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9+
import {noAutoChangeDetection, parallel} from '@angular/cdk/testing/harness-environment';
910
import {TestElement} from './test-element';
1011

1112
/** An async function that returns a promise when called. */
@@ -101,8 +102,6 @@ export interface HarnessLoader {
101102
* @return A list instances of the given harness type.
102103
*/
103104
getAllHarnesses<T extends ComponentHarness>(query: HarnessQuery<T>): Promise<T[]>;
104-
105-
batchCD<T>(fn: () => Promise<T>): Promise<T>;
106105
}
107106

108107
/**
@@ -242,8 +241,6 @@ export interface LocatorFactory {
242241
* authors to wait for async tasks outside of the Angular zone.
243242
*/
244243
waitForTasksOutsideAngular(): Promise<void>;
245-
246-
batchCD<T>(fn: () => Promise<T>): Promise<T>;
247244
}
248245

249246
/**
@@ -377,10 +374,6 @@ export abstract class ComponentHarness {
377374
protected async waitForTasksOutsideAngular() {
378375
return this.locatorFactory.waitForTasksOutsideAngular();
379376
}
380-
381-
async batchCD<T>(fn: () => Promise<T>) {
382-
return this.locatorFactory.batchCD(fn);
383-
}
384377
}
385378

386379

@@ -497,8 +490,7 @@ export class HarnessPredicate<T extends ComponentHarness> {
497490
if (harnesses.length === 0) {
498491
return [];
499492
}
500-
const results =
501-
await harnesses[0].batchCD(() => Promise.all(harnesses.map(h => this.evaluate(h))));
493+
const results = await parallel(harnesses.map(h => this.evaluate(h)));
502494
return harnesses.filter((_, i) => results[i]);
503495
}
504496

@@ -509,7 +501,7 @@ export class HarnessPredicate<T extends ComponentHarness> {
509501
* and resolves to false otherwise.
510502
*/
511503
async evaluate(harness: T): Promise<boolean> {
512-
const results = await harness.batchCD(() => Promise.all(this._predicates.map(p => p(harness))));
504+
const results = await parallel((this._predicates.map(p => p(harness))));
513505
return results.reduce((combined, current) => combined && current, true);
514506
}
515507

src/cdk/testing/harness-environment.ts

Lines changed: 114 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9+
import {BehaviorSubject, Subscription} from 'rxjs';
910
import {
1011
AsyncFactoryFn,
1112
ComponentHarness,
@@ -36,7 +37,108 @@ type ParsedQueries<T extends ComponentHarness> = {
3637
harnessTypes: Set<ComponentHarnessConstructor<T>>,
3738
};
3839

39-
let isCDBatching = false;
40+
/** Represents the status of change detection batching. */
41+
export interface ChangeDetectionBatchingStatus {
42+
/** Whether change detection is batching. */
43+
isBatching: boolean;
44+
/**
45+
* An optional callback, if present it indicates that change detection should be run immediately,
46+
* while handling the batching status change. The callback should then be called as soon as change
47+
* detection is done.
48+
*/
49+
onDetectChangesNow?: () => void;
50+
}
51+
52+
/** Subject used to dispatch and listen for changes to the change detection batching status . */
53+
const batchChangeDetectionSubject = new BehaviorSubject<ChangeDetectionBatchingStatus>({
54+
isBatching: false
55+
});
56+
57+
/** The current subscription to `batchChangeDetectionSubject`. */
58+
let batchChangeDetectionSubscription: Subscription | null;
59+
60+
/**
61+
* The default handler for change detection batching status changes. This handler will be used if
62+
* the specific environment does not install its own.
63+
* @param status The new change detection batching status.
64+
*/
65+
function defaultBatchChangeDetectionHandler(status: ChangeDetectionBatchingStatus) {
66+
status.onDetectChangesNow?.();
67+
}
68+
69+
/**
70+
* Allows a test `HarnessEnvironment` to install its own handler for change detection batching
71+
* status changes.
72+
* @param handler The handler for the change detection batching status.
73+
*/
74+
export function handleChangeDetectionBatching(
75+
handler: (status: ChangeDetectionBatchingStatus) => void) {
76+
stopHandlingChangeDetectionBatching();
77+
batchChangeDetectionSubscription = batchChangeDetectionSubject.subscribe(handler);
78+
}
79+
80+
/** Allows a `HarnessEnvironment` to stop handling change detection batching status changes. */
81+
export function stopHandlingChangeDetectionBatching() {
82+
batchChangeDetectionSubscription?.unsubscribe();
83+
batchChangeDetectionSubscription = null;
84+
}
85+
86+
/**
87+
* Batches together triggering of change detection over the duration of the given function.
88+
* @param fn The function to call with batched change detection.
89+
* @param triggerBeforeAndAfter Optionally trigger change detection once before and after the batch
90+
* operation. If false, change detection will not be triggered.
91+
* @return The result of the given function.
92+
*/
93+
async function batchChangeDetection<T>(fn: () => Promise<T>, triggerBeforeAndAfter: boolean) {
94+
// If change detection batching is already in progress, just run the function.
95+
if (batchChangeDetectionSubject.getValue().isBatching) {
96+
return await fn();
97+
}
98+
99+
// If nothing is handling change detection batching, install the default handler.
100+
if (!batchChangeDetectionSubscription) {
101+
batchChangeDetectionSubject.subscribe(defaultBatchChangeDetectionHandler);
102+
}
103+
104+
if (triggerBeforeAndAfter) {
105+
await new Promise(resolve => batchChangeDetectionSubject.next({
106+
isBatching: true,
107+
onDetectChangesNow: resolve,
108+
}));
109+
const result = await fn();
110+
await new Promise(resolve => batchChangeDetectionSubject.next({
111+
isBatching: false,
112+
onDetectChangesNow: resolve,
113+
}));
114+
return result;
115+
} else {
116+
batchChangeDetectionSubject.next({isBatching: true});
117+
const result = await fn();
118+
batchChangeDetectionSubject.next({isBatching: false});
119+
return result;
120+
}
121+
}
122+
123+
/**
124+
* Disables the harness system's auto change detection for the duration of the given function.
125+
* @param fn The function to disable auto change detection for.
126+
* @return The result of the given function.
127+
*/
128+
export async function noAutoChangeDetection<T>(fn: () => Promise<T>) {
129+
return batchChangeDetection(fn, false);
130+
}
131+
132+
/**
133+
* Resolves the given list of async values in parallel (i.e. via Promise.all) while batching change
134+
* detection over the entire operation such that change detection occurs exactly once before
135+
* resolving the values and once after.
136+
* @param values The async values to resolve in parallel with batched change detection.
137+
* @return The resolved values.
138+
*/
139+
export async function parallel<T>(values: Iterable<T | PromiseLike<T>>) {
140+
return batchChangeDetection(() => Promise.all(values), true);
141+
}
40142

41143
/**
42144
* Base harness environment class that can be extended to allow `ComponentHarness`es to be used in
@@ -121,33 +223,6 @@ export abstract class HarnessEnvironment<E> implements HarnessLoader, LocatorFac
121223
return (await this.getAllRawElements(selector)).map(e => this.createEnvironment(e));
122224
}
123225

124-
protected isCDBatching() {
125-
return isCDBatching;
126-
}
127-
128-
async batchCD<T>(fn: () => Promise<T>) {
129-
// Flipping on the `isCDBatching` flag will turn all calls to `forceStabilize` into no-ops to
130-
// prevent triggering redundant change detection. However, we want to make sure we trigger
131-
// change detection exactly once before flipping this flag to ensure that any pending changes
132-
// are flushed before the harness tries to read state from DOM elements.
133-
const alreadyBatching = isCDBatching;
134-
if (!alreadyBatching) {
135-
await this.forceStabilize();
136-
isCDBatching = true;
137-
}
138-
139-
const result = await fn();
140-
141-
// We also want to run change detection exactly once after the batched operation is complete.
142-
// This will ensure that any changes from interacting with DOM elements are properly flushed.
143-
if (!alreadyBatching) {
144-
isCDBatching = false;
145-
await this.forceStabilize();
146-
}
147-
148-
return result;
149-
}
150-
151226
/** Creates a `ComponentHarness` for the given harness type with the given raw host element. */
152227
protected createComponentHarness<T extends ComponentHarness>(
153228
harnessType: ComponentHarnessConstructor<T>, element: E): T {
@@ -198,18 +273,17 @@ export abstract class HarnessEnvironment<E> implements HarnessLoader, LocatorFac
198273
// may trigger change detection by reading state from DOM elements. If not batched these change
199274
// detections would be triggered once per potential match element which could cause significant
200275
// slowdown.
201-
const perElementMatches = await this.batchCD(() =>
202-
Promise.all(rawElements.map(async rawElement => {
203-
const testElement = this.createTestElement(rawElement);
204-
const allResultsForElement = await Promise.all(
205-
// For each query, get `null` if it doesn't match, or a `TestElement` or
206-
// `ComponentHarness` as appropriate if it does match. This gives us everything that
207-
// matches the current raw element, but it may contain duplicate entries (e.g.
208-
// multiple `TestElement` or multiple `ComponentHarness` of the same type).
209-
allQueries.map(query => this._getQueryResultForElement(
210-
query, rawElement, testElement, skipSelectorCheck)));
211-
return _removeDuplicateQueryResults(allResultsForElement);
212-
})));
276+
const perElementMatches = await parallel(rawElements.map(async rawElement => {
277+
const testElement = this.createTestElement(rawElement);
278+
const allResultsForElement = await Promise.all(
279+
// For each query, get `null` if it doesn't match, or a `TestElement` or
280+
// `ComponentHarness` as appropriate if it does match. This gives us everything that
281+
// matches the current raw element, but it may contain duplicate entries (e.g.
282+
// multiple `TestElement` or multiple `ComponentHarness` of the same type).
283+
allQueries.map(query => this._getQueryResultForElement(
284+
query, rawElement, testElement, skipSelectorCheck)));
285+
return _removeDuplicateQueryResults(allResultsForElement);
286+
}));
213287
return ([] as any).concat(...perElementMatches);
214288
}
215289

src/cdk/testing/testbed/testbed-harness-environment.ts

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@
99
import {
1010
ComponentHarness,
1111
ComponentHarnessConstructor,
12+
handleChangeDetectionBatching,
1213
HarnessEnvironment,
1314
HarnessLoader,
15+
stopHandlingChangeDetectionBatching,
1416
TestElement
1517
} from '@angular/cdk/testing';
1618
import {ComponentFixture, flush} from '@angular/core/testing';
@@ -30,6 +32,50 @@ const defaultEnvironmentOptions: TestbedHarnessEnvironmentOptions = {
3032
queryFn: (selector: string, root: Element) => root.querySelectorAll(selector)
3133
};
3234

35+
/** Whether auto change detection is currently disabled. */
36+
let disableAutoChangeDetection = false;
37+
38+
/**
39+
* The set of non-destroyed fixtures currently being used by `TestbedHarnessEnvironment` instances.
40+
*/
41+
const activeFixtures = new Set<ComponentFixture<unknown>>();
42+
43+
/**
44+
* Installs a handler for change detection batching status changes for a specific fixture.
45+
* @param fixture The fixture to handle change detection batching for.
46+
*/
47+
function installChangeDetectionBatchingHandler(fixture: ComponentFixture<unknown>) {
48+
if (!activeFixtures.size) {
49+
handleChangeDetectionBatching(({isBatching, onDetectChangesNow}) => {
50+
disableAutoChangeDetection = isBatching;
51+
if (onDetectChangesNow) {
52+
Promise.all([...activeFixtures].map(detectChanges)).then(onDetectChangesNow);
53+
}
54+
});
55+
}
56+
activeFixtures.add(fixture);
57+
}
58+
59+
/**
60+
* Uninstalls a handler for change detection batching status changes for a specific fixture.
61+
* @param fixture The fixture to stop handling change detection batching for.
62+
*/
63+
function uninstallChangeDetectionBatchingHandler(fixture: ComponentFixture<unknown>) {
64+
activeFixtures.delete(fixture);
65+
if (!activeFixtures.size) {
66+
stopHandlingChangeDetectionBatching();
67+
}
68+
}
69+
70+
/**
71+
* Triggers change detection for a specific fixture.
72+
* @param fixture The fixture to trigger change detection for.
73+
*/
74+
async function detectChanges(fixture: ComponentFixture<unknown>) {
75+
fixture.detectChanges();
76+
await fixture.whenStable();
77+
}
78+
3379
/** A `HarnessEnvironment` implementation for Angular's Testbed. */
3480
export class TestbedHarnessEnvironment extends HarnessEnvironment<Element> {
3581
/** Whether the environment has been destroyed. */
@@ -46,7 +92,11 @@ export class TestbedHarnessEnvironment extends HarnessEnvironment<Element> {
4692
super(rawRootElement);
4793
this._options = {...defaultEnvironmentOptions, ...options};
4894
this._taskState = TaskStateZoneInterceptor.setup();
49-
_fixture.componentRef.onDestroy(() => this._destroyed = true);
95+
installChangeDetectionBatchingHandler(_fixture);
96+
_fixture.componentRef.onDestroy(() => {
97+
uninstallChangeDetectionBatchingHandler(_fixture);
98+
this._destroyed = true;
99+
});
50100
}
51101

52102
/** Creates a `HarnessLoader` rooted at the given fixture's root element. */
@@ -79,13 +129,12 @@ export class TestbedHarnessEnvironment extends HarnessEnvironment<Element> {
79129
}
80130

81131
async forceStabilize(): Promise<void> {
82-
if (!this.isCDBatching()) {
132+
if (!disableAutoChangeDetection) {
83133
if (this._destroyed) {
84134
throw Error('Harness is attempting to use a fixture that has already been destroyed.');
85135
}
86136

87-
this._fixture.detectChanges();
88-
await this._fixture.whenStable();
137+
await detectChanges(this._fixture);
89138
}
90139
}
91140

0 commit comments

Comments
 (0)