refactor(core): Update callback schedulers to cancel pending timers (#57186)

atscott · thePunderWoman · commit 62ade76cf87b · 2024-07-30T18:05:09.000Z
Rather than leaving the timers around as no-ops, this commit updates the logic to also attempt to clear or cancel the timers. This is helpful for the eventual goal of running the scheduler in the `fakeAsync` zone (if the test is running in `fakeAsync`) rather than scheduling in the root zone and making it impossible to flush. PR Close #57186
diff --git a/packages/core/src/util/callback_scheduler.ts b/packages/core/src/util/callback_scheduler.ts
@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {global} from './global';
+import {noop} from './noop';
 
 /**
  * Gets a scheduling function that runs the callback after the first of setTimeout and
@@ -35,38 +35,40 @@ import {global} from './global';
  * @returns a function to cancel the scheduled callback
  */
 export function scheduleCallbackWithRafRace(callback: Function): () => void {
-  let executeCallback = true;
-  setTimeout(() => {
-    if (!executeCallback) {
-      return;
+  let timeoutId: number;
+  let animationFrameId: number;
+  function cleanup() {
+    callback = noop;
+    try {
+      if (animationFrameId !== undefined && typeof cancelAnimationFrame === 'function') {
+        cancelAnimationFrame(animationFrameId);
+      }
+      if (timeoutId !== undefined) {
+        clearTimeout(timeoutId);
+      }
+    } catch {
+      // Clearing/canceling can fail in tests due to the timing of functions being patched and unpatched
+      // Just ignore the errors - we protect ourselves from this issue by also making the callback a no-op.
     }
-    executeCallback = false;
+  }
+  timeoutId = setTimeout(() => {
     callback();
-  });
-  if (typeof global['requestAnimationFrame'] === 'function') {
-    global['requestAnimationFrame'](() => {
-      if (!executeCallback) {
-        return;
-      }
-      executeCallback = false;
+    cleanup();
+  }) as unknown as number;
+  if (typeof requestAnimationFrame === 'function') {
+    animationFrameId = requestAnimationFrame(() => {
       callback();
+      cleanup();
     });
   }
 
-  return () => {
-    executeCallback = false;
-  };
+  return () => cleanup();
 }
 
 export function scheduleCallbackWithMicrotask(callback: Function): () => void {
-  let executeCallback = true;
-  queueMicrotask(() => {
-    if (executeCallback) {
-      callback();
-    }
-  });
+  queueMicrotask(() => callback());
 
   return () => {
-    executeCallback = false;
+    callback = noop;
   };
 }
diff --git a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json
@@ -608,9 +608,6 @@
   {
     "name": "_getInsertInFrontOfRNodeWithI18n"
   },
-  {
-    "name": "_global"
-  },
   {
     "name": "_icuContainerIterate"
   },
diff --git a/packages/core/test/bundling/defer/bundle.golden_symbols.json b/packages/core/test/bundling/defer/bundle.golden_symbols.json
@@ -548,9 +548,6 @@
   {
     "name": "_getInsertInFrontOfRNodeWithI18n"
   },
-  {
-    "name": "_global"
-  },
   {
     "name": "_icuContainerIterate"
   },
diff --git a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json
@@ -371,9 +371,6 @@
   {
     "name": "_currentInjector"
   },
-  {
-    "name": "_global"
-  },
   {
     "name": "_icuContainerIterate"
   },
diff --git a/packages/core/test/bundling/hydration/bundle.golden_symbols.json b/packages/core/test/bundling/hydration/bundle.golden_symbols.json
@@ -548,9 +548,6 @@
   {
     "name": "_getInsertInFrontOfRNodeWithI18n"
   },
-  {
-    "name": "_global"
-  },
   {
     "name": "_icuContainerIterate"
   },
diff --git a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json
@@ -443,9 +443,6 @@
   {
     "name": "_getInsertInFrontOfRNodeWithI18n"
   },
-  {
-    "name": "_global"
-  },
   {
     "name": "_icuContainerIterate"
   },

Original file line number	Diff line number	Diff line change
`@@ -608,9 +608,6 @@`
`608`	`608`	`{`
`609`	`609`	`"name": "_getInsertInFrontOfRNodeWithI18n"`
`610`	`610`	`},`
`611`		`- {`
`612`		`- "name": "_global"`
`613`		`- },`
`614`	`611`	`{`
`615`	`612`	`"name": "_icuContainerIterate"`
`616`	`613`	`},`
Original file line number	Diff line number	Diff line change
`@@ -548,9 +548,6 @@`
`548`	`548`	`{`
`549`	`549`	`"name": "_getInsertInFrontOfRNodeWithI18n"`
`550`	`550`	`},`
`551`		`- {`
`552`		`- "name": "_global"`
`553`		`- },`
`554`	`551`	`{`
`555`	`552`	`"name": "_icuContainerIterate"`
`556`	`553`	`},`
Original file line number	Diff line number	Diff line change
`@@ -371,9 +371,6 @@`
`371`	`371`	`{`
`372`	`372`	`"name": "_currentInjector"`
`373`	`373`	`},`
`374`		`- {`
`375`		`- "name": "_global"`
`376`		`- },`
`377`	`374`	`{`
`378`	`375`	`"name": "_icuContainerIterate"`
`379`	`376`	`},`
Original file line number	Diff line number	Diff line change
`@@ -443,9 +443,6 @@`
`443`	`443`	`{`
`444`	`444`	`"name": "_getInsertInFrontOfRNodeWithI18n"`
`445`	`445`	`},`
`446`		`- {`
`447`		`- "name": "_global"`
`448`		`- },`
`449`	`446`	`{`
`450`	`447`	`"name": "_icuContainerIterate"`
`451`	`448`	`},`