fix(wallet): Fix builds and most lint issues#8420
fix(wallet): Fix builds and most lint issues#8420rekmarks wants to merge 7 commits intofeat/wallet-libraryfrom
Conversation
- Add dotenv to load INFURA_PROJECT_KEY from .env in tests - Add .env.example with placeholder key - Add missing tsconfig references for all repo-local imports - Add Wallet.destroy() to clean up controller instances - Use fake timers and proper afterEach cleanup to prevent open handles Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use method syntax in InitializationConfiguration for bivariant parameter checking, allowing heterogeneous configs in a single array - Change InstanceState fallback from null to unknown for type compatibility - Widen TransactionController messenger type to include init-time actions - Add missing RemoteFeatureFlagController constructor args via WalletOptions - Use type guards and specific casts instead of any where possible - Default RootMessenger type params to any for unconstrained call() usage Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replaces inline arrow function wrappers with bindMessengerAction, a typed helper around messenger.call.bind that restores per-action type inference lost by Function.prototype.bind. Also incorporates upstream refinements to initialization types and Wallet state accessor. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
| }); | ||
|
|
||
| afterEach(async () => { | ||
| await wallet?.destroy(); |
There was a problem hiding this comment.
Nit: Any concern with just doing wallet.destroy() in each test case for now?
There was a problem hiding this comment.
As opposed to in afterEach? Why?
There was a problem hiding this comment.
I generally find that more readable but it also provides flexibility in the way that we can pass different arguments to setupWallet in each test case.
There was a problem hiding this comment.
I prefer keeping cleanup in the hooks because customizing it between different test cases has a tendency to cause pernicious bugs in the test suite, IME.
| clientVersion: '1.0.0', | ||
| showApprovalRequest: () => undefined, | ||
| showApprovalRequest: (): undefined => undefined, | ||
| clientConfigApiService: new ClientConfigApiService({ |
There was a problem hiding this comment.
I wonder if we need to pass this entire thing in? Maybe just add client, distribution, environment to WalletOptions and we can construct ClientConfigApiService based on that?
There was a problem hiding this comment.
I think merging the constructor params of ClientConfigApiService into the Wallet constructor params would be more trouble than it's worth. If we ultimately decide that we don't care about all of the options of the former, we can always fold it in later.
| // Method syntax provides bivariant parameter checking, which is needed to | ||
| // collect heterogeneous InitializationConfiguration values in a single array. |
There was a problem hiding this comment.
I don't understand any of this
There was a problem hiding this comment.
It's basically saying that init is defined as a method as opposed to function property because the types will break otherwise. I edited the comment and moved it to init: e91bbe2
## Explanation Infer messenger, instance and state types from `defaultConfigurations`. Which lets us remove casting due to previous lack of messenger types. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Mostly TypeScript typing refactors, but it also downgrades `@metamask/utils`, which could introduce subtle compatibility or build/runtime differences across the wallet package. > > **Overview** > Improves type-safety for the wallet initialization flow by deriving `RootMessenger`, default action/event types, instance types, and `Wallet.state` shape directly from `defaultConfigurations` (new `initialization/defaults.ts`) and re-exporting them via `initialization/index.ts`. > > Updates `Wallet` and `initialize` to use these inferred types (including stricter `messenger` typing and typed `DefaultState`/`DefaultInstances`), removes the old `RootMessenger` type from `types.ts`, and adds a couple of targeted `@ts-expect-error` annotations for calling optional `destroy()`. > > Also adjusts `sendTransaction` to rely on the typed return value from `messenger.call` (removing an explicit cast) and downgrades `@metamask/utils` from `^11.11.0` to `^11.9.0` (with lockfile update). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit e130704. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
2 participants
Builds and tests pass. All lint issues are fixed except a handful that are deferred pending future changes.
Note
Medium Risk
Moderate risk due to new controller dependencies and changes to initialization wiring/typing that could affect runtime messaging and controller lifecycle. Test behavior now depends on external
INFURA_PROJECT_KEYand mocked timers, which may introduce CI/environment sensitivity.Overview
Stabilizes wallet builds/tests by wiring in missing controllers and config. The wallet package now depends on additional controllers (accounts/approval/connectivity/network/remote feature flags/transaction) and updates TS project references accordingly.
Improves runtime/test ergonomics. Jest loads a local
.env(with.env.exampleadded and.envgitignored),Walletexposes stronger typedmessenger/stateand addsdestroy()to clean up controller instances; tests are updated to requireINFURA_PROJECT_KEY, use fake timers, and properly teardown the wallet.Tightens initialization typing and controller wiring. Adds
initialization/defaults.tsfor inferredDefaultInstances/DefaultActions/DefaultEvents, introducesbindMessengerActionto preserve action typings, and updates controller initializers (notablyTransactionControllerandRemoteFeatureFlagController) to pass required options and bind messenger actions safely.Reviewed by Cursor Bugbot for commit a652933. Bugbot is set up for automated code reviews on this repo. Configure here.