Skip to content
This repository has been archived by the owner on Oct 7, 2024. It is now read-only.

Update utils package and rename dispose fn #234

Merged
merged 2 commits into from
Jul 4, 2023

Conversation

mikesposito
Copy link
Member

@mikesposito mikesposito commented Jun 21, 2023

Description

This PR pulls the latest version of @metamask/utils, which also includes an updated Keyring type with an optional destroy method: this allows us to remove some type narrowing and the ambiguity between dispose and destroy methods.

Changes

  • CHANGED: dispose optional method is not called anymore when destroying a keyring

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

@mikesposito mikesposito requested a review from a team as a code owner June 21, 2023 07:55
@socket-security
Copy link

socket-security bot commented Jun 21, 2023

Updated dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
@metamask/utils 5.0.2...6.1.0 None +0/-0 724 kB metamaskbot

mcmire
mcmire previously approved these changes Jun 21, 2023
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

I think we need to bump the minimum Node.js version first, this package still supports v14

package.json Outdated
@@ -81,7 +81,7 @@
},
"packageManager": "[email protected]",
"engines": {
"node": ">=14.0.0"
"node": ">=16.0.0"
Copy link
Member

Choose a reason for hiding this comment

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

Thoughts on moving this to a separate PR? It would be easier to keep track of that way

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I think it would be better to separate the scopes into two PRs

I created 236, that we could merge before this PR

@mikesposito mikesposito force-pushed the refactor/update-utils-and-rename-dispose branch from 4e4bbd5 to c02b085 Compare July 4, 2023 12:11
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

@mikesposito mikesposito merged commit dc21a99 into main Jul 4, 2023
@mikesposito mikesposito deleted the refactor/update-utils-and-rename-dispose branch July 4, 2023 12:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants