feat(platform-browser): can config zone/once/passive/capture in event listener by JiaLiPassion · Pull Request #38236 · angular/angular

JiaLiPassion · 2020-07-26T15:29:18Z

Close #19878

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

What is the current behavior?

Issue Number: 19878

What is the new behavior?

can't pass `passive`, `capture`, `once` parameters to `addEventListener`.

not easy to config use ngzone or noopzone with HostListener

user need to explicitly call runOutsideOfAngular to make sure eventhandler run outsideofngZone

@HostListener('window:resize', ['$event.target'])
  onResize(target: any) {
    this.ngZone.runOutsideOfAngular(() => {
       console.log('resize triggered');
    });
  }

or call ngZone.run explicitly make sure eventhandler run into ngZone.

@HostListener('window:resize', ['$event.target'])
  onResize(target: any) {
    this.ngZone.run(() => {
       console.log('resize triggered');
    });
  }

not easy to run event handler outsideOfAngular in template

<div (mouseover)="mouseover();"></div>

mouseover() {
  this.ngZone.runOfAngular(() => {
    ...
  });
}

What is the new behavior?

Can config which zone will event handler will run into in @HostListener decorator.

can config `passive`, `capture`, `once` parameters to `addEventListener`.

in template, can config those parameter like below.

<div (mouseover.capture.once.passive)="mouseover();"></div>

can config `ngzone` which will guarantee eventhandler run in `ngZone` even the handler was

added not in ngZone.

 @HostListener('window:resize.ngzone', ['$event.target'])
  onResize(target: any) {
    console.log('resize triggered');
  }

can config noopzone which will guarantee eventhandler run outside of ngZone even the handler was added in ngZone. and if config outsideOfAngular, DOMEventManager will bypass zone.js and directly use native addEventListener to improve performance.

And it can also work with #20672 to get better performance.

 @HostListener('window:resize.noopzone', ['$event.target'])
  onResize(target: any) {
    console.log('resize triggered');
  }

mhevery

The approach seems reasonable:

Needs tests
Needs documentation (so that developers can discover the feature)

mhevery · 2020-07-27T23:20:41Z

If I understand correctly the getZoneEventHandler will create a wrapper for the zone to run it int correct zone. If that is the case I don't think that is right approach, because the event will still get registered current zone. When event fires it will fire in the current zone and than execute the event handler in the target zone. What I think you want instead is just to register the event in the correct zone to begin with. (In other words the registration should be done in the correct zone, not the execution)

Yeah, you are right, I am following the way key_events_plugin does, I will update this part. Thanks.

mhevery · 2020-07-27T23:25:18Z

I would use !NgZone.isInAngularZone() to determine that.

Got it, I will update this part.

JiaLiPassion · 2020-07-27T23:34:30Z

@mhevery , I will continue to work on this PR and add tests and doc. thanks!

mary-poppins · 2020-07-29T13:54:40Z

You can preview 86b974c at https://pr38236-86b974c.ngbuilds.io/.
You can preview 1860e5d at https://pr38236-1860e5d.ngbuilds.io/.
You can preview d9f374b at https://pr38236-d9f374b.ngbuilds.io/.
You can preview 81f950b at https://pr38236-81f950b.ngbuilds.io/.
You can preview 0efb517 at https://pr38236-0efb517.ngbuilds.io/.

mary-poppins · 2020-07-29T14:04:51Z

You can preview b7cfed9 at https://pr38236-b7cfed9.ngbuilds.io/.

@HostListener

… listener. Close angular#19878 1. Can't pass `passive`, `capture`, `once` parameters to `addEventListener`. 2. Not easy to config use ngzone or noopzone with HostListener user need to explicitly call `runOutsideOfAngular` to make sure `eventhandler` run outsideof`ngZone` ``` @HostListener('window:resize', ['$event.target']) onResize(target: any) { this.ngZone.runOutsideOfAngular(() => { console.log('resize triggered'); }); } ``` or call `ngZone.run` explicitly make sure `eventhandler` run into `ngZone`. ``` @HostListener('window:resize', ['$event.target']) onResize(target: any) { this.ngZone.run(() => { console.log('resize triggered'); }); } ``` ``` <div (mouseover)="mouseover();"></div> ``` ``` mouseover() { this.ngZone.runOfAngular(() => { ... }); } ``` 3. Can config which `zone` will event handler will run into in `@HostListener` decorator. 4. can config `passive`, `capture`, `once` parameters to `addEventListener`. - in template, can config those parameter like below. ``` <div (mouseover.capture.once.passive)="mouseover();"></div> ``` even the handler was added not in `ngZone`. ``` <div (mouseover.ngZone)="mouseover();"></div> ``` ``` @HostListener('window:resize.ngZone', ['$event.target']) onResize(target: any) { console.log('resize triggered'); } ``` - Can config `noopZone` which will guarantee eventhandler run outside of `ngZone` even the handler was added in `ngZone`. And it can also work with angular#20672 to get better performance. ``` <div (mouseover.noopZone)="mouseover();"></div> ``` ``` @HostListener('window:resize.noopZone', ['$event.target']) onResize(target: any) { console.log('resize triggered'); } ```

mary-poppins · 2020-07-29T14:37:25Z

You can preview d20b7d0 at https://pr38236-d20b7d0.ngbuilds.io/.

mhevery · 2020-07-29T18:30:01Z

+import {Item} from './item';

-  currentItem = { name: 'teapot'} ;
+@Component(


This is example code. It should include comments explaining what you are demonstrating in each of the cases. Otherwise the reader does not know why you are showing this code to them.

mhevery · 2020-07-29T18:30:43Z

+
+And it is also possible to apply `ngZone` or `noopZone` option to make sure the event handler run into the specified `zone`.
+
+- `ngZone`: The event handler always runs into the `angular zone`, so it triggers the change detection automatically.


Suggested change

- `ngZone`: The event handler always runs into the `angular zone`, so it triggers the change detection automatically.

- `ngZone`: The event handler always runs in the `angular zone`, therefore change detection is triggered automatically.

Since everything is in ngZone already why would you ever use this option?

mhevery · 2020-07-29T18:31:17Z

+And it is also possible to apply `ngZone` or `noopZone` option to make sure the event handler run into the specified `zone`.
+
+- `ngZone`: The event handler always runs into the `angular zone`, so it triggers the change detection automatically.
+- `noopZone`: The event handler always runs outside of the `angular zone`, so it does not trigger the change detection automatically.


Suggested change

- `noopZone`: The event handler always runs outside of the `angular zone`, so it does not trigger the change detection automatically.

- `noopZone`: The event handler always runs outside of the `angular zone`, therefore no change detection is triggered.

Thinking more about this, I don't think we want to tie the options to the zone keywords as zones are implementation details not intention. So I think a better thing would be to say something like click.noCD or something like that to convey the intention that I don't want CD to happen here.

Possible candidates:

noCD

manual (I am leaning towards this one)

!CD

raw

exclude

hidden

mask

I agree with Misko and in addition, I prefer adding ng prefix for Angular-specific flags which isn't standards in DOM event context.
Or, I'm guessing maybe it shouldn't address Zone's concern in this feature because it is actually not needed to solve the problem in a single way. For example, scoping events inside/outside Zones, I think there are other approaches like the below. How do you think?

<ng-container *ngOutsideZone> <div (click)="clickInNoopZone()"></div> </ng-container>

In summary, I'm guessing "Passing DOM event handling options in Angular templates" and "Controlling zones on event binding" are able to separate.

@mhevery, @lacolaco , yeah, I agree that zone is the implementation detail, we need to consider another names.

And this idea is interesting,

<ng-container *ngOutsideZone> <div (click)="clickInNoopZone()"></div> </ng-container>

But it may be difficult if I want to only make some event outside of ngZone.

<ng-container *ngOutsideZone> <div (scroll)="scrollOutsideOfZone()" (click)="clickInsideNgZone()"></div> </ng-container>

But it may be difficult if I want to only make some event outside of ngZone.

A-ha! Totally agreed. I understood that Zone configuration must be an event binding syntax.

mhevery · 2020-07-29T18:31:57Z

+
+And it is also possible to apply `ngZone` or `noopZone` option to make sure the event handler run into the specified `zone`.
+
+- `ngZone`: The event handler always runs into the `angular zone`, so it triggers the change detection automatically.


you should enumerate all of the possible options including capture, passive, once in this list.

mhevery · 2020-07-29T18:32:52Z


+## Apply `capture`, `passive`, `once`, `ngZone`, `noopZone` options
+
+It is possible to apply [AddEventListenerOptions](https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener) `capture`, `passive`, `once` by combining the event name with the option.


this text should include instructions on syntax of combining multiple options.

mhevery · 2020-07-29T18:35:40Z

    getZone(): NgZone;
 }

+export declare interface EventManagerPluginOptions {


Suggested change

export declare interface EventManagerPluginOptions {

export declare interface EventOptions {

I think shorter names are better. In this case the options are for the event (or maybe event handler?)

mhevery · 2020-07-29T18:37:13Z

+import {EventManagerPluginOptions} from './event_manager';
+
+/**
+ *


needs jsdocs

mhevery · 2020-07-29T18:43:04Z

  }
+
+  /** @internal */
+  _splitEventNameAndOptions(eventName: string):


I would like to see a unit test specifically for this function where we feed it different event names and see how they get parsed.

Current syntax is:

eventName

window:evenName

Proposed syntax is:

eventName.option1.option2

window:evenName.option1.option2

It does not seem very consistent that window is a prefix but the other option are suffix. I think we should unified it.

option1,option2:eventName

window,option1,option2:eventName

Example:
window,passive,noCD:scroll would mean register a passive scroll listener on window with no change-detection.

We should change the signature of ɵɵlistener

export function ɵɵlistener( eventName: string, listenerFn: (e?: any) => any, useCapture = false, eventTargetResolver?: GlobalTargetResolver): typeof ɵɵlistener {

from useCapture=false to a bit flags representing the options. This would require a compiler change but it would be consistent and would allow for possible tree shaking benefits.

How is (eventTarget:eventName:options)="..." instead of . chaining? Because its signature order is similar to JavaScript API. And existing pipe parameters syntax is also like that.

window:resize:option1,option2 // JS API window.addEventListener(resize, ..., {option1,option2}) // Pipe parameters {{ prop | date:'short' }}

Optional arguments are typically placed at the last position in many languages and contexts. I guess inserting in the middle looks complex and not intuitive a little.

I totally get where you are coming from and agree with you. However given that we already have window:event which is a form of an option I would prefer syntax which is consistent with existing syntax. But we will have an official RFC to decide on the syntax to come.

mhevery · 2020-11-18T00:21:14Z

I am going to close this PR in the interest of keeping our PR queue clean. Per discussions in team meetings, we want this feature, but the current approach is not ideal, as we would like a more encompassing feature.

angular-automatic-lock-bot · 2020-12-19T16:39:08Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: yes label Jul 26, 2020

JiaLiPassion marked this pull request as draft July 26, 2020 15:29

pullapprove Bot requested a review from AndrewKushnir July 26, 2020 15:29

JiaLiPassion removed the request for review from AndrewKushnir July 26, 2020 15:29

JiaLiPassion mentioned this pull request Jul 26, 2020

WIP(compiler): fix #19878, can config zone/once/passive/capture in template/HostListener decorator #21681

Closed

3 tasks

mhevery self-assigned this Jul 27, 2020

mhevery reviewed Jul 27, 2020

View reviewed changes

mhevery added the area: zones Issues related to zone.js label Jul 28, 2020

ngbot Bot added this to the needsTriage milestone Jul 28, 2020

JiaLiPassion force-pushed the event-modifier branch 3 times, most recently from 0efb517 to b7cfed9 Compare July 29, 2020 13:53

JiaLiPassion marked this pull request as ready for review July 29, 2020 13:54

pullapprove Bot requested a review from kyliau July 29, 2020 13:54

JiaLiPassion added the aio: preview label Jul 29, 2020

JiaLiPassion added the target: major This PR is targeted for the next major release label Jul 29, 2020

JiaLiPassion force-pushed the event-modifier branch from b7cfed9 to d20b7d0 Compare July 29, 2020 14:25

mhevery suggested changes Jul 29, 2020

View reviewed changes

JiaLiPassion changed the title ~~feat(platform-browser): can config zone/once/passive/capture in event listenre~~ feat(platform-browser): can config zone/once/passive/capture in event listener Jul 30, 2020

mhevery mentioned this pull request Jul 30, 2020

Change detection not triggering for template event handler #37062

Closed

JiaLiPassion marked this pull request as draft August 22, 2020 14:40

mhevery closed this Nov 18, 2020

angular-automatic-lock-bot Bot locked and limited conversation to collaborators Dec 19, 2020


		And it is also possible to apply `ngZone` or `noopZone` option to make sure the event handler run into the specified `zone`.

		- `ngZone`: The event handler always runs into the `angular zone`, so it triggers the change detection automatically.

	- `noopZone`: The event handler always runs outside of the `angular zone`, so it does not trigger the change detection automatically.
	- `noopZone`: The event handler always runs outside of the `angular zone`, therefore no change detection is triggered.


		## Apply `capture`, `passive`, `once`, `ngZone`, `noopZone` options

		It is possible to apply [AddEventListenerOptions](https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener) `capture`, `passive`, `once` by combining the event name with the option.

	export declare interface EventManagerPluginOptions {
	export declare interface EventOptions {

Conversation

JiaLiPassion commented Jul 26, 2020

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

can't pass passive, capture, once parameters to addEventListener.

not easy to config use ngzone or noopzone with HostListener

not easy to run event handler outsideOfAngular in template

What is the new behavior?

can config passive, capture, once parameters to addEventListener.

can config ngzone which will guarantee eventhandler run in ngZone even the handler was

Uh oh!

mhevery left a comment • edited by JiaLiPassion Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JiaLiPassion commented Jul 27, 2020

Uh oh!

mary-poppins commented Jul 29, 2020

Uh oh!

mary-poppins commented Jul 29, 2020

Uh oh!

mary-poppins commented Jul 29, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mhevery commented Nov 18, 2020

Uh oh!

angular-automatic-lock-bot Bot commented Dec 19, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

can't pass `passive`, `capture`, `once` parameters to `addEventListener`.

can config `passive`, `capture`, `once` parameters to `addEventListener`.

can config `ngzone` which will guarantee eventhandler run in `ngZone` even the handler was

mhevery left a comment •

edited by JiaLiPassion

Loading