fix: clear the 12 pre-existing lint errors (1 React refs correctness fix + typing debt)

[Ryanmello07]

What this is

This repo has never had CI, so nobody ever saw that npm run lint fails with 12 errors on main. The new build-and-test workflow in #35 surfaced them, and the same lint step fails in urnetwork/build #19. This branch fixes all 12 in the code — no eslint-disable, no config loosening, no rule severity changes, no renames-to-silence.

Verified on this branch (Node 18.19.1, eslint 9.39, tsc 5.9, vitest 4.1):

gate	before	after
npm run lint	12 errors	0 errors
npx tsc -b	clean	clean
./test.sh (tsc -p tsconfig.test.json + vitest)	3 files / 28 passed	3 files / 28 passed

Two commits, deliberately separated, because they are not the same kind of change.

---

Commit 1 — correctness: stop writing refs during render

This is the one worth real review. The other 10 errors are typing debt; these 2 were react-hooks/refs catching an actually-unsound pattern.

useConnectionManager kept the latest SDK callbacks in refs using bare assignments in the hook body:

const authFnRef = useRef(authNetworkClient);
const removeFnRef = useRef(removeNetworkClient);
authFnRef.current = authNetworkClient;      // <- during render
removeFnRef.current = removeNetworkClient;  // <- during render

Those refs are read by the ConnectionManager, which is constructed once (useCallback with [] deps) and outlives every render. It invokes the auth thunk from timers — silentRenew() at 80% of the proxy lifetime, and the scheduleReconnect() retry — i.e. long after the render that published the value has finished.

That is the unsound part: React may begin a render and discard it, but the discarded render has already published its callback into a live, long-lived object. A renew or reconnect firing afterwards would use a callback the committed UI never adopted.

The fix moves both writes into a dependency-array-less effect, which runs after every commit and therefore stores the latest committed value. The useRef seeds stay, so the refs are never empty before the first commit.

What changes for users today: nothing — and I want to be straight about why

I could not find a reachable failure path on main, and I don't want to oversell this as a live bug. Three things independently close every path, all re-verified in the tree:

1. No discardable render exists in this app. grep over src/ and elements/src/ finds no startTransition, useDeferredValue, Suspense, or <Activity>, and the React Compiler is not enabled (vite.config.ts uses plain @vitejs/plugin-react). StrictMode is on, but its double-render writes the same value twice, which is idempotent.
2. The published values never actually change. The SDK memoises authNetworkClient on [api, token] and removeNetworkClient on [api]; api is pinned in a useRef by URNetworkAPIProvider, and token cannot change while the hook is mounted (see 3). So both lines only ever rewrite the value already stored.
3. The hook never renders with a null token. AppRoutes.tsx:27 gates <ConnectScreen/> behind isAuthenticated (!!token), and useConnectionManager is called only from ConnectScreen.tsx:48. setAuth is called only in AuthInitial (unmounted by then); clearAuth is called only from ConnectScreen's own logout path, which unmounts ConnectScreen rather than re-rendering it with a new token.

So: a genuine violation of a rule that is right about the mechanism, fixed because the mechanism is unsafe — not because a user is hitting it today. It is also a precondition for ever turning on the React Compiler, which may skip re-executing a straight-line statement like that assignment regardless of concurrency.

Rejected alternatives: useEffectEvent (these functions are stored inside ConnectionManager and called from bare timers outside React, which effect events may not do); recreating the ConnectionManager when the callback identity changes (it owns live state — multiClientIds, ping interval, renew timeout, operationLock — and rebuilding it would drop the provisioned proxy pool mid-session).

---

Commit 2 — typing debt: 9 × no-explicit-any + 1 × no-unused-vars

No behaviour change. as any erases at compile time, and the single runtime edit (dropping a trailing listener parameter) is invisible to the caller.

Note for reviewers scanning for weasel-typing: no type was narrowed to unknown, and no interface was invented to describe something I couldn't substantiate. Every any fell into one of two buckets:

Bucket A — a real external typing gap (7 sites)

@types/chrome is the only extension typing installed and it declares chrome only; nothing in the dependency tree declares Firefox's browser global, which is why every access was cast through any. New src/types/firefox-webext.d.ts declares only the members this codebase actually calls — proxy.onRequest add/remove/hasListener, the optional proxy.onError, and a request shape containing just url. Every member is optional, matching the fact that Chrome has no browser at all and every call site feature-detects.

Two design points worth a look:

• It's a module that call sites import type, not declare global { var browser }. Partly because keeping browser unreachable as a bare identifier is correct (browser?.x throws a ReferenceError on Chrome — optional chaining doesn't guard an undeclared identifier — whereas globalThis.browser?.x is a plain property read); partly because tsconfig.test.json sets "include": ["tests"], so an ambient file under src/ would not reliably reach the test program. kill-switch-apply.ts and proxy-manager.ts are test-reachable via tests/bridge/*, so this matters. An explicit import type follows module resolution and works in both programs.
• proxy-manager.ts had byte-identical local copies of FirefoxProxyInfo / FirefoxProxyDetails; they move into the shared file instead of being duplicated a second time.

Bucket B — casts that were simply unnecessary (2 sites)

• sso.ts:51 — (browser as any).runtime?.lastError?.message. browser is already declared as typeof chrome | undefined at the top of that same file, and typeof browser !== "undefined" narrows it. Cast deleted, nothing added.
• use-provider-list-enhanced.ts:193 — (api as any).networkProviderLocations(). useAPI() returns URNetworkAPI, and networkProviderLocations(): Promise<FindLocationsResult> is declared in the SDK's own dist/api.d.ts:14. Cast deleted, nothing added.

The unused-var

background/index.ts:126 — dropped the unused third parameter of the SSO tab listener. tsconfig sets noUnusedParameters and tsc honours the _tab underscore convention, but @typescript-eslint/no-unused-vars resolves here as bare [2] with no argsIgnorePattern, so the two tools disagreed. It is the trailing parameter and Chrome passes all three arguments regardless of declared arity, so removing it changes nothing.

---

Things a reviewer who owns this code should check

1. proxy-manager.ts grew two small structural changes that typing getFirefoxProxyApi() forced, and they're the only non-mechanical edits in commit 2:
   • ensureFirefoxListener() now returns the listener it guarantees, instead of void (which left callers holding FirefoxProxyRequestListener | null and failing the typecheck). addFirefoxProxyListener uses that return value.
   • enableMultiIp() now narrows proxy.onRequest with an explicit if (!onRequest) return;. This is redundant at runtime — the existing isFirefoxProxyApiAvailable() guard on the first line already establishes it — but the type checker can't see through that function. If you'd rather express it differently, say so.
2. The Firefox surface is hand-written and intentionally partial. If you know of another browser.* member the extension touches (or plans to), it belongs in that file. Adding @types/firefox-webext-browser as a devDependency would be the alternative; I didn't, because it pulls in a full second WebExtension namespace to type six property reads.
3. The test suite does not cover any of this. tests/bridge/{origin-gating,refresh-jwt,verbs}.test.ts is bridge-only and exercises neither the hook nor the proxy manager. A green suite here is not evidence of behaviour preservation — the argument above is. Firefox proxying and the connect/renew path are worth a manual smoke test.
4. declare var browser was tried and rejected, in case it looks like the obvious approach: it typechecks, but eslint then fails on no-var (severity 2, from typescript-eslint's eslint-recommended layer), and declare const/let can't back a globalThis.browser access since only var/function global declarations become properties of typeof globalThis.

Deliberately not fixed

• eslint.config.js is untouched. Adding argsIgnorePattern: '^_' would also have made the _tab error disappear, but it's a lint-config change, it's unnecessary once the trailing param is gone, and it would permanently hide future trailing-unused-arg bugs. Worth recording that the repo has a real convention mismatch here (tsc honours _, eslint doesn't) — but the fix for that is a conversation, not this PR.
• The other three underscore-prefixed params stay (_sender at background/index.ts:200 and :265, _target at content/geo-main.ts:152). ESLint's default args: "after-used" correctly doesn't flag them: each is followed by a used parameter, so deleting one would shift the arguments. Confirmed by the typecheck failing when tried.
• The three copies of isFirefox() were not deduplicated. Merging them is a refactor beyond what the errors require; they now share the type, not the function.
• An SDK bug found on the way, out of scope here: useRemoveNetworkClient is memoised on [api] while its body closes over token, so a token change can never be delivered to it. If a token change during mount ever became possible, releaseMultiClientIds would call with a stale token and get back a resolved { error: ... } — not a rejection — which .catch(() => {}) at connection-manager.ts:339 never sees and Promise.allSettled discards, leaking network clients silently. That's upstream in @urnetwork/sdk-js, not here.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MAXFxG1EK4jTxQ1iW73BUr