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

AbortController error with walletconnect lib after zone.js 0.14.3 update #54142

Closed
imaksp opened this issue Jan 29, 2024 · 5 comments
Closed
Assignees
Labels
area: zones regression Indicates than the issue relates to something that worked in a previous version
Milestone

Comments

@imaksp
Copy link

imaksp commented Jan 29, 2024

Which @angular/* package(s) are the source of the bug?

zone.js

Is this a regression?

Yes

Description

After zone.js update to 0.14.3, walletconnect integration is breaking in our app. earlier it was opening QR code modal without any error.

Please provide a link to a minimal reproduction of the bug

No response

Please provide the exception or error you saw

core.mjs:11806 ERROR TypeError: Cannot set property signal of [object AbortController] which has only a getter
    at zone.js:1957:38
    at ae.addKeyboardEvents (index.js:1:67056)
    at ae.onOpenModalEvent (index.js:1:66362)
    at index.js:1:65722
    at index.js:1:7509
    at vanilla.mjs:242:11
    at _ZoneDelegate.invoke (zone.js:368:26)
    at Object.onInvoke (core.mjs:14778:33)
    at _ZoneDelegate.invoke (zone.js:367:52)
    at _Zone.run (zone.js:129:43)

Please provide the environment you discovered this bug in (run ng version)

Angular CLI: 17.1.1
Node: 18.18.2
Package Manager: yarn 1.22.19
OS: darwin arm64

Angular: 17.1.1
... animations, cli, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router, service-worker

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1701.1
@angular-devkit/build-angular   17.1.1
@angular-devkit/core            17.1.1
@angular-devkit/schematics      17.1.1
@schematics/angular             17.1.1
rxjs                            7.8.1
typescript                      5.3.3
zone.js                         0.14.3

Anything else?

For now solved by downgrading zone.js to 0.14.2.
This line from latest zone.js is throwing error:

taskData.options.signal = undefined;

@imaksp imaksp changed the title AbortController error with walletconnect lib with zone.js 0.14.3 AbortController error with walletconnect lib after zone.js 0.14.3 update Jan 29, 2024
@ngbot ngbot bot added this to the needsTriage milestone Jan 29, 2024
@JiaLiPassion
Copy link
Contributor

@imaksp , could you provide a reproduce repo? thanks.

@JoostK
Copy link
Member

JoostK commented Jan 29, 2024

This will happen when passing an AbortController as options for addEventListener. That's a bit odd but should work, considering that it does have the signal field (but it's readonly)

I think zone.js can prepare a separate options object instead of mutating the original object, or a wrapper using Object.create(options) or similar.

@imaksp
Copy link
Author

imaksp commented Jan 29, 2024

Hi @JiaLiPassion here is the minimum reproduction repo:
https://github.com/imaksp/wc-zone-js-issue

Steps:

  • clone, and run yarn && yarn dev
  • Click on Connect Wallet button
  • See the error in console

If you change zone.js version to 0.14.2 it will open QR modal like this without any errors.

connect-qr

@JiaLiPassion
Copy link
Contributor

@imaksp
The reason is the walletconnect lib pass the AbortController directly to the addEventListener function, and the AbortController.signal is readonly, just like @JoostK said, I will create a new Options to fix this issue.

@attilacsanyi
Copy link

Unfortunately with zone.js@0.14.4 the issue still present.

@JiaLiPassion did you found a fix without downgrading tozone.js@0.14.2? Thank you so much.

arturovt added a commit to arturovt/angular that referenced this issue May 14, 2024
Prior to this commit, event listener options were mutated directly, for example,
`options.signal = undefined`. This caused errors in libraries such as WalletConnect,
which pass an AbortController instance directly to `addEventListener` because it has
a `signal` property. The controller's `signal` property is a getter (which means it's
readonly) and cannot be mutated. Therefore, with this commit, we change the approach
from mutation to re-assignment.

Closes angular#54142
arturovt added a commit to arturovt/angular that referenced this issue 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 angular#54142
arturovt added a commit to arturovt/angular that referenced this issue May 15, 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 angular#54142
arturovt added a commit to arturovt/angular that referenced this issue May 15, 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 angular#54142
arturovt added a commit to arturovt/angular that referenced this issue May 15, 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 angular#54142
@AndrewKushnir AndrewKushnir added the regression Indicates than the issue relates to something that worked in a previous version label May 17, 2024
arturovt added a commit to arturovt/angular that referenced this issue May 19, 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 angular#54142
dylhunn pushed a commit that referenced this issue 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: zones regression Indicates than the issue relates to something that worked in a previous version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants