From db2f2d99c82aae52d8a0ae46616c6411d070b35e Mon Sep 17 00:00:00 2001
From: Andrew Scott <atscott@google.com>
Date: Mon, 24 Feb 2025 10:53:34 -0800
Subject: [PATCH] fix(router): Scroller should scroll as soon as change
 detection completes (#60086)

Using `setTimeout` to delay scrolling can result in scrolling in the
next frame and cause noticeable flicker. This commit races rAF and
timeout to ensure scroll happens before the render.

fixes #53985

PR Close #60086
---
 packages/router/src/router_scroller.ts | 42 ++++++++++++++++++--------
 1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/packages/router/src/router_scroller.ts b/packages/router/src/router_scroller.ts
index 03b87b00f62..5e2f5d01cee 100644
--- a/packages/router/src/router_scroller.ts
+++ b/packages/router/src/router_scroller.ts
@@ -7,7 +7,14 @@
  */
 
 import {ViewportScroller} from '@angular/common';
-import {Injectable, InjectionToken, NgZone, OnDestroy} from '@angular/core';
+import {
+  EnvironmentInjector,
+  inject,
+  Injectable,
+  InjectionToken,
+  NgZone,
+  OnDestroy,
+} from '@angular/core';
 import {Unsubscribable} from 'rxjs';
 
 import {
@@ -105,21 +112,30 @@ export class RouterScroller implements OnDestroy {
     routerEvent: NavigationEnd | NavigationSkipped,
     anchor: string | null,
   ): void {
-    this.zone.runOutsideAngular(() => {
+    this.zone.runOutsideAngular(async () => {
       // The scroll event needs to be delayed until after change detection. Otherwise, we may
       // attempt to restore the scroll position before the router outlet has fully rendered the
       // component by executing its update block of the template function.
-      setTimeout(() => {
-        this.zone.run(() => {
-          this.transitions.events.next(
-            new Scroll(
-              routerEvent,
-              this.lastSource === 'popstate' ? this.store[this.restoredId] : null,
-              anchor,
-            ),
-          );
-        });
-      }, 0);
+      //
+      // #57109 (we need to wait at least a macrotask before scrolling. AfterNextRender resolves in microtask event loop with Zones)
+      // We could consider _also_ waiting for a render promise though one should have already happened or been scheduled by this point
+      // and should definitely happen before rAF/setTimeout.
+      // #53985 (cannot rely solely on setTimeout because a frame may paint before the timeout)
+      await new Promise((resolve) => {
+        setTimeout(resolve);
+        if (typeof requestAnimationFrame !== 'undefined') {
+          requestAnimationFrame(resolve);
+        }
+      });
+      this.zone.run(() => {
+        this.transitions.events.next(
+          new Scroll(
+            routerEvent,
+            this.lastSource === 'popstate' ? this.store[this.restoredId] : null,
+            anchor,
+          ),
+        );
+      });
     });
   }