perf(core): avoid unnecessary callbacks in after render hooks (#52292)

crisbeto · dylhunn · commit cce37435725e · 2023-10-23T09:29:09.000-07:00
A few performance improvements and code cleanups in the after render hooks: 1. We were wrapping each `destroy` callback in another callback, because it was typed as `|undefined`. This is unnecessary, because the callback is guaranteed to exist. These changes pass the `destroy` function around directly and avoid the additional callback. 2. In server platforms we were recreating a noop `AfterRenderRef` on each invocation. We can save some memory by returning the same one. 3. Reworks the `AfterRenderCallback` so that it injects `NgZone` and `ErrorHandler` itself, instead of expecting them to be passed in. This reduces the amount of repetition in the code. PR Close #52292
diff --git a/packages/core/src/render3/after_render_hooks.ts b/packages/core/src/render3/after_render_hooks.ts
@@ -130,6 +130,11 @@ export interface InternalAfterNextRenderOptions {
   injector?: Injector;
 }
 
+/** `AfterRenderRef` that does nothing. */
+const NOOP_AFTER_RENDER_REF: AfterRenderRef = {
+  destroy() {}
+};
+
 /**
  * Register a callback to run once before any userspace `afterRender` or
  * `afterNextRender` callbacks.
@@ -216,24 +221,21 @@ export function afterRender(callback: VoidFunction, options?: AfterRenderOptions
   const injector = options?.injector ?? inject(Injector);
 
   if (!isPlatformBrowser(injector)) {
-    return {destroy() {}};
+    return NOOP_AFTER_RENDER_REF;
   }
 
-  let destroy: VoidFunction|undefined;
-  const unregisterFn = injector.get(DestroyRef).onDestroy(() => destroy?.());
   const afterRenderEventManager = injector.get(AfterRenderEventManager);
   // Lazily initialize the handler implementation, if necessary. This is so that it can be
   // tree-shaken if `afterRender` and `afterNextRender` aren't used.
   const callbackHandler = afterRenderEventManager.handler ??= new AfterRenderCallbackHandlerImpl();
-  const ngZone = injector.get(NgZone);
-  const errorHandler = injector.get(ErrorHandler, null, {optional: true});
   const phase = options?.phase ?? AfterRenderPhase.MixedReadWrite;
-  const instance = new AfterRenderCallback(ngZone, errorHandler, phase, callback);
-
-  destroy = () => {
+  const destroy = () => {
     callbackHandler.unregister(instance);
     unregisterFn();
   };
+  const unregisterFn = injector.get(DestroyRef).onDestroy(destroy);
+  const instance = new AfterRenderCallback(injector, phase, callback);
+
   callbackHandler.register(instance);
   return {destroy};
 }
@@ -293,27 +295,24 @@ export function afterNextRender(
   const injector = options?.injector ?? inject(Injector);
 
   if (!isPlatformBrowser(injector)) {
-    return {destroy() {}};
+    return NOOP_AFTER_RENDER_REF;
   }
 
-  let destroy: VoidFunction|undefined;
-  const unregisterFn = injector.get(DestroyRef).onDestroy(() => destroy?.());
   const afterRenderEventManager = injector.get(AfterRenderEventManager);
   // Lazily initialize the handler implementation, if necessary. This is so that it can be
   // tree-shaken if `afterRender` and `afterNextRender` aren't used.
   const callbackHandler = afterRenderEventManager.handler ??= new AfterRenderCallbackHandlerImpl();
-  const ngZone = injector.get(NgZone);
-  const errorHandler = injector.get(ErrorHandler, null, {optional: true});
   const phase = options?.phase ?? AfterRenderPhase.MixedReadWrite;
-  const instance = new AfterRenderCallback(ngZone, errorHandler, phase, () => {
-    destroy?.();
-    callback();
-  });
-
-  destroy = () => {
+  const destroy = () => {
     callbackHandler.unregister(instance);
     unregisterFn();
   };
+  const unregisterFn = injector.get(DestroyRef).onDestroy(destroy);
+  const instance = new AfterRenderCallback(injector, phase, () => {
+    destroy();
+    callback();
+  });
+
   callbackHandler.register(instance);
   return {destroy};
 }
@@ -322,9 +321,15 @@ export function afterNextRender(
  * A wrapper around a function to be used as an after render callback.
  */
 class AfterRenderCallback {
+  private zone: NgZone;
+  private errorHandler: ErrorHandler|null;
+
   constructor(
-      private zone: NgZone, private errorHandler: ErrorHandler|null,
-      public readonly phase: AfterRenderPhase, private callbackFn: VoidFunction) {}
+      injector: Injector, public readonly phase: AfterRenderPhase,
+      private callbackFn: VoidFunction) {
+    this.zone = injector.get(NgZone);
+    this.errorHandler = injector.get(ErrorHandler, null, {optional: true});
+  }
 
   invoke() {
     try {
