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

Fix @metamask/eth-sig-util types #248

Merged
merged 1 commit into from
Jul 17, 2023
Merged

Fix @metamask/eth-sig-util types #248

merged 1 commit into from
Jul 17, 2023

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Jul 17, 2023

Description

The types for the package @metamask/eth-sig-util were being overwritten by a local type declaration. That has been removed, and all type errors have been resolved.

All of the type errors were related to normalize returning undefined if given a nullish input. We never pass it a nullish input, but TypeScript wasn't able to infer that, so that issue was addressed with type casts. This isn't ideal, but it's temporary. Once we finish migrating all Keyrings to the new Keyring interface, we won't need to normalize addresses anymore at all.

Changes

None

References

I was attempting to fix type errors in this package that were discovered while reviewing MetaMask/core#1441. I was unable to use additional types from @metamask/eth-sig-util due to the problem fixed by this PR.

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation for new or updated code as appropriate (note: this will usually be JSDoc)
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate

The types for the package `@metamask/eth-sig-util` were being
overwritten by a local type declaration. That has been removed, and all
type errors have been resolved.

All of the type errors were related to `normalize` returning
`undefined` if given a nullish input. We never pass it a nullish input,
but TypeScript wasn't able to infer that, so that issue was addressed
with type casts. This isn't ideal, but it's temporary. Once we finish
migrating all Keyrings to the new Keyring interface, we won't need to
normalize addresses anymore at all.
@Gudahtt Gudahtt requested a review from a team as a code owner July 17, 2023 21:39
Copy link
Member

@mikesposito mikesposito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Gudahtt Gudahtt merged commit c3afc44 into main Jul 17, 2023
16 checks passed
@Gudahtt Gudahtt deleted the fix-eth-sig-util-types branch July 17, 2023 22:35
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 this pull request may close these issues.

2 participants