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

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

Closed
mcintyre94 opened this issue Mar 4, 2024 · 8 comments · Fixed by #920
Closed

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

mcintyre94 opened this issue Mar 4, 2024 · 8 comments · Fixed by #920

Comments

@mcintyre94
Copy link
Collaborator

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
Copy link
Contributor

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?

@mcintyre94
Copy link
Collaborator Author

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 added a commit to vacuumlabs/wallet-adapter that referenced this issue Mar 5, 2024
gabrielKerekes added a commit to vacuumlabs/wallet-adapter that referenced this issue Mar 5, 2024
gabrielKerekes added a commit to vacuumlabs/wallet-adapter that referenced this issue Mar 5, 2024
@gabrielKerekes
Copy link
Contributor

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

#920

@yakphi
Copy link

yakphi commented Mar 5, 2024

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
Copy link
Contributor

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
Copy link
Collaborator Author

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

@gabrielKerekes
Copy link
Contributor

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
Copy link
Collaborator Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants