-
Notifications
You must be signed in to change notification settings - Fork 5k
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
refactor: Typescript conversion of watch-asset.js #23776
base: main
Are you sure you want to change the base?
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. |
} & HandlerWrapper; | ||
|
||
const watchAsset = { | ||
methodNames: [MESSAGE_TYPE.WATCH_ASSET], |
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.
methodNames: [MESSAGE_TYPE.WATCH_ASSET], | |
methodNames: [MESSAGE_TYPE.WATCH_ASSET, MESSAGE_TYPE.WATCH_ASSET_LEGACY], |
_next: JsonRpcEngineNextCallback, | ||
end: JsonRpcEngineEndCallback, | ||
{ handleWatchAssetRequest }: WatchAssetOptions, | ||
) { |
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.
Can we add a return type here?
BTW we can directly merge to develop :)
|
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. |
a6936af
to
35c24dc
Compare
35c24dc
to
fefe62f
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.
PR Summary
Converted watch-asset.js
to TypeScript, enhancing type safety and maintainability.
- New Type Definitions: Added
app/scripts/lib/rpc-method-middleware/handlers/types.ts
to define TypeScript types for thewatch-asset
handler. - File Removal: Deleted
app/scripts/lib/rpc-method-middleware/handlers/watch-asset.js
as part of the TypeScript conversion. - TypeScript Conversion: Created
app/scripts/lib/rpc-method-middleware/handlers/watch-asset.ts
to replace the JavaScript file, ensuring type safety and maintaining the same functionality.
3 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
fefe62f
to
2ec2750
Compare
Quality Gate passedIssues Measures |
Builds ready [2ec2750]
Page Load Metrics (73 ± 22 ms)
Bundle size diffs
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #23776 +/- ##
========================================
Coverage 69.98% 69.98%
========================================
Files 1422 1422
Lines 49926 49926
Branches 13861 13861
========================================
Hits 34940 34940
Misses 14986 14986 ☔ View full report in Codecov by Sentry. |
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. |
Part of #23014
Fixes #23474
Converting the level 6 dependency file
app/scripts/lib/rpc-method-middleware/handlers/watch-asset.js
to typescript for contributing tometamask-controller.js
.Description
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist