Skip to content

fix(sdk): promote three bug-catching lint rules to error and fix the fallout - #2945

Open
alex-connolly wants to merge 1 commit into
mainfrom
chore/lint-promote-rules-to-error
Open

fix(sdk): promote three bug-catching lint rules to error and fix the fallout#2945
alex-connolly wants to merge 1 commit into
mainfrom
chore/lint-promote-rules-to-error

Conversation

@alex-connolly

@alex-connolly alex-connolly commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #2944.

Why

#2944 shipped because the rule that would have caught it was set to warn, so nothing failed CI. This promotes three rules that catch runtime defects rather than style preferences, and clears the codebase so the gate stays green.

Rule Was Now Catches
typescript/no-non-null-asserted-optional-chain warn error a?.b! — undefined at runtime, typed as present
typescript/no-base-to-string warn error "[object Object]" in error messages and analytics
eslint/preserve-caught-error error rethrow without cause, discarding the original stack

Two real defects surfaced

1. Bridge fee validation has been dead code. tokenBridge.getFee passed an uninvoked async function to Promise.all, so it resolved the function object and validateChainIds never ran:

-      async () => {
+      (async () => {
         if (req.action !== BridgeFeeActions.FINALISE_WITHDRAWAL) {
           await validateChainIds(req.sourceChainId, req.destinationChainId, this.config);
         }
-      },
+      })(),

⚠️ Behaviour change: this restores validation that has not been running. Callers passing mismatched chain ids will now get the intended error instead of silently proceeding. Worth a look from someone who knows the bridge callers.

2. Sale failure analytics reported nothing. PayWithCoins called error.toString() on a SignOrderError ({ type, data }, not an Error), so every checkoutPrimarySalePaymentMethods_FailEventFailed event carried the literal string "[object Object]". Now sends error.type.

Also fixed

On oxlint --fix

Not used. It is not safe on this repo. A trial run produced:

  • .sort().toSorted() in 11 places — ES2023 array methods (Chrome 110 / Safari 16.4 / Node 20), shipped unpolyfilled. Note this is worse than a compile error: widgets-lib sets "lib": ["dom", "dom.iterable", "esnext"], so toSorted type-checks there and would have shipped silently. It only fails to compile in packages that inherit target: ES2022 from tsconfig.base.json (orderbook, wallet, checkout/sdk, …). Verified both ways.
  • Dropped entries from React dependency arrays (×2) — silent behaviour change
  • Dead whitespace throughout cancellablePromise.ts

Which is the same failure mode as #2944: an autofix applied without review. Every change here is hand-written.

Deferred, with counts

Rule Count Why not here
no-floating-promises 230 Closest to #2944's root cause, but needs per-site judgment — void for genuine fire-and-forget, real handling for user-triggered actions. Blanket-voiding would silence the signal rather than fix it. Own PR.
no-unsafe-enum-comparison 93 Needs a canonical type chosen per comparison — a refactor, not a lint fix
react-hooks/exhaustive-deps 235 Every fix changes render behaviour and can cause infinite loops

Do not enable the radix rule. It is currently off (style category), and its autofix is what caused #2944.

Testing

  • pnpm typecheck clean across widgets-lib, orderbook, wallet, bridge-sdk, dex-sdk
  • widgets-lib: 39 suites / 265 tests pass
  • bridge-sdk + dex-sdk: 297 tests pass
  • oxlint packages/ reports 0 errors

🤖 Generated with Claude Code

…fallout

Follow-up to #2944. That bug shipped because the rule which would have caught
it was set to `warn`, so nothing failed CI. This promotes three rules that
catch runtime defects rather than style preferences, and clears the codebase
of them so the gate stays green.

Promoted to error:
- typescript/no-non-null-asserted-optional-chain — `a?.b!` is undefined at
  runtime while typed as present
- typescript/no-base-to-string — stringifying an object yields "[object
  Object]", silently gutting error messages and analytics payloads
- eslint/preserve-caught-error — rethrowing without `cause` discards the
  original stack

Two real defects surfaced by this:

- tokenBridge.getFee passed `async () => {...}` to Promise.all without
  invoking it, so Promise.all resolved the function object and the chain-id
  validation never ran. Note this restores validation that has been dead:
  callers passing mismatched chain ids will now get the intended error.
- PayWithCoins reported sale failures via `error.toString()` on a
  SignOrderError ({ type, data }), so every failure event carried the literal
  string "[object Object]" instead of the failure type.

Also fixed: an undefined provider could reach NetworkSwitchDrawer via
`from?.browserProvider!`; OrderSummary dereferenced a possibly-missing
smartCheckoutResult; SaleWidget built block-explorer links containing
"undefined"; fundingBalanceFees pushed fees with a missing required token.

`oxlint --fix` was not used. On this repo it rewrites `.sort()` to `.toSorted()`
(ES2023, while tsconfig targets ES2022), drops entries from React dependency
arrays, and leaves dead whitespace. Every change here is hand-written.

Deferred, with counts, to keep this reviewable:
- no-floating-promises (230) — the rule closest to #2944's root cause. Needs
  per-site judgment (`void` for fire-and-forget, real handling for
  user-triggered actions), so it gets its own PR.
- no-unsafe-enum-comparison (93) — needs a canonical type per comparison
- react-hooks/exhaustive-deps (235) — every fix changes render behaviour

Do not enable the `radix` rule. It is off (style category), and its autofix is
what caused #2944.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@alex-connolly
alex-connolly requested review from a team as code owners August 7, 2026 08:33
@nx-cloud

nx-cloud Bot commented Aug 7, 2026

Copy link
Copy Markdown

View your CI Pipeline Execution ↗ for commit 06508eb

Command Status Duration Result
nx run-many -p @imtbl/sdk,@imtbl/checkout-widge... ✅ Succeeded 58s View ↗
nx affected -t build,test ✅ Succeeded 1m 56s View ↗

☁️ Nx Cloud last updated this comment at 2026-08-07 08:36:46 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant