fix(core): deferred blocks not removing content immediately when animations are enabled (#51971)

crisbeto · alxhub · commit 4f69d620d946 · 2023-10-02T09:12:48.000-07:00
Fixes an issue where if animations are enabled, deferred blocks don't remove their placeholder blocks immediately from the DOM. The problem is that we register the event handlers in `afterRender` which runs outside the zone, but the logic that removes the DOM nodes during animations is tied to change detection. These changes resolve the issue by binding the listeners inside the zone. This was the intention from the beginning, I just forgot that `afterRender` runs outside the zone. Fixes #51970. PR Close #51971
diff --git a/packages/core/src/render3/instructions/defer.ts b/packages/core/src/render3/instructions/defer.ts
@@ -7,7 +7,6 @@
  */
 
 import {InjectionToken, Injector, ɵɵdefineInjectable} from '../../di';
-import {inject} from '../../di/injector_compatibility';
 import {findMatchingDehydratedView} from '../../hydration/views';
 import {populateDehydratedViewsInLContainer} from '../../linker/view_container_ref';
 import {assertDefined, assertElement, assertEqual, throwError} from '../../util/assert';
diff --git a/packages/core/src/render3/instructions/defer_events.ts b/packages/core/src/render3/instructions/defer_events.ts
@@ -39,8 +39,10 @@ class DeferEventEntry {
  * Registers an interaction trigger.
  * @param trigger Element that is the trigger.
  * @param callback Callback to be invoked when the trigger is interacted with.
+ * @param injector Injector that can be used by the trigger to resolve DI tokens.
  */
-export function onInteraction(trigger: Element, callback: VoidFunction) {
+export function onInteraction(
+    trigger: Element, callback: VoidFunction, injector: Injector): VoidFunction {
   let entry = interactionTriggers.get(trigger);
 
   // If this is the first entry for this element, add the listeners.
@@ -59,9 +61,13 @@ export function onInteraction(trigger: Element, callback: VoidFunction) {
     entry = new DeferEventEntry();
     interactionTriggers.set(trigger, entry);
 
-    for (const name of interactionEventNames) {
-      trigger.addEventListener(name, entry.listener, eventListenerOptions);
-    }
+    // Ensure that the handler runs in the NgZone since it gets
+    // registered in `afterRender` which runs outside.
+    injector.get(NgZone).run(() => {
+      for (const name of interactionEventNames) {
+        trigger.addEventListener(name, entry!.listener, eventListenerOptions);
+      }
+    });
   }
 
   entry.callbacks.add(callback);
@@ -84,15 +90,21 @@ export function onInteraction(trigger: Element, callback: VoidFunction) {
  * Registers a hover trigger.
  * @param trigger Element that is the trigger.
  * @param callback Callback to be invoked when the trigger is hovered over.
+ * @param injector Injector that can be used by the trigger to resolve DI tokens.
  */
-export function onHover(trigger: Element, callback: VoidFunction): VoidFunction {
+export function onHover(
+    trigger: Element, callback: VoidFunction, injector: Injector): VoidFunction {
   let entry = hoverTriggers.get(trigger);
 
   // If this is the first entry for this element, add the listener.
   if (!entry) {
     entry = new DeferEventEntry();
-    trigger.addEventListener('mouseenter', entry.listener, eventListenerOptions);
     hoverTriggers.set(trigger, entry);
+    // Ensure that the handler runs in the NgZone since it gets
+    // registered in `afterRender` which runs outside.
+    injector.get(NgZone).run(() => {
+      trigger.addEventListener('mouseenter', entry!.listener, eventListenerOptions);
+    });
   }
 
   entry.callbacks.add(callback);
diff --git a/packages/core/test/acceptance/defer_spec.ts b/packages/core/test/acceptance/defer_spec.ts
@@ -1225,6 +1225,7 @@ describe('@defer', () => {
         template: 'Primary block content.',
       })
       class NestedCmp {
+        @Input() block!: string;
       }
 
       @Component({
@@ -1401,6 +1402,7 @@ describe('@defer', () => {
         template: 'Primary block content.',
       })
       class NestedCmp {
+        @Input() block!: string;
       }
 
       @Component({
@@ -1954,6 +1956,32 @@ describe('@defer', () => {
          expect(spy).toHaveBeenCalledWith('keydown', jasmine.any(Function), jasmine.any(Object));
        }));
 
+    it('should bind the trigger events inside the NgZone', fakeAsync(() => {
+         @Component({
+           standalone: true,
+           template: `
+           @defer (on interaction(trigger)) {
+             Main content
+           }
+
+           <button #trigger></button>
+         `
+         })
+         class MyCmp {
+         }
+
+         const eventsInZone: Record<string, boolean> = {};
+         const fixture = TestBed.createComponent(MyCmp);
+         const button = fixture.nativeElement.querySelector('button');
+
+         spyOn(button, 'addEventListener').and.callFake((name: string) => {
+           eventsInZone[name] = NgZone.isInAngularZone();
+         });
+         fixture.detectChanges();
+
+         expect(eventsInZone).toEqual({click: true, keydown: true});
+       }));
+
     it('should prefetch resources on interaction', fakeAsync(() => {
          @Component({
            standalone: true,
@@ -2252,6 +2280,32 @@ describe('@defer', () => {
          expect(spy).toHaveBeenCalledWith('mouseenter', jasmine.any(Function), jasmine.any(Object));
        }));
 
+    it('should bind the trigger events inside the NgZone', fakeAsync(() => {
+         @Component({
+           standalone: true,
+           template: `
+          @defer (on hover(trigger)) {
+            Main content
+          }
+
+          <button #trigger></button>
+        `
+         })
+         class MyCmp {
+         }
+
+         const eventsInZone: Record<string, boolean> = {};
+         const fixture = TestBed.createComponent(MyCmp);
+         const button = fixture.nativeElement.querySelector('button');
+
+         spyOn(button, 'addEventListener').and.callFake((name: string) => {
+           eventsInZone[name] = NgZone.isInAngularZone();
+         });
+         fixture.detectChanges();
+
+         expect(eventsInZone).toEqual({mouseenter: true});
+       }));
+
     it('should prefetch resources on hover', fakeAsync(() => {
          // Domino doesn't support creating custom events so we have to skip this test.
          if (!isBrowser) {
