Event Bubbling Change Detection Coalescing

by Miško Hevery & Jia Li (inspired by Igor Minar)

Overview

Assume that you have Angular template such as this:

<div (click)="doSomethingElse()">
  <button (click)="doSomething()">click me</button>
</div>

It turns out that as the click event bubbles from the <button> towards the root document it triggers change detection on each listener. This example shows the phenomena in interactive form. Notice that clicking on the <button> will run the change detection twice. Once for the <button> and once for the <div>.

This is a common scenario especially for applications which use Material Components, because the Material Components often need to register events on interactive elements to update their internal state. Additionally the developer of the application also registers the event listener to perform application logic.

<input (keyup)="logic()" (input)="otherLogic()">

Proposed Change

The proposal is to coalesce the change-detection as the event is bubbling and only execute one change detection for all of the events.

Technically this is a breaking change, however, we don't think there are many applications which rely on this behavior and so the coalescing should not have behavior impact on most of them. (Other than making them faster.) We propose that we introduce this change in google3 and then depending on the impact make this either an opt-in or opt-out feature.

Approach

The first thing we need to do is detect when the last listener has executed. One way to do this would be to put a global listener at the root of the application. Unfortunately this approach is error prone as there could be other listener which could cancel the bubbling and we could be in a detached tree. (This listener do not have to be registered in our current zone and so there is no easy way to know about them)

A more promising approach is to try to use Promise.resolve(), requestAnimationFrame and setTimeout. An example of each of the approaches is demonstrated in here.

From the above table it seems that requestAnimationFrame is the best way to coalesce multiple event listeners into singe change detection. However, requestAnimationFrame will not work for tests because element.click() will expect the change detection to be synchronous.

Hybrid Approach

Perhaps best way is to have a hybrid approach with requestAnimationFrame fallback. Here is the algorithm.

1. When the listener executes the first thing it does is to call maybeDelayChangeDetection() as described later.
2. maybeDelayChangeDetection() does three things.

• It walks to the parent most DOM element and registers a listener of the same type at that place. (This is where the bubbling ends). When this listener triggers we know it is time to run the change-detection.
• For the listeners which cancel the bubbling we do change detection right there.
• Just in case there was a listener which canceled the event and we were not aware of it, we can also register requestAnimationFrame as a fallback method.

Implementation

The best place to hook into the event listeners is in the decoratePreventDefault

function decoratePreventDefault(eventHandler: Function): Function {
  return (event: any) => {
    // START: INSERT THIS CODE
    if (typeof Zone !== 'undefined') {
      const maybeDelayChangeDetection = Zone.current
                .get('maybeDelayChangeDetection');
      maybeDelayChangeDetection && maybeDelayChangeDetection();
    }
    // END: INSERT THIS CODE
    const allowDefaultBehavior = eventHandler(event);
    if (allowDefaultBehavior === false) {
      // TODO(tbosch): move preventDefault into event plugins...
      event.preventDefault();
      event.returnValue = false;
    }
  };
}

addEventListener(element: HTMLElement, eventName: string, handler: Function): Function {
  let callback = function(evt: Event) {
    // START: INSERT THIS CODE
    if (typeof Zone !== 'undefined') {
      const maybeDelayChangeDetection = Zone.current
                .get('maybeDelayChangeDetection');
      maybeDelayChangeDetection && maybeDelayChangeDetection();
    }
    // END: INSERT THIS CODE
    return handler.call(this, evt);
  }
  element.addEventListener(eventName, callback);
  return () => removeEventListener(element, eventName, callback);
}

it ('should only successfully add one handler when try to add the same handler twice', () => {
  const handler = jasmine.createSpy();
  button.addEventListener('click', handler);
  button.addEventListener('click', handler);
  dispatchEvent(clickEvt);
  expect(handler.calls.count()).toBe(1);
});

This case will fail, because even the handler is the same, because it will be wrapped with maybeDelayChangeDetection callback when call addEventListener,
so from Browser's perspective, they are different callbacks. And we can do some tricks to resolve this issue, but the code will not look elegent.

So I think maybe this logic should be added to NgZone directly like this.

onInvokeTask: (delegate: ZoneDelegate, current: Zone, target: Zone, task: Task, applyThis: any,
                   applyArgs: any): any => {
      try {
        onEnter(zone);
        if (maybeDelayChangeDetection && task.type === 'eventTask') {
          maybeDelayChangeDetection();
        }
        return delegate.invokeTask(target, task, applyThis, applyArgs);
      } finally {
        onLeave(zone);
      }
    },

We can then change the NgZone implementation like so:

function forkInnerZoneWithAngularBehavior(zone: NgZonePrivate) {
  zone._inner = zone._inner.fork({
    name: 'angular',
    properties: <any>{
      'isAngularZone': true,
      'maybeDelayChangeDetection': shouldCoalesceEventChangeDetection 
          && delayChangeDetectionForEvents
    },

Possible implementation of delayChangeDetectionForEvents.

let nativeRequestAnimationFrame = requestAnimationFrame;
if (typeof Zone !== 'undefined') {
  // use unpatched version of requestAnimationFrame if possible
  // to avoid another Change detection
  const unpatchedRequestAnimationFrame = global[Zone.__symbol__('requestAnimationFrame')];
  if (unpatchedRequestAnimationFrame) {
    nativeRequestAnimationFrame = unpatchedRequestAnimationFrame;
  }
}
let lastRequestAnimationFrameId = null;
function delayChangeDetectionForEvents() {
  if (lastRequestAnimationFrameId !== null) {
    lastRequestAnimationFrameId = nativeRequestAnimationFrame(() => {
      lastRequestAnimationFrameId = null;
      change detection because the `Zone.js` has wrapped the `addEventListener` callback function with the task block. This behavior needs to be preserved.
      
      There are two ways to preserve it:
      1. We can patch the `click()` (zone); // Try to run change method to flussh change detection.
    });
  }
}

We would also need to update checkStable implementation like so:

function checkStable(zone: NgZonePrivate) {
  if (zone._nesting == 0 && !zone.hasPendingMicrotasks && !zone.isStable) {
    try {
      zone._nesting++;
      zone.onMicrotaskEmpty.emit(null);
    } finally {
      zone._nesting--;
      if (!zone.hasPendingMicrotasks 
          && lastRequestAnimationFrameId == null) { // ADD THIS LINE
        try {
          zone.runOutsideAngular(() => zone.onStable.emit(null));
        } finally {
          zone.isStable = true;
        }
      }
    }
  }
}

get hasPendingMicrotasks(): boolean {
  return this.hasPendingZoneMicrotasks ||
        (this.shouldCoalesceEventChangeDetection && this.lastRequestAnimationFrameId !== -1);
}

Finally we need to update the bootstrap API to allow the application to opt-in/out of this behavior in BootstrapOptions.

/**
 * Provides additional options to the bootstraping process.
 *
 *
 */
export interface BootstrapOptions {
  /**
   * Optionally specify which `NgZone` should be used.
   *
   * - Provide your own `NgZone` instance.
   * - `zone.js` - Use default `NgZone` which requires `Zone.js`.
   * - `noop` - Use `NoopNgZone` which does nothing.
   */
  ngZone?: NgZone|'zone.js'|'noop';
  
  // START<<<<<<<<<
  /**
   * Optionally specify if `NgZone` should be configure to coalesce
   * events.
   */
  ngZoneEventCoalescing?: true|false;
  // END<<<<<<<<<<<
}

Test Considerations

Currently when tests programmatically fire events they have their change detection executed synchronously.

buttonDomElement.click();

In the above case the click() invocation will invoke change detection because the Zone.js has wrapped the addEventListener callback function with the task block. This behavior needs to be preserved.

There are two ways to preserve it:

1. We can patch the click() and dispatchEvent method to flush the change detection synchronously in tests.
2. When event happens we can walk the tree and find the highest parent element on which we can attach the same element listener type using addEventListener (and auto clean up). When the bubbling event gets there we flush the changed-detection.
3. Can we just add an switch in ngZone to let user be able to enable/disable this coalescing behavior? Such as this,

function forkInnerZoneWithAngularBehavior(zone: NgZonePrivate, shouldCoalesceEventChangeDetection?: boolean, runtimeEnableCoalesceEventChangeDetection?: boolean) {
  zone._inner = zone._inner.fork({
    name: 'angular',
    properties: <any>{
      'isAngularZone': true,
      'runtimeEnableCoalesceEventChangeDetection': {
        'enabled': !!runtimeEnableCoalesceEventChangeDetection
      },
      'maybeDelayChangeDetection': shouldCoalesceEventChangeDetection 
          && delayChangeDetectionForEvents
    },

function decoratePreventDefault(eventHandler: Function): Function {
  return (event: any) => {
    // START: INSERT THIS CODE
    if (typeof Zone !== 'undefined') {
      const maybeDelayChangeDetection = Zone.current
                .get('maybeDelayChangeDetection');
      const runtimeEnableCoalesceEventChangeDetection = Zone.current.get('runtimeEnableCoalesceEventChangeDetection');
      runtimeEnableCoalesceEventChangeDetection && !!runtimeEnableCoalesceEventChangeDetection.enabled && 
      maybeDelayChangeDetection && maybeDelayChangeDetection();
    }

And we can disable this coalesceEvent behaviour in two ways,

1. in TestBed, we initialize the ngZone with runtimeEnableCoalesceEventChangeDetection = false
2. Or we can disable the specified case inside it

it('test without coalesce event', () => {
  const runtimeEnableCoalesceEventChangeDetection = Zone.current.get('runtimeEnableCoalesceEventChangeDetection'); 
  if (runtimeEnableCoalesceEventChangeDetection) {
    runtimeEnableCoalesceEventChangeDetection.enabled = false;
  }
})

NOTE: In the current API if the test schedules a micro-task Promise.resolve() then the change detection will not run because the microtask will prevent it. Not sure how many tests rely on such behavior.

Multiple Events Consideration

In browser an <input> text box will generate keydown followed by keypress. However when called programatically from tests a keydown will not generate keypress. I don't think there is anything special we should do in this case. We could try to coalesce keydown and keyup but getting it working correctly in tests seems like more trouble than it is worth

Recommendation

Current recommendation is that we use the requestAnimationFrame for coalescing browser native events. This will also work with Multiple Event Consideration use case.

We patch click() and dispatchEvent() method in @angular/core/testing so that any programmatically dispatched events would auto change-detect just as before. (Internally the patch would also have to cancel the requestAnimationFrame)

Notes

• All delay code needs to run outside of zone. (ie requestAnimationFrame.__zone_symbol__requestAnimationFrame)
• Related document DOM listeners and change detection
• Related WIP: performance tuning test by JiaLiPassion · Pull Request #29610 · angular/angular

Comments

Jia Li
And I think maybe the following case maybe the rare case(not sure how much requestAnimationFrame was used inside Material), if inside the event handlers, there are requestAnimationFrame calls, there will be still another change detection being triggered. In your new solution, we have take the checkStable code outside of the eventHandler, so maybe we can use ngZone to monitor if there are any requestAnimationFrame calls are scheduled during the bubble phase, we can reschedule those tasks into <root> Zone, so only one change detection happen finally.

The case looks like:

  <button (click)="doSomething()">click me</button>
</div>```
```doSomethingElse() {
  requestAnimationFrame(() => {});
}
doSomething() {
 requestAnimationFrame(() => {});
}```

Miško Hevery I don't think we should coalesce that as now we would be in the business of coalescing across multiple macro tasks.

Pawel Kozlowski
@jialipassion @misko @igorminar glad that we are discussing this, although I wanted to highlight that this is not only about bubbling of the same event - there are more cases where multiple change detections can happen in response to one single action of a user:

One target, different events:

<input (keypress)="noop()" (input)="noop()">

Multiple targets, same event:

<button (click)="noop()" (document:click)="noop()"></button>

Multiple targets, different events

<div (focusin)="noop()">
    <input (focus)="noop()">
</div>

Miško Hevery I don't think we should be doing anything special in this case, see Multiple Event Consideration section. Those really should be treated as different events for the purposes of testing.