Skip to content

Conversation

arturovt
Copy link
Contributor

@arturovt arturovt commented May 22, 2025

Prior to this commit, calling ChangeDetectorRef.markForCheck() could schedule a redundant change detection in zoneless mode. This commit ensures listeners are cleaned up once the view is destroyed, preventing unnecessary change detection cycles when placeholders load or fail.

@pullapprove pullapprove bot requested a review from alxhub May 22, 2025 17:17
@angular-robot angular-robot bot added the area: common Issues related to APIs in the @angular/common package label May 22, 2025
@ngbot ngbot bot added this to the Backlog milestone May 22, 2025
@AndrewKushnir AndrewKushnir requested review from AndrewKushnir and removed request for alxhub June 5, 2025 17:58
@AndrewKushnir AndrewKushnir added target: patch This PR is targeted for the next patch release action: review The PR is still awaiting reviews from at least one requested reviewer labels Jun 5, 2025
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

Prior to this commit, attempting to resolve a ChangeDetectorRef after views or app have been destroyed would result in an error.

@arturovt could you please provide a bit more info on this - when does it happen? I'm also wondering it it might be possible to have a test to capture this behavior?

@arturovt
Copy link
Contributor Author

arturovt commented Jun 5, 2025

The most basic showcase:

// server.ts
app.get('/cat-blurred.jpg', async (req, res) => {
  await new Promise((resolve) => setTimeout(resolve, 4000));
  res.sendFile('/cat-blurred-file.jpg');
});
<img ngSrc="cat.jpg" width="400" height="200" placeholder="cat-blurred.jpg" />
<button (click)="appRef.destroy()">Destroy</button>

@arturovt arturovt force-pushed the fix/common-remove-placeholder-listeners branch from d192401 to 5240906 Compare June 18, 2025 23:41
@arturovt
Copy link
Contributor Author

@AndrewKushnir added unit test and removal of event listeners in other places too (see comments).

Prior to this commit, attempting to resolve a `ChangeDetectorRef` after views or app have been destroyed would result in an error. In this commit, we clean up listeners once the view is destroyed, before the placeholder loads or fails to load.
@arturovt arturovt force-pushed the fix/common-remove-placeholder-listeners branch from 5240906 to c4b654c Compare August 25, 2025 22:02
@arturovt
Copy link
Contributor Author

@AndrewKushnir any more changes required for this change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action: review The PR is still awaiting reviews from at least one requested reviewer area: common Issues related to APIs in the @angular/common package target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants