After render app ref tick #52455

atscott · 2023-10-30T20:14:50Z

he afterRender hooks currently run after ApplicationRef.tick but
also run after any call to ChangeDetectorRef.detectChanges. This is
problematic because code which uses afterRender cannot expect the
component it's registered from to be rendered when the callback
executes. If there is a call to ChangeDetectorRef.detectChanges before
the global change detection, that will cause the hooks to run earlier
than expected.

This behavior is somewhat of a blocker for the zoneless project. There
is plenty of application code that do things like setTimeout(() => doSomethingThatExpectsComponentToBeRendered()), NgZone.onStable(() => ...) or ApplicationRef.onStable.... ApplicationRef.onStable
should likely work similarly, but all of these are really wanting an API
that is afterRender with the requirement that the hook runs after the
global render, not an individual CDRef instance.

This change updates the afterRender hooks to only run when
ApplicationRef.tick happens.

fixes #52429
fixes #53232

ngbot · 2024-01-04T21:47:14Z

I see that you just added the action: merge label, but the following checks are still failing:
     status "google-internal-tests" is failing
     status "pullapprove" is pending
     3 pending code reviews

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken main, please try rebasing to main and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

AndrewKushnir

Reviewed-for: public-api

jelbourn · 2024-01-05T18:14:17Z

goldens/public-api/core/errors.md

@@ -59,7 +59,7 @@ export const enum RuntimeErrorCode {
    // (undocumented)
    IMPORT_PROVIDERS_FROM_STANDALONE = 800,
    // (undocumented)
-    INFINITE_CHANGE_DETECTION = 103,
+    INFINITE_CHANGE_DETECTION = 102,


Should this change, or should error codes be permanent? (even if they're not documented?)

I'm happy to revert this. I think I had the same question in the past and I was informed that we do not commit to keeping the error codes stable. @AndrewKushnir @alxhub

I'm going to revert this error code change because I also agree with there being value in keeping them stable. Even if we decide in the meeting that it's not something we will do, there's really no reason to change it in this PR.

+1 to keep error codes stable (this is the main point of having golden files for them). I was under impression that those code represent similar conditions under which the error occurs, thus the change was ok. If it's not the case, let's keep the code stable.

packages/core/src/application/application_ref.ts

jelbourn

LGTM for now on the error code, but I added an item to the team meeting agenda to discuss at some point (I feel like there's some value in keeping them stable)

Reviewed-for: public-api

….tick`

atscott · 2024-01-08T19:30:31Z

This PR was merged into the repository by commit dfcf0d5.

…52455) The `afterRender` hooks currently run after `ApplicationRef.tick` but also run after any call to `ChangeDetectorRef.detectChanges`. This is problematic because code which uses `afterRender` cannot expect the component it's registered from to be rendered when the callback executes. If there is a call to `ChangeDetectorRef.detectChanges` before the global change detection, that will cause the hooks to run earlier than expected. This behavior is somewhat of a blocker for the zoneless project. There is plenty of application code that do things like `setTimeout(() => doSomethingThatExpectsComponentToBeRendered())`, `NgZone.onStable(() => ...)` or `ApplicationRef.onStable...`. `ApplicationRef.onStable` is a should likely work similarly, but all of these are really wanting an API that is `afterRender` with the requirement that the hook runs after the global render, not an individual CDRef instance. This change updates the `afterRender` hooks to only run when `ApplicationRef.tick` happens. fixes #52429 fixes #53232 PR Close #52455

…ngular#52455) The `afterRender` hooks currently run after `ApplicationRef.tick` but also run after any call to `ChangeDetectorRef.detectChanges`. This is problematic because code which uses `afterRender` cannot expect the component it's registered from to be rendered when the callback executes. If there is a call to `ChangeDetectorRef.detectChanges` before the global change detection, that will cause the hooks to run earlier than expected. This behavior is somewhat of a blocker for the zoneless project. There is plenty of application code that do things like `setTimeout(() => doSomethingThatExpectsComponentToBeRendered())`, `NgZone.onStable(() => ...)` or `ApplicationRef.onStable...`. `ApplicationRef.onStable` is a should likely work similarly, but all of these are really wanting an API that is `afterRender` with the requirement that the hook runs after the global render, not an individual CDRef instance. This change updates the `afterRender` hooks to only run when `ApplicationRef.tick` happens. fixes angular#52429 fixes angular#53232 PR Close angular#52455

angular-automatic-lock-bot · 2024-02-08T00:08:31Z

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

atscott added target: rc This PR is targeted for the next release-candidate area: core Issues related to the framework runtime labels Oct 30, 2023

ngbot bot added this to the Backlog milestone Oct 30, 2023

atscott marked this pull request as draft October 30, 2023 21:59

atscott modified the milestones: Backlog, v17-final Oct 30, 2023

devknoll mentioned this pull request Nov 1, 2023

afterRender hook called multiple times when there are multiple root views in the application #52429

Closed

atscott force-pushed the afterRenderAppRefTick branch 4 times, most recently from cd2e2ab to de2b943 Compare November 8, 2023 17:56

atscott added target: patch This PR is targeted for the next patch release and removed target: rc This PR is targeted for the next release-candidate labels Nov 8, 2023

atscott force-pushed the afterRenderAppRefTick branch from de2b943 to ae042ba Compare November 8, 2023 17:58

atscott requested a review from alxhub November 8, 2023 17:58

atscott marked this pull request as ready for review November 8, 2023 17:58

alxhub removed this from the v17-final milestone Nov 9, 2023

ngbot bot added this to the Backlog milestone Nov 9, 2023

atscott force-pushed the afterRenderAppRefTick branch 5 times, most recently from d8d8e31 to c344e24 Compare November 16, 2023 18:49

atscott force-pushed the afterRenderAppRefTick branch 7 times, most recently from 97dd35f to bc3118b Compare December 14, 2023 17:21

atscott added the action: merge The PR is ready for merge by the caretaker label Jan 4, 2024

atscott removed request for pkozlowski-opensource, alxhub and jessicajaniuk January 4, 2024 23:40

pullapprove bot requested review from AndrewKushnir, dylhunn and pkozlowski-opensource January 4, 2024 23:40

atscott mentioned this pull request Jan 4, 2024

refactor(cdk/overlay): Reduce dependency on NgZone angular/components#28332

Merged

AndrewKushnir approved these changes Jan 5, 2024

View reviewed changes

pullapprove bot requested a review from jessicajaniuk January 5, 2024 17:58

jelbourn reviewed Jan 5, 2024

View reviewed changes

atscott force-pushed the afterRenderAppRefTick branch from 89d8cf7 to c6b59c0 Compare January 5, 2024 18:32

jelbourn approved these changes Jan 5, 2024

View reviewed changes

fixup! fix(core): afterRender hooks now only run on `ApplicationRef…

a87a3e1

….tick`

atscott force-pushed the afterRenderAppRefTick branch from c6b59c0 to a87a3e1 Compare January 5, 2024 18:42

atscott removed request for dylhunn, pkozlowski-opensource and jessicajaniuk January 8, 2024 17:29

atscott closed this in dfcf0d5 Jan 8, 2024

mateusbeckup mentioned this pull request Feb 2, 2024

[Snyk] Upgrade zone.js from 0.14.0 to 0.14.3 mateusbeckup/angular#1

Open

angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

After render app ref tick #52455

After render app ref tick #52455

atscott commented Oct 30, 2023 •

edited

ngbot bot commented Jan 4, 2024

AndrewKushnir left a comment

jelbourn Jan 5, 2024

atscott Jan 5, 2024

atscott Jan 5, 2024

AndrewKushnir Jan 5, 2024

jelbourn left a comment

atscott commented Jan 8, 2024

angular-automatic-lock-bot bot commented Feb 8, 2024

After render app ref tick #52455

After render app ref tick #52455

Conversation

atscott commented Oct 30, 2023 • edited

ngbot bot commented Jan 4, 2024

AndrewKushnir left a comment

Choose a reason for hiding this comment

jelbourn Jan 5, 2024

Choose a reason for hiding this comment

atscott Jan 5, 2024

Choose a reason for hiding this comment

atscott Jan 5, 2024

Choose a reason for hiding this comment

AndrewKushnir Jan 5, 2024

Choose a reason for hiding this comment

jelbourn left a comment

Choose a reason for hiding this comment

atscott commented Jan 8, 2024

angular-automatic-lock-bot bot commented Feb 8, 2024

atscott commented Oct 30, 2023 •

edited