-
Notifications
You must be signed in to change notification settings - Fork 644
refactor: Use hash-private in BaseSnapExecutor
#3830
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
Conversation
| try { | ||
| const result = await (this.methods as any)[method](...paramsAsArray); | ||
| const result = await this.#methods[method as keyof Methods]( | ||
| ...(paramsAsArray as any), |
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.
This typing is still not great. The ideal case is to make a breaking change to always pass parameters as objects and use structs to validate them better
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3830 +/- ##
==========================================
- Coverage 98.40% 98.39% -0.01%
==========================================
Files 430 430
Lines 12448 12457 +9
Branches 1933 1932 -1
==========================================
+ Hits 12249 12257 +8
- Misses 199 200 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This PR refactors
BaseSnapExecutorto follow our more recent patterns, notably using hash private for most properties and functions, but also changing a couple of naming patterns and improving types. This makesBaseSnapExecutorway more compliant with our linter.Split off #3803 to make this easier to review.
Note
Medium Risk
Touches core snap execution/termination and teardown tracking; while mostly a refactor, it changes how termination promises and teardown state are managed and could affect edge-case lifecycle behavior.
Overview
Refactors
BaseSnapExecutorto use hash-private fields/methods throughout (removing prior lint suppressions) and tightens command dispatch typing by calling#methods[method as keyof Methods]instead ofany.Updates execution/termination plumbing: snap evaluation cancellation now uses
createDeferredPromiseand a shared#teardownRefobject is passed intowithTeardowninstead ofthis, changing the termination error string toThe Snap "..." has been terminated during execution.(and updating the corresponding browser test expectation).Written by Cursor Bugbot for commit 0082a8c. This will update automatically on new commits. Configure here.