fix(platform-browser): do not run change detection when loading Hammer (#44921)

The Hammer.JS might be loaded asynchronously if the `HAMMER_LOADER` token is provided.
Its loading causes additional change detection cycles when the script `load` event is fired
and the `import()` promise resolves. Its loading doesn't have to require Angular running `tick()`
since the Hammer instance is created outside of the Angular zone anyway.

BREAKING CHANGE:

This change may cause a breaking change in unit tests that are implicitly depending on a specific
number and sequence of change detections in order for their assertions to pass.

PR Close #44921
This commit is contained in:
arturovt
2022-02-01 09:31:13 +02:00
committed by Jessica Janiuk
parent 39b90b46b6
commit a01bcb8e7e
2 changed files with 49 additions and 28 deletions
@@ -195,7 +195,7 @@ export class HammerGesturesPlugin extends EventManagerPlugin {
// If Hammer is not present but a loader is specified, we defer adding the event listener
// until Hammer is loaded.
if (!(window as any).Hammer && this.loader) {
this._loaderPromise = this._loaderPromise || this.loader();
this._loaderPromise = this._loaderPromise || zone.runOutsideAngular(() => this.loader!());
// This `addEventListener` method returns a function to remove the added listener.
// Until Hammer is loaded, the returned function needs to *cancel* the registration rather
// than remove anything.
@@ -204,32 +204,34 @@ export class HammerGesturesPlugin extends EventManagerPlugin {
cancelRegistration = true;
};
this._loaderPromise
.then(() => {
// If Hammer isn't actually loaded when the custom loader resolves, give up.
if (!(window as any).Hammer) {
if (typeof ngDevMode === 'undefined' || ngDevMode) {
this.console.warn(
`The custom HAMMER_LOADER completed, but Hammer.JS is not present.`);
}
deregister = () => {};
return;
}
zone.runOutsideAngular(
() => this._loaderPromise!
.then(() => {
// If Hammer isn't actually loaded when the custom loader resolves, give up.
if (!(window as any).Hammer) {
if (typeof ngDevMode === 'undefined' || ngDevMode) {
this.console.warn(
`The custom HAMMER_LOADER completed, but Hammer.JS is not present.`);
}
deregister = () => {};
return;
}
if (!cancelRegistration) {
// Now that Hammer is loaded and the listener is being loaded for real,
// the deregistration function changes from canceling registration to removal.
deregister = this.addEventListener(element, eventName, handler);
}
})
.catch(() => {
if (typeof ngDevMode === 'undefined' || ngDevMode) {
this.console.warn(
`The "${eventName}" event cannot be bound because the custom ` +
`Hammer.JS loader failed.`);
}
deregister = () => {};
});
if (!cancelRegistration) {
// Now that Hammer is loaded and the listener is being loaded for real,
// the deregistration function changes from canceling registration to
// removal.
deregister = this.addEventListener(element, eventName, handler);
}
})
.catch(() => {
if (typeof ngDevMode === 'undefined' || ngDevMode) {
this.console.warn(
`The "${eventName}" event cannot be bound because the custom ` +
`Hammer.JS loader failed.`);
}
deregister = () => {};
}));
// Return a function that *executes* `deregister` (and not `deregister` itself) so that we
// can change the behavior of `deregister` once the listener is added. Using a closure in
@@ -5,8 +5,8 @@
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {NgZone} from '@angular/core';
import {fakeAsync, inject, tick} from '@angular/core/testing';
import {ApplicationRef, NgZone} from '@angular/core';
import {fakeAsync, inject, TestBed, tick} from '@angular/core/testing';
import {EventManager} from '@angular/platform-browser';
import {HammerGestureConfig, HammerGesturesPlugin,} from '@angular/platform-browser/src/dom/events/hammer_gestures';
@@ -67,6 +67,7 @@ import {HammerGestureConfig, HammerGesturesPlugin,} from '@angular/platform-brow
}));
let loaderCalled = 0;
let loaderIsCalledInAngularZone: boolean|null = null;
beforeEach(() => {
originalHammerGlobal = (window as any).Hammer;
@@ -79,6 +80,7 @@ import {HammerGestureConfig, HammerGesturesPlugin,} from '@angular/platform-brow
loader = () => {
loaderCalled++;
loaderIsCalledInAngularZone = NgZone.isInAngularZone();
return new Promise((resolve, reject) => {
resolveLoader = resolve;
failLoader = reject;
@@ -169,6 +171,23 @@ import {HammerGestureConfig, HammerGesturesPlugin,} from '@angular/platform-brow
.toHaveBeenCalledWith(
`The custom HAMMER_LOADER completed, but Hammer.JS is not present.`);
}));
it('should call the loader outside of the Angular zone', fakeAsync(() => {
const ngZone = TestBed.inject(NgZone);
// Unit tests are being run in a ProxyZone, thus `addEventListener` is called within the
// ProxyZone. In real apps, `addEventListener` is called within the Angular zone; we
// mimic that behaviour by entering the Angular zone.
ngZone.run(() => plugin.addEventListener(someElement, 'swipe', () => {}));
const appRef = TestBed.inject(ApplicationRef);
spyOn(appRef, 'tick');
resolveLoader();
tick();
expect(appRef.tick).not.toHaveBeenCalled();
expect(loaderIsCalledInAngularZone).toEqual(false);
}));
});
});
}