Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(core): Omit listeners from out-of-zone scheduling when using… #55525

Closed
wants to merge 1 commit into from

Conversation

atscott
Copy link
Contributor

@atscott atscott commented Apr 24, 2024

… ZoneJS (#55492)

In Angular today, a bound listener automatically marks the view for check. When using ZoneJS, these listeners are most often executed in the Angular Zone as well, so synchronization (ApplicationRef.tick) will eventually happen. However, developers can opt out of zone-patching for events in several ways, and often do this for very frequent listeners like mousemove, resize, and scroll. We do not want to break existing expectations that these are now "safe" events to have listeners for by automatically scheduling change detection regardless of whether the listener executed inside or outside the Angular zone.

In contrast, in order for developers to more easily transition to zoneless, we need to be able to ensure that components which are using OnPush are, for the most part, compatible with zoneless as well. Because listeners automatically mark the component for check, developers using OnPush did not/do not need to also call ChangeDetectorRef.markForCheck or a similar API. Unfortunately, this means that we need to consider the listener callbacks as a notification to schedule a tick when Zoneless is enabled. In the future, we would like to have an opt-out for this (i.e. signal components) since it's not really how we want things to work.

Also includes the fix for #54919 that got reverted only because it was easier to revert the set of conflicting commits

… ZoneJS (angular#55492)

In Angular today, a bound listener automatically marks the view for
check. When using ZoneJS, these listeners are most often executed in the
Angular Zone as well, so synchronization (`ApplicationRef.tick`) will
eventually happen. _However_, developers can opt out of zone-patching
for events in several ways, and often do this for very frequent
listeners like `mousemove`, `resize`, and `scroll`. We do not want to
break existing expectations that these are now "safe" events to have
listeners for by automatically scheduling change detection regardless of
whether the listener executed inside or outside the Angular zone.

In contrast, in order for developers to more easily transition to zoneless,
we need to be able to ensure that components which are using `OnPush`
are, for the most part, compatible with zoneless as well. Because listeners
automatically mark the component for check, developers using `OnPush`
did not/do not need to also call `ChangeDetectorRef.markForCheck` or a
similar API. Unfortunately, this means that we need to consider the
listener callbacks as a notification to schedule a `tick` when Zoneless
is enabled. In the future, we would like to have an opt-out for this
(i.e. signal components) since it's not really how we _want_ things to work.

Also includes the fix for angular#54919 that got reverted only because it was
easier to revert the set of conflicting commits
@atscott atscott added the target: minor This PR is targeted for the next minor release label Apr 24, 2024
@pullapprove pullapprove bot requested a review from alxhub April 24, 2024 23:42
@atscott atscott added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: rc This PR is targeted for the next release-candidate and removed target: minor This PR is targeted for the next minor release labels Apr 25, 2024
@atscott
Copy link
Contributor Author

atscott commented Apr 25, 2024

Caretaker note: this is a roll forward of changes that were reverted. These changes did not contribute to the g3 failures.

@AndrewKushnir AndrewKushnir removed the request for review from alxhub April 25, 2024 19:56
@AndrewKushnir
Copy link
Contributor

This PR was merged into the repository by commit bf8814c.

AndrewKushnir pushed a commit that referenced this pull request Apr 25, 2024
… ZoneJS (#55492) (#55525)

In Angular today, a bound listener automatically marks the view for
check. When using ZoneJS, these listeners are most often executed in the
Angular Zone as well, so synchronization (`ApplicationRef.tick`) will
eventually happen. _However_, developers can opt out of zone-patching
for events in several ways, and often do this for very frequent
listeners like `mousemove`, `resize`, and `scroll`. We do not want to
break existing expectations that these are now "safe" events to have
listeners for by automatically scheduling change detection regardless of
whether the listener executed inside or outside the Angular zone.

In contrast, in order for developers to more easily transition to zoneless,
we need to be able to ensure that components which are using `OnPush`
are, for the most part, compatible with zoneless as well. Because listeners
automatically mark the component for check, developers using `OnPush`
did not/do not need to also call `ChangeDetectorRef.markForCheck` or a
similar API. Unfortunately, this means that we need to consider the
listener callbacks as a notification to schedule a `tick` when Zoneless
is enabled. In the future, we would like to have an opt-out for this
(i.e. signal components) since it's not really how we _want_ things to work.

Also includes the fix for #54919 that got reverted only because it was
easier to revert the set of conflicting commits

PR Close #55525
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note PullApprove: disable target: rc This PR is targeted for the next release-candidate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants