feat: support passive events by define global variables in zone.js config file by JiaLiPassion · Pull Request #34503 · angular/angular

JiaLiPassion · 2019-12-19T22:59:37Z

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: #8866

It is not easy to addEventListener with passive option. We may need to discuss to add something like in this PR, #21681 in the future.

What is the new behavior?

For now, this PR is just a temp solution until we can add a new template syntax in event listener.
In this PR, we can define a global variable before importing zone.js

(window as any)['__zone_symbol__PASSIVE_EVENTS'] = ['touchstart', 'scroll'];

and then all event listeners defined here will be automatically be passive: true.

Does this PR introduce a breaking change?

Yes
No

If this idea is ok, I will update the document. And of course we still need to discuss the formal way to resolve this issue by introducing some new options in template event binding syntax.

JiaLiPassion · 2019-12-20T01:58:08Z

@mhevery, please check whether this temp solution is ok, thank you!

mhevery · 2020-01-29T23:00:08Z

Looks like a reasonable approach. Needs documentation explaining how to use it.

JiaLiPassion · 2020-01-30T04:33:36Z

@mhevery, thanks for the review, I have updated the PR and add the document.

mhevery · 2020-02-11T18:00:23Z

presubmit

JiaLiPassion · 2020-02-17T06:01:21Z

@kara, I have rebased the PR, please review, thanks.

mhevery · 2020-02-20T19:21:37Z

presubmit

kara

Can we also add a descriptive commit message that explains why this change is necessary?

kara · 2020-02-21T21:34:52Z

before import zone.js => before importing zone.js

Got it, updated, thanks.

kara · 2020-02-21T21:37:24Z

This is a somewhat large increase. Do we know why? Can we find out how much is just accumulation?

I agree, this seems excessive especially for the diff in this PR. Let's find out how did we accumulate this much (I've reset the polyfills limits recently so it seems like a recent accumulation of code)

mhevery · 2020-02-21T22:13:48Z

This was accidentally merged. Reverting until we have better docs review.

…les in zone.js config file (#34503)" This reverts commit d7d359e.

JiaLiPassion · 2020-02-22T06:29:00Z

@kara, @mhevery , I have updated the doc and remove some unused code. Now the size will have no changes. Please review, thanks.

mhevery · 2020-02-24T22:00:38Z

presubmit

kara

Thanks for updating the PR! There are still a few things to do:

Update the commit message to describe what problem this PR is fixing and why (should not just be one sentence).
A few more docs updates below to fix

kara · 2020-02-24T22:47:29Z

Suggested change

Angular also supports passive event listeners. For example, you can use the following steps to make scroll event passive.

Angular also supports passive event listeners. For example, you can use the following steps to make the scroll event passive.

Got it, thanks, updated.

kara · 2020-02-24T22:47:41Z

Suggested change

3. In the `src/polyfills.ts` file, before importing zone.js, import the new created `zone-flags`.

3. In the `src/polyfills.ts` file, before importing zone.js, import the newly created `zone-flags`.

Got it, updated, thanks.

Now Angular doesn't support add event listeners as passive very easily. User needs to use `elem.addEventListener('scroll', listener, {passive: true});` or implements their own EventManagerPlugin to do that. Angular may finally support new template syntax to support passive event, for now, this commit introduces a temp solution to allow user to define the passive event names in zone.js configurations. User can define a global varibale like this. ``` (window as any)['__zone_symbol__PASSIVE_EVENTS'] = ['scroll']; ``` to let all `scroll` event listeners passive.

JiaLiPassion · 2020-02-25T00:55:39Z

@kara, thank you for the review, I have updated the doc and the commit message. Please review.

kara

LGTM, thanks for updating the PR @JiaLiPassion!

sod · 2020-03-04T13:43:19Z

What's the release schedule for zone.js? I ask as I saw quite some changes over time, but the last release was 8 months ago (0.10.2 from August 14, 2019 - https://yarnpkg.com/package/zone.js). Quite interesting that all changes are mentioned in the angular changelog anyway.

JiaLiPassion · 2020-03-05T06:50:36Z

@sod, there should be a new version very soon.

angular-automatic-lock-bot · 2020-04-05T16:05:18Z

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.}

JiaLiPassion requested a review from a team December 19, 2019 22:59

googlebot added the cla: yes label Dec 19, 2019

JiaLiPassion requested a review from a team December 19, 2019 23:57

JiaLiPassion requested a review from mhevery December 20, 2019 01:58

atscott added the area: zones Issues related to zone.js label Jan 9, 2020

ngbot Bot added this to the needsTriage milestone Jan 9, 2020

mhevery suggested changes Jan 29, 2020

View reviewed changes

Comment thread packages/zone.js/test/browser/browser.spec.ts Outdated

Comment thread packages/zone.js/lib/common/events.ts Outdated

Comment thread packages/zone.js/lib/common/events.ts Outdated

Comment thread packages/zone.js/lib/common/events.ts Outdated

JiaLiPassion force-pushed the passive branch 2 times, most recently from b543ecb to d2909ca Compare January 30, 2020 03:26

JiaLiPassion requested a review from mhevery January 30, 2020 04:33

mhevery suggested changes Jan 31, 2020

View reviewed changes

Comment thread packages/zone.js/lib/common/events.ts Outdated

Comment thread packages/zone.js/test/browser/browser.spec.ts Outdated

Comment thread integration/_payload-limits.json Outdated

Comment thread packages/zone.js/test/browser/browser.spec.ts Outdated

JiaLiPassion force-pushed the passive branch 2 times, most recently from 3525ed1 to 203918b Compare February 5, 2020 13:49

JiaLiPassion added the target: patch This PR is targeted for the next patch release label Feb 5, 2020

JiaLiPassion requested a review from mhevery February 5, 2020 22:47

JiaLiPassion force-pushed the passive branch from 203918b to 1780454 Compare February 6, 2020 04:01

mhevery approved these changes Feb 11, 2020

View reviewed changes

mhevery added the action: merge The PR is ready for merge by the caretaker label Feb 11, 2020

kara added target: major This PR is targeted for the next major release action: review The PR is still awaiting reviews from at least one requested reviewer and removed target: patch This PR is targeted for the next patch release labels Feb 11, 2020

JiaLiPassion force-pushed the passive branch 2 times, most recently from 623fe10 to feb7db8 Compare February 15, 2020 05:44

mhevery removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Feb 20, 2020

kara reviewed Feb 21, 2020

View reviewed changes

mhevery reopened this Feb 21, 2020

mhevery added a commit that referenced this pull request Feb 21, 2020

Revert "feat: support passive event options by defining global variab…

e084835

…les in zone.js config file (#34503)" This reverts commit d7d359e.

mhevery added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews area: core Issues related to the framework runtime labels Feb 22, 2020

JiaLiPassion force-pushed the passive branch 6 times, most recently from 063165d to 87b34e0 Compare February 22, 2020 06:07

JiaLiPassion removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Feb 22, 2020

JiaLiPassion requested a review from kara February 22, 2020 06:29

JiaLiPassion force-pushed the passive branch from 87b34e0 to 3806892 Compare February 22, 2020 06:54

mhevery removed the request for review from kara February 24, 2020 21:59

kara suggested changes Feb 24, 2020

View reviewed changes

JiaLiPassion force-pushed the passive branch from 3806892 to d46182b Compare February 25, 2020 00:55

JiaLiPassion requested a review from kara February 25, 2020 00:55

kara approved these changes Feb 25, 2020

View reviewed changes

mhevery closed this in f9d483e Feb 25, 2020

AndrewKushnir mentioned this pull request Mar 3, 2020

build: adding a script to compare commits in master and stable branches #35130

Closed

3 tasks

angular-automatic-lock-bot Bot locked and limited conversation to collaborators Apr 5, 2020

	Angular also supports passive event listeners. For example, you can use the following steps to make scroll event passive.
	Angular also supports passive event listeners. For example, you can use the following steps to make the scroll event passive.

	3. In the `src/polyfills.ts` file, before importing zone.js, import the new created `zone-flags`.
	3. In the `src/polyfills.ts` file, before importing zone.js, import the newly created `zone-flags`.

Conversation

JiaLiPassion commented Dec 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Uh oh!

JiaLiPassion commented Dec 20, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mhevery commented Jan 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JiaLiPassion commented Jan 30, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mhevery commented Feb 11, 2020

Uh oh!

JiaLiPassion commented Feb 17, 2020

Uh oh!

mhevery commented Feb 20, 2020

Uh oh!

kara left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mhevery commented Feb 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JiaLiPassion commented Feb 22, 2020

Uh oh!

mhevery commented Feb 24, 2020

Uh oh!

kara left a comment • edited 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 Feb 25, 2020

Uh oh!

kara left a comment

Choose a reason for hiding this comment

Uh oh!

sod commented Mar 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JiaLiPassion commented Mar 5, 2020

Uh oh!

angular-automatic-lock-bot Bot commented Apr 5, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

JiaLiPassion commented Dec 19, 2019 •

edited

Loading

mhevery commented Jan 29, 2020 •

edited

Loading

mhevery commented Feb 21, 2020 •

edited

Loading

kara left a comment •

edited

Loading

sod commented Mar 4, 2020 •

edited

Loading