Skip to content

fix(zone.js): do not mutate event listener options (may be readonly)#55796

Closed
arturovt wants to merge 1 commit intoangular:mainfrom
arturovt:fix/54142-zone-ac
Closed

fix(zone.js): do not mutate event listener options (may be readonly)#55796
arturovt wants to merge 1 commit intoangular:mainfrom
arturovt:fix/54142-zone-ac

Conversation

@arturovt
Copy link
Copy Markdown
Contributor

@arturovt arturovt commented May 14, 2024

Prior to this commit, event listener options were mutated directly, for example,
options.signal = undefined or options.once = false.

This issue arises in apps using third-party libraries where the responsibility lies
with the library provider. Some libraries, like WalletConnect, pass an abort controller
as addEventListener options. Because the abort controller has the signal property,
this is a valid case. Thus, mutating options would throw an error since signal
is a readonly property.

Even though zone.js is being deprecated as Angular moves towards zoneless change detection,
this fix is necessary for apps that still use zone.js and cannot migrate to zoneless change
detection because they rely on third-party libraries and are not responsible for the code
used in them.

Closes #54142

@arturovt arturovt force-pushed the fix/54142-zone-ac branch from 5dbe16b to 29cfe68 Compare May 14, 2024 19:47
@arturovt arturovt marked this pull request as ready for review May 14, 2024 20:12
@pullapprove pullapprove Bot requested a review from JiaLiPassion May 14, 2024 20:12
@arturovt arturovt force-pushed the fix/54142-zone-ac branch 3 times, most recently from 651aea4 to 2b8f010 Compare May 15, 2024 16:27
@AndrewKushnir AndrewKushnir added action: review The PR is still awaiting reviews from at least one requested reviewer area: zones Issues related to zone.js requires: TGP This PR requires a passing TGP before merging is allowed labels May 17, 2024
@ngbot ngbot Bot modified the milestone: Backlog May 17, 2024
@AndrewKushnir AndrewKushnir added the target: patch This PR is targeted for the next patch release label May 17, 2024
@pullapprove pullapprove Bot added target: patch This PR is targeted for the next patch release and removed target: patch This PR is targeted for the next patch release requires: TGP This PR requires a passing TGP before merging is allowed labels May 17, 2024
Copy link
Copy Markdown
Contributor

@JiaLiPassion JiaLiPassion left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks.

@AndrewKushnir AndrewKushnir added requires: TGP This PR requires a passing TGP before merging is allowed and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels May 18, 2024
@pullapprove pullapprove Bot removed requires: TGP This PR requires a passing TGP before merging is allowed labels May 18, 2024
@AndrewKushnir AndrewKushnir added the requires: TGP This PR requires a passing TGP before merging is allowed label May 19, 2024
@pullapprove pullapprove Bot removed the requires: TGP This PR requires a passing TGP before merging is allowed label May 19, 2024
@AndrewKushnir AndrewKushnir added the action: global presubmit The PR is in need of a google3 global presubmit label May 19, 2024
@AndrewKushnir
Copy link
Copy Markdown
Contributor

Global Presubmit.

Comment thread packages/zone.js/lib/common/events.ts Outdated
Prior to this commit, event listener options were mutated directly, for example,
`options.signal = undefined` or `options.once = false`.

This issue arises in apps using third-party libraries where the responsibility lies
with the library provider. Some libraries, like WalletConnect, pass an abort controller
as `addEventListener` options. Because the abort controller has the `signal` property,
this is a valid case. Thus, mutating options would throw an error since `signal`
is a readonly property.

Even though zone.js is being deprecated as Angular moves towards zoneless change detection,
this fix is necessary for apps that still use zone.js and cannot migrate to zoneless change
detection because they rely on third-party libraries and are not responsible for the code
used in them.

Closes angular#54142
@arturovt arturovt force-pushed the fix/54142-zone-ac branch from 2b8f010 to 6679674 Compare May 19, 2024 06:49
@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: global presubmit The PR is in need of a google3 global presubmit labels May 20, 2024
@AndrewKushnir
Copy link
Copy Markdown
Contributor

Caretaker note: TGP is "green" (except for unrelated build breakages), this PR is ready for merge.

@dylhunn
Copy link
Copy Markdown
Contributor

dylhunn commented May 22, 2024

This PR was merged into the repository by commit 85c1719.

dylhunn pushed a commit that referenced this pull request May 22, 2024
…55796)

Prior to this commit, event listener options were mutated directly, for example,
`options.signal = undefined` or `options.once = false`.

This issue arises in apps using third-party libraries where the responsibility lies
with the library provider. Some libraries, like WalletConnect, pass an abort controller
as `addEventListener` options. Because the abort controller has the `signal` property,
this is a valid case. Thus, mutating options would throw an error since `signal`
is a readonly property.

Even though zone.js is being deprecated as Angular moves towards zoneless change detection,
this fix is necessary for apps that still use zone.js and cannot migrate to zoneless change
detection because they rely on third-party libraries and are not responsible for the code
used in them.

Closes #54142

PR Close #55796
@dylhunn dylhunn closed this in 85c1719 May 22, 2024
@arturovt arturovt deleted the fix/54142-zone-ac branch May 23, 2024 05:40
@angular-automatic-lock-bot
Copy link
Copy Markdown

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 Jun 23, 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: zones Issues related to zone.js 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.

AbortController error with walletconnect lib after zone.js 0.14.3 update

4 participants