refactor: Typescript conversion of eth-accounts.js #23504
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
metamaskbot
added
INVALID-PR-TEMPLATE
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #23504 +/- ##
========================================
Coverage 70.14% 70.14%
========================================
Files 1424 1424
Lines 49572 49572
Branches 13868 13868
========================================
Hits 34769 34769
Misses 14803 14803 ☔ View full report in Codecov by Sentry. |
davidmurdoch
changed the title
There was a problem hiding this comment.
It looks like extension is using a version (v6.0.1) of the sunsetted json-rpc-engine package that was released 3 years ago. The current version (v8.0.1) of this package has been migrated into core and renamed @metamask/json-rpc-engine. There are major breaking changes between the two versions and important updates.
Packages used in extension are already using the latest version of @metamask/json-rpc-engine internally, which means incompatibilities might pop up if we use the older version.
Maybe we should considered this PR blocked until we can get an upgrade to @metamask/json-rpc-engine merged? This is a new issue arising from this TypeScript conversion and directly affects how the types in this file should be defined.
| async function ethAccountsHandler( | ||
| _req: JsonRpcRequest<unknown>, | ||
| res: PendingJsonRpcResponse<unknown>, |
There was a problem hiding this comment.
Same as above.
| async function ethAccountsHandler( | |
| _req: JsonRpcRequest<unknown>, | |
| res: PendingJsonRpcResponse<unknown>, | |
| async function ethAccountsHandler< | |
| Params extends JsonRpcParams = JsonRpcParams, | |
| Result extends Json = Json | |
| >( | |
| _req: JsonRpcRequest<Params>, | |
| res: PendingJsonRpcResponse<Result>, |
|
@MajorLift i'm working on the upgrade of the json-rpc-engine dependency unless you already are. |
metamaskbot
added
in-progress
and removed
needs-dev-review
NiranjanaBinoy
force-pushed
the
ts-eth-accounts
branch
from
a6d3e31
to
6a1f7f6
Compare
|
This PR has been automatically marked as stale because it has not had recent activity in the last 60 days. It will be closed in 14 days. Thank you for your contributions. |
|
This PR was closed because there has been no follow up activity in the last 14 days. Thank you for your contributions. |
Builds ready [d5a01f4]
Page Load Metrics (50 ± 4 ms)
Bundle size diffs
|
e with @metamask/json-rpc-engine
NiranjanaBinoy
force-pushed
the
ts-eth-accounts
branch
from
15aa057
to
54ab602
Compare
|
Builds ready [54ab602]
Page Load Metrics (1802 ± 86 ms)
Bundle size diffs
|
| @@ -65,6 +65,8 @@ export const MESSAGE_TYPE = { | |||
| ///: END:ONLY_INCLUDE_IF | |||
| } as const; | |||
|
|
|||
| export type MessageType = (typeof MESSAGE_TYPE)[keyof typeof MESSAGE_TYPE]; | |||
There was a problem hiding this comment.
Weird, MessageType is resolving to string for me. Investigating...
There was a problem hiding this comment.
OK IsEqual is evaluating to true, so I'm going to conclude that this is language server flakiness.
type IsEqual = [MessageType, AllMessageTypes] extends [AllMessageTypes, MessageType]
? true
: false;
type AllMessageTypes =
| 'wallet_addEthereumChain'
| 'eth_accounts'
| 'eth_decrypt'
| 'eth_chainId'
| 'eth_getEncryptionPublicKey'
| 'eth_getBlockByNumber'
| 'eth_requestAccounts'
| 'eth_sendTransaction'
| 'eth_sendRawTransaction'
| 'eth_signTransaction'
| 'eth_signTypedData'
| 'eth_signTypedData_v1'
| 'eth_signTypedData_v3'
| 'eth_signTypedData_v4'
| 'metamask_getProviderState'
| 'metamask_logWeb3ShimUsage'
| 'personal_sign'
| 'metamask_sendDomainMetadata'
| 'wallet_switchEthereumChain'
| 'transaction'
| 'wallet_requestPermissions'
| 'wallet_watchAsset'
| 'metamask_watchAsset'
| typeof DIALOG_APPROVAL_TYPES.alert
| typeof DIALOG_APPROVAL_TYPES.confirmation
| typeof DIALOG_APPROVAL_TYPES.prompt
| typeof DIALOG_APPROVAL_TYPES.default
| 'metamaskinstitutional_authenticate'
| 'metamaskinstitutional_reauthenticate'
| 'metamaskinstitutional_refresh_token'
| 'metamaskinstitutional_supported'
| 'metamaskinstitutional_portfolio'
| 'metamaskinstitutional_open_swaps'
| 'metamaskinstitutional_checkIfTokenIsPresent'
| 'metamaskinstitutional_setAccountAndNetwork'
| 'metamaskinstitutional_openAddHardwareWallet';
metamaskbot
added
release-12.6.0
Description
Part of #23014
Fixes #23468
Converting the level 6 dependency file
app/scripts/lib/rpc-method-middleware/handlers/eth-accounts.jsto typescript for contributing tometamask-controller.js.