Skip to content

Comments

fix(zone.js): should allow add passive/non-passive listeners together#49477

Closed
JiaLiPassion wants to merge 1 commit intoangular:mainfrom
JiaLiPassion:allow-passive-event
Closed

fix(zone.js): should allow add passive/non-passive listeners together#49477
JiaLiPassion wants to merge 1 commit intoangular:mainfrom
JiaLiPassion:allow-passive-event

Conversation

@JiaLiPassion
Copy link
Contributor

Close #45020
In the current version, if we add both passive and not passive listeners for the same eventName together, they will be registered with all passive or all non passive listeners depends on the order.

import 'zone.js';
div1.addEventListener('mousemove', (ev) => {}, { passive: true });
div1.addEventListener('mousemove', (ev) => {
ev.preventDefault(); // throws error since this one is also be registered as a passive event handler
});

div2.addEventListener('mousemove', (ev) => {
});
div2.addEventListener('mousemove', (ev) => {
ev.preventDefault(); // will not throw error since this one is also be registered as non passive event handler
}, { passive: true });

So this PR fix this issue and allow both passive and non-passive listeners registeration together whatever the order.

@JiaLiPassion JiaLiPassion added area: zones Issues related to zone.js target: minor This PR is targeted for the next minor release labels Mar 19, 2023
@ngbot ngbot bot modified the milestone: Backlog Mar 19, 2023
@JiaLiPassion JiaLiPassion force-pushed the allow-passive-event branch 3 times, most recently from a45dd8d to 13247d7 Compare March 25, 2023 11:47
Copy link
Contributor

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One NIT

@alan-agius4 alan-agius4 added target: patch This PR is targeted for the next patch release and removed target: minor This PR is targeted for the next minor release labels Dec 14, 2023
@alan-agius4 alan-agius4 removed the request for review from AndrewKushnir December 14, 2023 14:19
@alan-agius4
Copy link
Contributor

TGP

@alan-agius4 alan-agius4 added the action: global presubmit The PR is in need of a google3 global presubmit label Dec 14, 2023
@alan-agius4 alan-agius4 self-assigned this Dec 14, 2023
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change breaks a large number of apps in Google.

Error:

TypeError: task.callback.apply is not a function
    at _ZoneDelegate.invokeTask (third_party/javascript/angular2/rc/packages/zone.js/lib/zone.ts?l=1187

@alan-agius4 alan-agius4 added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Dec 14, 2023
@JiaLiPassion JiaLiPassion force-pushed the allow-passive-event branch 3 times, most recently from 320ff96 to 1cadb57 Compare December 16, 2023 21:52
@alan-agius4 alan-agius4 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 Dec 17, 2023
@alan-agius4
Copy link
Contributor

TGP

@JiaLiPassion JiaLiPassion force-pushed the allow-passive-event branch 2 times, most recently from 8caae4f to f174766 Compare December 18, 2023 14:08
In the current version, if we add both `passive` and `not passive` listeners for the
same eventName together, they will be registered with all passive or all non passive
listeners depends on the order.

```
import 'zone.js';
div1.addEventListener('mousemove', (ev) => {}, { passive: true });
div1.addEventListener('mousemove', (ev) => {
ev.preventDefault(); // throws error since this one is also be registered as a passive event handler
});

div2.addEventListener('mousemove', (ev) => {
});
div2.addEventListener('mousemove', (ev) => {
ev.preventDefault(); // will not throw error since this one is also be registered as non passive event handler
}, { passive: true });

```
So this PR fix this issue and allow both passive and non-passive listeners registeration
together whatever the order.

PR closes angular#45020
@alan-agius4
Copy link
Contributor

TGP

@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jul 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: global presubmit The PR is in need of a google3 global presubmit area: zones Issues related to zone.js target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

zone.js cause "Unable to preventDefault inside passive event listener invocation." "errors", if passive handler is added first

6 participants