Skip to content

Conversation

@arturovt
Copy link
Contributor

This commit removes event listeners from the script element once loading is
complete. If the element is not garbage collected properly, in Firefox, the script
element still appears in the memory tree view, captured by
__zone_symbol__loadfalse -> HTMLScriptElement -> GC Roots. We should always be good
citizens and clean up event listeners when we no longer need them, as browser's garbage
collectors work differently. Calling remove() on the node doesn't guarantee that the
node can be collected.

This commit removes event listeners from the `script` element once loading is
complete. If the element is not garbage collected properly, in Firefox, the script
element still appears in the memory tree view, captured by
`__zone_symbol__loadfalse -> HTMLScriptElement -> GC Roots`. We should always be good
citizens and clean up event listeners when we no longer need them, as browser's garbage
collectors work differently. Calling `remove()` on the node doesn't guarantee that the
node can be collected.
@arturovt arturovt marked this pull request as ready for review September 19, 2024 13:15
@pkozlowski-opensource pkozlowski-opensource added the area: common/http Issues related to HTTP and HTTP Client label Sep 19, 2024
@ngbot ngbot bot added this to the Backlog milestone Sep 19, 2024
Copy link
Member

@JoostK JoostK left a comment

Choose a reason for hiding this comment

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

I don't think I'm a code owner, but LGTM.

@JoostK JoostK added target: patch This PR is targeted for the next patch release action: presubmit The PR is in need of a google3 presubmit labels Sep 23, 2024
@AndrewKushnir
Copy link
Contributor

Presubmit.

@AndrewKushnir AndrewKushnir 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 and removed action: presubmit The PR is in need of a google3 presubmit labels Sep 30, 2024
@AndrewKushnir AndrewKushnir removed the request for review from thePunderWoman September 30, 2024 00:11
@AndrewKushnir
Copy link
Contributor

Caretaker note: presubmit is "green", this PR is ready for merge.

@devversion
Copy link
Member

This PR was merged into the repository by commit 22dafa6.

The changes were merged into the following branches: main, 18.2.x

devversion pushed a commit that referenced this pull request Oct 1, 2024
)

This commit removes event listeners from the `script` element once loading is
complete. If the element is not garbage collected properly, in Firefox, the script
element still appears in the memory tree view, captured by
`__zone_symbol__loadfalse -> HTMLScriptElement -> GC Roots`. We should always be good
citizens and clean up event listeners when we no longer need them, as browser's garbage
collectors work differently. Calling `remove()` on the node doesn't guarantee that the
node can be collected.

PR Close #57877
@devversion devversion closed this in 22dafa6 Oct 1, 2024
@arturovt arturovt deleted the fix/http-jsonp-listeners branch October 1, 2024 08:27
@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 Nov 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: common/http Issues related to HTTP and HTTP Client merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants