-
Notifications
You must be signed in to change notification settings - Fork 10
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
fix: collisions with other wallet providers #162
fix: collisions with other wallet providers #162
Conversation
4b5088f
to
f45fa3a
Compare
Getting some peculiar errors in this pr that I'm not seeing on master. When running No tests are run as it seems to be waiting on the server to start. I can confirm that the server is running by fetching a file from that directory: @hugomrdias Hoping you can help me out a bit here, I'm not understanding what/where is checking |
Digging in a bit more on this webserver failure, after updating packages in hugomrdias/metamask#32 and updating some packages locally to get rid of the peer dependency issue, I'm seeing an actual error message:
There is something bizarre happening when importing this line in import { getBIP44AddressKeyDeriver } from '@metamask/key-tree' I've verified locally that these files do exist in @hugomrdias I did notice that you had run into previous issues with this library's imports as referenced here MetaMask/key-tree#144, but it looks like this issue was resolved in |
yeah its a metamask problem they have no extensions in the import and also are importing json files without attribute |
its too deep i tried fixing this on our side, but can't find a quick fix |
@hugomrdias I see, we are using the newer style imports with ts and esm node. We should be able to just use the same module rules they are using in our tsconfig
and switch away from using esm at the package.json level. That way we still get to use import statements in all of the actual codebase, but use requires in the rollup.js config, and we shouldnt have any issues with how dependencies are doing their packaging/importing. |
33d3803
to
cec6eac
Compare
@hugomrdias Scratch that last comment. These test failures are happening on the master branch, and out of scope of this pr. Those test should be fixed elsewhere. I cleaned up the commit history to reflect the actual changes here. |
on main ? https://github.com/filecoin-project/filsnap/actions/workflows/manual.yml its passing |
metamask broke its own rollup plugin (kinda it now support proper esm but key-tree is not package right for esm) , and the cli serve cmd also doesnt work anymore it defaults to a full webpack build and serve, and they updated most snap packages to a proper esm packaging but forget key-tree.... |
check this PR #163 |
#163 accomplishes the updates and fixes weirdness around metamask packaging. Will rebase and fix up any remaining issues here once that lands :) |
- fixes filecoin-project#160 - check for other existing providers which may cause conflict
81055b3
to
11d8cd2
Compare
11d8cd2
to
46cef2f
Compare
70a0411
to
4020b09
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Type of change