anza-xyz / wallet-adapter

Modular TypeScript wallet adapters and components for Solana applications.
https://anza-xyz.github.io/wallet-adapter/
Apache License 2.0
1.55k stars 944 forks source link

Trezor wallet dependency issue causing error in `wallet-adapter-wallets` #918

Closed mcintyre94 closed 7 months ago

mcintyre94 commented 7 months ago

Describe the bug

When importing from wallet-adapter-wallets there's an error caused by a bad dependency in wallet-adapter-trezor:

 ⨯ ./node_modules/@trezor/connect-web/lib/channels/serviceworker-window.js:4:19
Module not found: Can't resolve '@trezor/connect-common/src/messageChannel/abstract'

https://nextjs.org/docs/messages/module-not-found

Import trace for requested module:
./node_modules/@trezor/connect-web/lib/popup/index.js
./node_modules/@trezor/connect-web/lib/index.js
./node_modules/@solana/wallet-adapter-trezor/lib/esm/adapter.js
./node_modules/@solana/wallet-adapter-trezor/lib/esm/index.js
./node_modules/@solana/wallet-adapter-wallets/lib/esm/index.js
./app/layout.tsx

Note that @trezor/connect-web and @trezor/connect-common have had recent updates, and an app installing wallet-adapter-wallets has the latest version of each (9.2.0 and 0.0.29 respectively)

To Reproduce Steps to reproduce the behavior:

Expected behavior

No error, Next app starts correctly

@gabrielKerekes tagging because you opened the previous Trezor PR - could you figure out how the adapter dependencies need to be set up please?

gabrielKerekes commented 7 months ago

I opened an issue in Trezor Connect repo - https://github.com/trezor/trezor-suite/issues/11442. Unfortunately I'm not able how to fix this myself as it seems to be an issue with Trezor Connect not the integration into wallet adapter.

As a temporary solution for wallet-adapter we could pin @trezor/connect-web to version 9.1.6? What do you think?

mcintyre94 commented 7 months ago

As a temporary solution for wallet-adapter we could pin @trezor/connect-web to version 9.1.6? What do you think?

Thanks for looking into this :) That sounds like a good workaround for now! Would get the Trevor adapter working for those who need it and fix the umbrella package for everyone. Would you be able to open a PR on the adapter for that?

gabrielKerekes commented 7 months ago

Would you be able to open a PR on the adapter for that?

https://github.com/anza-xyz/wallet-adapter/pull/920

yakphi commented 7 months ago

I opened an issue in Trezor Connect repo - trezor/trezor-suite#11442. Unfortunately I'm not able how to fix this myself as it seems to be an issue with Trezor Connect not the integration into wallet adapter.

As a temporary solution for wallet-adapter we could pin @trezor/connect-web to version 9.1.6? What do you think?

This solve the build issue util the fix is released

For yarn adding the resolutions rule to package.json.

"resolutions": { "@trezor/connect-web": "9.1.6" }

gabrielKerekes commented 7 months ago

A fixed @trezor/connect-web has been released yesterday. Available on NPM. I tested the reproduction repo (here) and it now builds as expected 👍 I tested end to end with the nextjs-starter in this repo and that worked as well.

mcintyre94 commented 7 months ago

Hmm I'm still seeing the error on the stackblitz repro, but overriding to 9.2.1 works correctly for me.

gabrielKerekes commented 7 months ago

Hmm I'm still seeing the error on the stackblitz repro, but overriding to 9.2.1 works correctly for me.

The override shouldn't be needed. Maybe the issue is that version 9.2.0 was written in package-lock.json? Or some similar problem?

mcintyre94 commented 7 months ago

Looks like something was cached, all good! Looks like this should be sorted for good :) Thanks again Gabriel!