search

airgap-it / beacon-sdk

The beacon sdk allows developers of dApps and wallets on Tezos to implement the wallet interaction standard tzip-10.
https://walletbeacon.io
MIT License
101 stars 65 forks source link

Lifetime management of the BeaconDapp class #799

Closed ac10n closed 1 month ago

ac10n commented 3 months ago

Describe the bug (current behavior)

In Taquito unit tests, Jest is detecting open handles.

To Reproduce

  1. Clone Taquito git@github.com:ecadlabs/taquito.git
  2. Check out the branch beacon-4.3.0-beta.0
  3. npm ci
  4. npm run build
  5. npm run test -w packages/taquito-beacon-wallet
  6. Jest will complain about open handles.

Expected (correct) behavior Jest should exit cleanly

Screenshots and/or logs image

Environment

Additional context We will be creating a PR that shows our explorations to fix the problem. It is not a clean code contribution, just a POC.

IsaccoSordo commented 3 months ago

Hey @ac10n,

Here's what we've found so far:

In Beacon, we don't close the channel, allowing tabs to join and leave without worrying about whether the channel is open. However, we could call channel.close() when running destroy since the client instance will stop working afterward.

We tried this approach and encountered the error you linked in your PR. Then, we manually edited broadcast-channel.js from node_modules as you suggested. While this resolves the error, a new one appears: Jest has detected the following 2 open handles potentially keeping Jest from exiting: […].

At this point, I am unsure if all these issues are caused by Beacon. I wanted to open an issue with the BroadcastChannel's developers here, but it is not possible.

We are still investigating potential solutions. In the meantime, is this issue only affecting your tests, or are you also experiencing problems with Taquito?

dsawali commented 3 months ago

Hey @IsaccoSordo,

I have not discovered anything on the test dapp. The only place where I saw an issue was with our unit test behaviour. We are currently putting a workaround to make sure the tests resolve and exit properly. While it not being a hard blocker, it's definitely not ideal. It could also have potential issues in the future that we aren't fully aware of yet.

You might be able to put issues in RxDB? I agree it seems strange that a public repo does not have issue tracking.

cc @ac10n

IsaccoSordo commented 3 months ago

Hey @ac10n @dsawali, I have opened https://github.com/ecadlabs/taquito/pull/3015 to solve this issue. Tell me what you think

IsaccoSordo commented 1 month ago

@ac10n I'm closing this issue since this pull request has been merged. Feel free to open another issue if anything else comes up.