sneak/AutistMask
1
0
Fork 0
Code Issues 42 Pull Requests 8 Actions Packages Projects Releases Wiki Activity

chore: remove dead exports and de-duplicate copy-pasted view helpers #168

New Issue
Open
opened 2026-08-09 03:48:42 +02:00 by clawbot · 0 comments
clawbot commented 2026-08-09 03:48:42 +02:00
Collaborator
Copy Link

Problem

Accumulated dead code and copy-paste duplication. None of it is user-visible,
but the duplication is actively dangerous in one respect: three independent
copies of the timestamp formatters mean a fix to one does not reach the others,
and the README Display Consistency policy (README.md:186-195) requires the
same data to be formatted identically everywhere.

Never-called exports:

  • src/shared/networks.js:47-49 explorerLink() — exported at :56, zero call
    sites. Every view hand-builds
    currentNetwork().explorerUrl + "/..." instead (txStatus.js:40,
    transactionDetail.js:60, helpers.js:344). Either adopt the helper
    everywhere or delete it; do not leave both patterns.
  • src/shared/prices.js:85-96 getTotalValueUsd() — exported at :104, zero
    call sites. getWalletValueUsd (prices.js:74) is used only by it, so it is
    transitively dead. Check before deleting: its intended consumer was the
    Home "Total USD value across all tokens" line at README.md:283, and
    home.js:68-72 currently shows only the active address total while
    labelling it "Total:". That is arguably a bug — the README promises a
    cross-wallet total. Decide whether to wire the function up (fixing the label
    or the behaviour) or to delete it and correct the README. State which and
    why; if it turns into a behaviour change, split it into its own issue rather
    than smuggling it into a cleanup commit.
  • src/shared/constants.js:6 ETHEREUM_SEPOLIA_CHAIN_ID — exported at :41,
    never used; the live value lives at networks.js:22. Duplicate source of
    truth.
  • src/popup/views/helpers.js:447 addressColor and :455 etherscanAddressUrl
    are exported but used only inside helpers.js. Unexport them.

Triplicated formatters:

  • isoDate() and timeAgo() exist three times with identical bodies:
    helpers.js:279-333 (the exported pair), addressDetail.js:84-138, and
    addressToken.js:41-98. Worse, addressDetail.js and addressToken.js
    shadow the imported helpers with their local copies — so the exported
    versions are silently not what those two views use.

Duplicated small helpers:

  • formatTxValue — approval.js:28-33 and transactions.js:13-18
  • blockieHtml — confirmTx.js:48-51 and transactionDetail.js:46-49
  • tokenLabel — approval.js:35-38 and txStatus.js:108-111

Implementation requirements

  • Collapse each duplicated function to a single shared definition and import it
    at every use site. For isoDate/timeAgo the canonical home is
    helpers.js; delete the shadowing local copies in addressDetail.js and
    addressToken.js.
  • Before deleting a shadowed copy, diff it against the canonical version.
    The bodies are reported as identical, but confirm it — if they have drifted,
    the behaviour change is the point and must be called out, especially for
    anything touching the timezone/UTC display setting.
  • Do not use scripted search-and-replace. Edit the files directly.
  • Unused imports are handled by the ESLint issue (#152); this issue is about
    unused definitions and exports. Coordinate so the two don't collide — if
    #152 has landed, the linter should already be green and stay green here.
  • Purely mechanical. No behaviour change except where explicitly decided for
    getTotalValueUsd, and that decision must be recorded.

Definition of done

  • Exactly one definition each of isoDate, timeAgo, formatTxValue,
    blockieHtml, and tokenLabel remains in the tree.
  • No view shadows an imported helper with a local copy.
  • explorerLink is either adopted at all three hand-built call sites or
    removed.
  • ETHEREUM_SEPOLIA_CHAIN_ID duplication is resolved to a single source of
    truth.
  • addressColor and etherscanAddressUrl are no longer exported.
  • The getTotalValueUsd decision is recorded, and if it is a behaviour
    change it has been split into its own issue.
  • Timestamps still render identically to before on AddressDetail,
    AddressToken, and TransactionDetail, including under the UTC setting.
    Confirm by inspection and say so in the PR.
  • TODO.md updated in the same commit.
  • make check passes.
## Problem Accumulated dead code and copy-paste duplication. None of it is user-visible, but the duplication is actively dangerous in one respect: three independent copies of the timestamp formatters mean a fix to one does not reach the others, and the README Display Consistency policy (`README.md:186-195`) requires the same data to be formatted identically everywhere. **Never-called exports:** - `src/shared/networks.js:47-49 explorerLink()` — exported at `:56`, zero call sites. Every view hand-builds `currentNetwork().explorerUrl + "/..."` instead (`txStatus.js:40`, `transactionDetail.js:60`, `helpers.js:344`). Either adopt the helper everywhere or delete it; do not leave both patterns. - `src/shared/prices.js:85-96 getTotalValueUsd()` — exported at `:104`, zero call sites. `getWalletValueUsd` (`prices.js:74`) is used only by it, so it is transitively dead. **Check before deleting:** its intended consumer was the Home "Total USD value across all tokens" line at `README.md:283`, and `home.js:68-72` currently shows only the *active address* total while labelling it "Total:". That is arguably a bug — the README promises a cross-wallet total. Decide whether to wire the function up (fixing the label or the behaviour) or to delete it and correct the README. State which and why; if it turns into a behaviour change, split it into its own issue rather than smuggling it into a cleanup commit. - `src/shared/constants.js:6 ETHEREUM_SEPOLIA_CHAIN_ID` — exported at `:41`, never used; the live value lives at `networks.js:22`. Duplicate source of truth. - `src/popup/views/helpers.js:447 addressColor` and `:455 etherscanAddressUrl` are exported but used only inside `helpers.js`. Unexport them. **Triplicated formatters:** - `isoDate()` and `timeAgo()` exist three times with identical bodies: `helpers.js:279-333` (the exported pair), `addressDetail.js:84-138`, and `addressToken.js:41-98`. Worse, `addressDetail.js` and `addressToken.js` **shadow the imported helpers with their local copies** — so the exported versions are silently not what those two views use. **Duplicated small helpers:** - `formatTxValue` — `approval.js:28-33` and `transactions.js:13-18` - `blockieHtml` — `confirmTx.js:48-51` and `transactionDetail.js:46-49` - `tokenLabel` — `approval.js:35-38` and `txStatus.js:108-111` ## Implementation requirements - Collapse each duplicated function to a single shared definition and import it at every use site. For `isoDate`/`timeAgo` the canonical home is `helpers.js`; delete the shadowing local copies in `addressDetail.js` and `addressToken.js`. - **Before deleting a shadowed copy, diff it against the canonical version.** The bodies are reported as identical, but confirm it — if they have drifted, the behaviour change is the point and must be called out, especially for anything touching the timezone/UTC display setting. - Do not use scripted search-and-replace. Edit the files directly. - Unused *imports* are handled by the ESLint issue (#152); this issue is about unused *definitions and exports*. Coordinate so the two don't collide — if #152 has landed, the linter should already be green and stay green here. - Purely mechanical. No behaviour change except where explicitly decided for `getTotalValueUsd`, and that decision must be recorded. ## Definition of done - [ ] Exactly one definition each of `isoDate`, `timeAgo`, `formatTxValue`, `blockieHtml`, and `tokenLabel` remains in the tree. - [ ] No view shadows an imported helper with a local copy. - [ ] `explorerLink` is either adopted at all three hand-built call sites or removed. - [ ] `ETHEREUM_SEPOLIA_CHAIN_ID` duplication is resolved to a single source of truth. - [ ] `addressColor` and `etherscanAddressUrl` are no longer exported. - [ ] The `getTotalValueUsd` decision is recorded, and if it is a behaviour change it has been split into its own issue. - [ ] Timestamps still render identically to before on AddressDetail, AddressToken, and TransactionDetail, including under the UTC setting. Confirm by inspection and say so in the PR. - [ ] `TODO.md` updated in the same commit. - [ ] `make check` passes.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#168
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?