refactor: convert shared/lib/transactions-controller-utils to TypeScript (level 29) - #41333
Conversation
Agent-Logs-Url: https://github.com/MetaMask/metamask-extension/sessions/bda8384d-d827-4f0e-a207-e6abddb13981 Co-authored-by: DDDDDanica <12678455+DDDDDanica@users.noreply.github.com>
Builds ready [8fcd6f5]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs
|
|
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. |
Builds ready [5e8ac6f]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs
|
Builds ready [83a3186]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs
|
…actions-controller-utils-to-typescrip
Builds ready [2c01f08]
⚡ Performance Benchmarks (Total: 🟢 18 pass · 🟡 0 warn · 🔴 0 fail)
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs
|
|
Builds ready [170fb34]
⚡ Performance Benchmarks (Total: 🟢 16 pass · 🟡 2 warn · 🔴 0 fail)
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚀 Bundle size reduced!]
|
🧪 Validation RunVerdict: Note Trial run of the MetaMask evidence skills — The conversion adds Substitution alone: Authored type vs authoritative source ·
|
| Arm | Change | distinct tsc errors |
|---|---|---|
| A — baseline | none | 0 |
| B — substituted | shared/lib/transactions-controller-utils.ts:146 |
0 |
| new under substitution | 0 |
Silent — this is not proof of agreement. Existing call sites may satisfy both
shapes; indexing and .match() compile against string and string[] alike.
Re-run with --probe to inject a sink only the authoritative type accepts.
Produced by tsc-substitution.sh; source restored after the run. head 7e59505f5fbbb7e75daf0db18f56194b436b6d07 · 0 tracked changes · Version 5.6.3. Run: https://github.com/MajorLift/metamask-skills/actions/runs/30766103170 — logs and artifacts attached there.
The same substitution with a sink typed to accept only the array shape:
Authored type vs authoritative source · divergence surfaced
| Arm | Change | distinct tsc errors |
|---|---|---|
| A — baseline | none | 0 |
| B — substituted | shared/lib/transactions-controller-utils.ts:146 + typed sink |
1 |
| new under substitution | 1 |
Errors present in B and absent in A — what the local type was concealing:
shared/lib/transactions-controller-utils.ts(150,11): error TS2322
Produced by tsc-substitution.sh; source restored after the run. head 7e59505f5fbbb7e75daf0db18f56194b436b6d07 · 0 tracked changes · Version 5.6.3. Run: https://github.com/MajorLift/metamask-skills/actions/runs/30766466349 — logs and artifacts attached there.
Follows from the arms above
- Replacing
string[]withstringproduces zero new errors on its own. The obvious check reports a false clean. - Adding
const _probe: string[] | undefined = …topicssurfaces oneTS2322at line 150 — not assignable. The types do diverge; nothing in the existing call sites distinguishes them. - The reason is visible in the call sites:
topics[0]andtopics[2].match(…)compile againststringandstring[]alike — indexing a string yields a character, and.matchexists on both.
Open for review: this shows the local type and the upstream one are not interchangeable, not which is correct. If upstream's string is the error — as the comment asserts and the Ethereum spec suggests — the durable fix is upstream in @metamask/transaction-controller, and the local type is a correct workaround that should carry a link to that issue rather than a prose note. If upstream is right for some log shapes, indexing a string here would silently yield characters.



[skip-e2e]
Converts
shared/lib/transactions-controller-utils.jsand its test file to TypeScript, removing both fromfiles-to-convert.json.Changes
transactions-controller-utils.ts— typed all exports:calcGasTotal:string | numberparams (was JS untyped, callers pass numbers)toPrecisionWithoutTrailingZeros:string | number | BigNumber→stringcalcTokenAmount:string | number | BigNumber→BigNumbergetSwapsTokensReceivedFromTxMeta: all params optional to match existing JS call-site patterns;chainId: string | number;precision: number | null = 6txReceiptto{ type?: string }to access MetaMask's internal envelope-type field (absent from@metamask/transaction-controller'sTransactionReceipt)Log[]to a localLogWithTopicsArray[]type sinceLog.topicsis incorrectly typed asstringinstead ofstring[]in the upstream packagetransactions-controller-utils.test.ts(renamed fromtransaction-controller-utils.test.js):as unknown as TransactionMeta/* eslint-disable @typescript-eslint/naming-convention */fortoken_to_amount(external API snake_case property).toBe()call (silently ignored in JS, TypeScript surfaced it)transaction-breakdown-utils.ts: removed now-unnecessary@ts-expect-erroronprecision: nullcall (previously required because the JS version had no type for this parameter)✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
Note
Medium Risk
Mostly a type-migration, but it touches
getSwapsTokensReceivedFromTxMetalogic (default-token detection, receipt typing, and output formatting), which could subtly affect swaps amount display in the UI.Overview
Migrates
shared/lib/transactions-controller-utilsand its tests from JS to TS. Exports are now explicitly typed (including optional params forgetSwapsTokensReceivedFromTxMeta) and tests use typedTransactionMetafixtures.getSwapsTokensReceivedFromTxMetaadds stricter handling aroundtxReceipt.type,chainId(supportsnumberby converting to hex), optionalgasPriceaccess, and log topic typing, and slightly changes formatting behavior (e.g., removes an ignored extra arg in a.toBe()assertion). The transaction breakdown caller drops an unnecessary@ts-expect-errorfor passingprecision: null.Also removes the converted files from
development/ts-migration-dashboard/files-to-convert.json.Written by Cursor Bugbot for commit 170fb34. This will update automatically on new commits. Configure here.