Skip to content

Commit 514d833

Browse files
committed
fix(cdk/scrolling): Prevent virtual scroll 'flickering' with zoneless
This commit reworks the change detection coalescing to use signal and effects, which ensures the transform and the afterNextRender are applied within the same application tick without any awkward workarounds or fiddling with being inside or outside the zone
1 parent 8cb522d commit 514d833

File tree

1 file changed

+62
-69
lines changed

1 file changed

+62
-69
lines changed

src/cdk/scrolling/virtual-scroll-viewport.ts

Lines changed: 62 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ import {
2626
signal,
2727
ViewChild,
2828
ViewEncapsulation,
29+
ApplicationRef,
30+
effect,
31+
linkedSignal,
2932
} from '@angular/core';
3033
import {
3134
animationFrameScheduler,
@@ -170,19 +173,16 @@ export class CdkVirtualScrollViewport extends CdkVirtualScrollable implements On
170173
*/
171174
private _renderedContentOffsetNeedsRewrite = false;
172175

173-
/** Whether there is a pending change detection cycle. */
174-
private _isChangeDetectionPending = false;
175-
176176
/** A list of functions to run after the next change detection cycle. */
177-
private _runAfterChangeDetection: Function[] = [];
177+
private _runAfterChangeDetection = signal<Function[]>([]);
178178

179179
/** Subscription to changes in the viewport size. */
180180
private _viewportChanges = Subscription.EMPTY;
181181

182-
private _injector = inject(Injector);
183-
184182
private _isDestroyed = false;
185183

184+
private _changeDetectionNeeded = linkedSignal(() => this._runAfterChangeDetection().length > 0);
185+
186186
constructor(...args: unknown[]);
187187

188188
constructor() {
@@ -202,6 +202,41 @@ export class CdkVirtualScrollViewport extends CdkVirtualScrollable implements On
202202
this.elementRef.nativeElement.classList.add('cdk-virtual-scrollable');
203203
this.scrollable = this;
204204
}
205+
206+
const injector = inject(ApplicationRef).injector;
207+
effect(
208+
() => {
209+
if (!this._changeDetectionNeeded() || this._isDestroyed) {
210+
return;
211+
}
212+
213+
// Apply the content transform. The transform can't be set via an Angular binding because
214+
// bypassSecurityTrustStyle is banned in Google. However the value is safe, it's composed of
215+
// string literals, a variable that can only be 'X' or 'Y', and user input that is run through
216+
// the `Number` function first to coerce it to a numeric value.
217+
this._contentWrapper.nativeElement.style.transform = this._renderedContentTransform;
218+
219+
// Apply changes to Angular bindings. Note: We must call `markForCheck` to run change detection
220+
// from the root, since the repeated items are content projected in. Calling `detectChanges`
221+
// instead does not properly check the projected content.
222+
this._changeDetectorRef.markForCheck();
223+
224+
afterNextRender(
225+
{
226+
mixedReadWrite: () => {
227+
// TODO: should this be done at the top?
228+
const runAfterChangeDetection = this._runAfterChangeDetection();
229+
this._runAfterChangeDetection.set([]);
230+
for (const fn of runAfterChangeDetection) {
231+
fn();
232+
}
233+
},
234+
},
235+
{injector},
236+
);
237+
},
238+
{injector},
239+
);
205240
}
206241

207242
override ngOnInit() {
@@ -238,7 +273,7 @@ export class CdkVirtualScrollViewport extends CdkVirtualScrollable implements On
238273
)
239274
.subscribe(() => this._scrollStrategy.onContentScrolled());
240275

241-
this._markChangeDetectionNeeded();
276+
this._changeDetectionNeeded.set(true);
242277
}),
243278
);
244279
}
@@ -274,7 +309,7 @@ export class CdkVirtualScrollViewport extends CdkVirtualScrollable implements On
274309
this._dataLength = newLength;
275310
this._scrollStrategy.onDataLengthChanged();
276311
}
277-
this._doChangeDetection();
312+
this._changeDetectionNeeded.set(true);
278313
});
279314
});
280315
}
@@ -317,7 +352,7 @@ export class CdkVirtualScrollViewport extends CdkVirtualScrollable implements On
317352
if (this._totalContentSize !== size) {
318353
this._totalContentSize = size;
319354
this._calculateSpacerSize();
320-
this._markChangeDetectionNeeded();
355+
this._changeDetectionNeeded.set(true);
321356
}
322357
}
323358

@@ -328,7 +363,11 @@ export class CdkVirtualScrollViewport extends CdkVirtualScrollable implements On
328363
range = {start: 0, end: Math.max(this._renderedRange.end, range.end)};
329364
}
330365
this._renderedRangeSubject.next((this._renderedRange = range));
331-
this._markChangeDetectionNeeded(() => this._scrollStrategy.onContentRendered());
366+
this._runAfterChangeDetection.update(v => [
367+
...v,
368+
() => this._scrollStrategy.onContentRendered(),
369+
]);
370+
this._changeDetectionNeeded.set(true);
332371
}
333372
}
334373

@@ -366,15 +405,19 @@ export class CdkVirtualScrollViewport extends CdkVirtualScrollable implements On
366405
// We know this value is safe because we parse `offset` with `Number()` before passing it
367406
// into the string.
368407
this._renderedContentTransform = transform;
369-
this._markChangeDetectionNeeded(() => {
370-
if (this._renderedContentOffsetNeedsRewrite) {
371-
this._renderedContentOffset -= this.measureRenderedContentSize();
372-
this._renderedContentOffsetNeedsRewrite = false;
373-
this.setRenderedContentOffset(this._renderedContentOffset);
374-
} else {
375-
this._scrollStrategy.onRenderedOffsetChanged();
376-
}
377-
});
408+
this._runAfterChangeDetection.update(v => [
409+
...v,
410+
() => {
411+
if (this._renderedContentOffsetNeedsRewrite) {
412+
this._renderedContentOffset -= this.measureRenderedContentSize();
413+
this._renderedContentOffsetNeedsRewrite = false;
414+
this.setRenderedContentOffset(this._renderedContentOffset);
415+
} else {
416+
this._scrollStrategy.onRenderedOffsetChanged();
417+
}
418+
},
419+
]);
420+
this._changeDetectionNeeded.set(true);
378421
}
379422
}
380423

@@ -482,56 +525,6 @@ export class CdkVirtualScrollViewport extends CdkVirtualScrollable implements On
482525
this._viewportSize = this.scrollable.measureViewportSize(this.orientation);
483526
}
484527

485-
/** Queue up change detection to run. */
486-
private _markChangeDetectionNeeded(runAfter?: Function) {
487-
if (runAfter) {
488-
this._runAfterChangeDetection.push(runAfter);
489-
}
490-
491-
// Use a Promise to batch together calls to `_doChangeDetection`. This way if we set a bunch of
492-
// properties sequentially we only have to run `_doChangeDetection` once at the end.
493-
if (!this._isChangeDetectionPending) {
494-
this._isChangeDetectionPending = true;
495-
this.ngZone.runOutsideAngular(() =>
496-
Promise.resolve().then(() => {
497-
this._doChangeDetection();
498-
}),
499-
);
500-
}
501-
}
502-
503-
/** Run change detection. */
504-
private _doChangeDetection() {
505-
if (this._isDestroyed) {
506-
return;
507-
}
508-
509-
this.ngZone.run(() => {
510-
// Apply changes to Angular bindings. Note: We must call `markForCheck` to run change detection
511-
// from the root, since the repeated items are content projected in. Calling `detectChanges`
512-
// instead does not properly check the projected content.
513-
this._changeDetectorRef.markForCheck();
514-
515-
// Apply the content transform. The transform can't be set via an Angular binding because
516-
// bypassSecurityTrustStyle is banned in Google. However the value is safe, it's composed of
517-
// string literals, a variable that can only be 'X' or 'Y', and user input that is run through
518-
// the `Number` function first to coerce it to a numeric value.
519-
this._contentWrapper.nativeElement.style.transform = this._renderedContentTransform;
520-
521-
afterNextRender(
522-
() => {
523-
this._isChangeDetectionPending = false;
524-
const runAfterChangeDetection = this._runAfterChangeDetection;
525-
this._runAfterChangeDetection = [];
526-
for (const fn of runAfterChangeDetection) {
527-
fn();
528-
}
529-
},
530-
{injector: this._injector},
531-
);
532-
});
533-
}
534-
535528
/** Calculates the `style.width` and `style.height` for the spacer element. */
536529
private _calculateSpacerSize() {
537530
this._totalContentHeight.set(

0 commit comments

Comments
 (0)