chore(lint): eliminate ESLint suppressions and adopt high-value rules#234
Open
Kelbie wants to merge 15 commits into
Open
chore(lint): eliminate ESLint suppressions and adopt high-value rules#234Kelbie wants to merge 15 commits into
Kelbie wants to merge 15 commits into
Conversation
Adds devDependencies for the next-tier lint rules: neverthrow must-consume-result (@okee-tech), no-secrets, security, unicorn, sonarjs, and a direct dep on eslint-plugin-import (previously only transitive via expo). No rules enabled yet.
Replace the three object-literal `{...} as RoutstrError` throws with a
`makeRoutstrError` factory (satisfies consistent-type-assertions without
a cast) and narrow `per_request_limits: any` to `unknown` (the field is
never read).
Replaces opaque eslint-suppressions.json counts with documented, scoped exceptions for genuinely-legitimate cases: - redux/**/*.deprecated.ts: no-explicit-any off (live migration scaffolding reading arbitrary old persisted Redux blobs; slated for deletion). - routstr/api.ts: no-restricted-globals off (every endpoint needs the raw Response for its status-aware error pipeline + SSE streaming; fetchJson cannot carry that contract). - Button/SpriteView/AmountFormatter: no-restricted-imports off with a TODO(reanimated-migration) note (legacy Animated, deferred to its own PR). - easeGradient.ts: inline disable (MIT adaptation driving RN's internal color-interpolation API; no Reanimated equivalent).
Clears the no-restricted-syntax (hex literal) suppressions: - White-on-dark icons/text -> INVARIANT_WHITE; immersive/story/shadow blacks -> INVARIANT_BLACK. - New shared brand constants for genuinely cross-site values: LIKE_ACCENT (#ff5a7a), AMBER_ACCENT (#f59e0b), WALLPAPER_PLACEHOLDER gradient. - Color-definition homes (map/categories marker palette, useThemeColor resolver fallback, RowStatsAccent STAT_COLOR_* exports) added to the no-restricted-syntax exempt list alongside themes.ts/brandColors.ts. - Localized intentional invariants (notification-type palette, native-tab theme colors, default decorative gradient, avatar placeholder fill) get documented inline disables instead of opaque bulk-suppression counts. No rendered colors change: every swap maps a literal to a constant of the same value or an existing token of the same value.
The eslint-disable-next-line directive sat above the explanatory comment block, so it targeted a comment line instead of the restricted import. Move the directive to the line immediately before the import.
Clears the no-explicit-any suppressions for live (non-deprecated) code: - Concrete types where derivable: forwardRef<ElementRef<typeof View>>, StyleProp<ViewStyle>, a structural Measurable ref type (hero-transition), typed routstr stream via Awaited<ReturnType<...>>, Href for the router pathname, the reanimated scroll-handler type, typed mint-info/theme-registry records, and narrowed object casts. - Redundant casts dropped: the (delta as any) chat-stream casts are covered by the zod chunk spine; Supercluster <any,any> generics fall back to the library's AnyProps default. - Documented inline disables only where any is genuinely load-bearing: typedUpdate's implementation signature (behind two typed overloads), migrateSettings reading a legacy Redux blob, gorhom/bottom-sheet render-prop props, react-native-image-colors' platform union, the compose() provider util, and the Animated.FlatList ref typing gap.
Clears the consistent-type-assertions suppressions (object-literal casts): - Typed seams instead of casts: reduce<Record<string,number>>, typed .catch arrow returns (BalanceMap / byMint result), satisfies for the NDK->Applesauce event and the custom-sheet payloads, drop the redundant useMemo<T> inner cast. - Drop unneeded spread-copy cast in PaymentRequestScreen. - Assert the merged variable (not the literal) in createMergeWithSchema where TS can't prove a generic spread is complete. - Documented inline disables for the two genuinely-load-bearing cases: the generic TActiveChildren empty default and gorhom's undocumented useDirectView contentContainerProp.
…file Enables the researched high-signal rules, drives their backlog to zero (or to warn), and removes eslint-suppressions.json ENTIRELY (193 suppressed violations -> 0; the file is deleted, not emptied). Also includes the mechanical --fix output the autofixable rules produced (prettier, optional-chain collapses, import dedupe, dead-import removal) — no behavioural changes there. error tier (backlog driven to zero): - await-thenable, prefer-optional-chain - unicorn: no-thenable, no-instanceof-array, error-message, throw-new-error - sonarjs: no-identical-expressions, no-all-duplicated-branches, no-element-overwrite, no-collection-size-mischeck, non-existent-operator - import/no-duplicates, import/no-self-import - security: detect-pseudoRandomBytes, detect-eval-with-expression, detect-bidi-characters - (the 4 remaining optional-chain + 2 Error-message sites fixed by hand) warn tier (visible, never suppressed): - neverthrow/must-consume-result - no-secrets (tuned tolerance + ignore Nostr/Cashu encodings; off in fixtures) - import/no-cycle (depth-gated) - switch-exhaustiveness-check (~18 genuine gaps -> focused follow-up) - prefer-nullish-coalescing (||->?? needs a falsy-0/'' audit before promotion) eslint-plugin-import promoted to a direct dependency. Per the no-new-suppressions rule, every adopted rule either finished at zero or landed at warn.
tsc --noEmit surfaced 15 errors where a removed `any`/cast was load-bearing: - Colada: the useMemo<MachineOperations> object needed both the assertion AND contextual param typing — extract to a variable, assert that, and type the one inline operation's params explicitly. - useAiSend catch, list.tsx router pathname, Supercluster Options generics: the value is genuinely untyped/3rd-party — restore `any` with a documented inline disable rather than a wrong type. - PaymentRequestScreen: double-assert through unknown (types don't overlap). - compose(): guard the reduce accumulator off undefined instead of `as any`. type-check and lint both clean (0 errors).
…port - eslint-plugin-import is provided transitively by eslint-config-expo and is referenced via import/* rule names (not require()d), so the direct devDependency is redundant — knip flagged it. The import/no-duplicates, no-self-import and no-cycle rules still resolve through expo. - Drop the unused `export` on the RelayHealth type (used only internally; pre-existing knip finding, fixed here so the gate stays green).
React Compiler is enabled, so manual useMemo wrapping a cheap pure value is dead weight it already covers. Commit 584186b removed the codemod-safe 360 (1320->960); this pass clears 70 of the residue it left behind — value memos stranded in files a coarser per-file filter preserved. Method: unwrap only single-line `useMemo(() => expr, [deps])` value memos (no React.memo wrappers, no useCallback, excluding animation/gesture/Skia/ shared-value expressions), then verify each batch against a fresh `react-doctor --scope changed` run. Reverted every removal that introduced a new compiler-health finding rather than force it through: - targetItem (ThreadView) fed a kept useCallback -> whole-component bailout. - PollCard / NearPayScreen / LightningStrike / SkeletonExitShimmer memos fed effects -> no-effect-with-fresh-deps (effect re-runs every render). - SettingsDesignSystemTimelineScreen -> purity. react-doctor's per-removal scan catches these value/callback bailouts (preserve-manual-memoization / no-effect-with-fresh-deps / purity); it does NOT catch React.memo render-bailouts (verified: removing the load-bearing MenuButton memo produced no finding), so component wrappers are left alone. The rn-no-non-native-navigator (x3) and js-length-check-first (x2) findings from the same pass remain confirmed false positives and untouched: RN v7 has no native drawer, two of three nav sites are type/hook imports, and the flagged compares are an intentional prefix match and an already-guarded equality check. react-compiler-no-manual-memoization 960 -> 890. type-check 0 errors, eslint 0 errors / 0 new warnings, 1664 tests pass (one source-assertion test updated to match the unwrapped contactRows line).
React Compiler (app.json experiments.reactCompiler) auto-memoizes values, callbacks, and component output, so most manual useMemo/useCallback is redundant. The react-perf/jsx-no-new-* eslint rules were deliberately paired with the compiler to flag prop instability — which directly contradicts deleting those memos. Make the compiler the source of truth: remove the three react-perf rules + plugin + devDependency, then sweep the safe subset of react-compiler-no-manual-memoization findings (890 -> 580). Bounded to compiler-covered cases only. Kept every React.memo wrapper, preserve-manual-memoization site, commented memo, and generic useMemo<T>/useCallback<T> (dropping the type arg breaks inner-param inference). Skipped any binding consumed by a hook dependency array (removing it re-fires the effect every render) and multi-statement block useMemo. Verified per-file with `react-doctor --scope changed` and a full before/after per-(file,rule) count delta against the original report: total issues 2434 -> 2121, zero net-new issues of any rule. tsc, eslint (0 errors), and knip all clean. 16 files with genuine new findings were reverted wholesale. Left two false-positive clusters untouched (see ADR 0006): rn-no-non-native-navigator (fix target @react-navigation/native-drawer does not exist; sites are a deliberate expo-router/drawer surface, a useDrawerProgress hook, and a type-only BottomTabBarProps import) and js-length-check-first (both are the rule's own documented exceptions: an existing length guard, and an intentional prefix-match).
Move render-stable helper functions out of component bodies to module scope. None captured component state/props — only their own params, module imports, and module consts — so React rebuilt them on every render for nothing, and a fresh function identity defeated memoized children. Pure relocation; zero behaviour change (the compiler already stabilised these, so their location never affected runtime). Clears all 30 react-doctor prefer-module-scope-pure-function findings. MerchantDetailScreen's handleCall/handleEmail/handleContactPress and SearchResultRows' renderItem were hoisted as cascade follow-ups once their sole dependency (handleOpenURL / renderSearchResult) moved out. Verified against the real tool: prefer-module-scope 30 -> 0, total react-doctor 2121 -> 2012, zero net-new findings of any rule; tsc, eslint (0 errors), and knip all clean.
The first React-Compiler memo sweep (584186b) removed ~20 React.memo/memo() wrappers and never restored them — including the app's most render-sensitive components (charts, animated backgrounds, gradient overlays, feed media, transfer/transaction list rows). React.memo is a props-comparison RENDER bailout, a different mechanism from value memoization; the compiler does not reliably replace it (React Compiler 1.0 guidance, point #2). The "keep all wrappers" rule was only adopted in pass 2 — pass 1 had already stripped them. Runtime logs confirmed the regression: a log-doctor renders pass showed the three instrumented stripped components re-rendering repeatedly (tx.monthly_chart 6x, bg.view 7x, ScrollableGradientOverlay 4x) — the user-reported lag. Changes: - Re-wrap all 20 stripped components in React.memo/memo(). - Restore getMintInfo's useCallback([manager]) in useMintManagement — a callback consumed as a useEffect dependency in CONSUMER hooks (useMintInfo.ts, useMintContacts.ts); the sweep's "skip dep-array bindings" guard only checked the same file, so this cross-file escape hatch slipped. - Add a React Compiler coverage gate: scripts/check-react-compiler.mjs + check:react-compiler npm script + CI step. It runs react-compiler-healthcheck and fails when compiled < total. The eslint-plugin-react-compiler rule that would normally catch a bailout is dead under the repo's zod@4 override, so this restores the signal using the compiler itself as source of truth. - ADR 0007 documents the finding + gate; corrects ADR 0006's "kept every wrapper" claim. Verified: react-compiler-healthcheck 596/596 (zero bailouts), tsc 0, eslint 0 errors, knip 0, jest pass, prettier clean.
Second audit pass over the React-Compiler memo sweep diff. - Restore 3 gesture useMemos the bulk pass (8bd4f74) stripped despite the earlier passes' explicit policy to exclude gesture/animation memos: ThreadEmbedSheet/DistributionSlider/NearPayScreen panGesture+gesture. A fresh Gesture.Pan() each render re-registers the GestureDetector handler — a footgun the render-count log wouldn't surface. Re-wrapped with their original deps. - Cut the two over-logging events that were 61% of a sample session's log volume (the user-reported "repeated logs"), neither a memo bug: * mint.info.fetch.success (31%): the hook re-logged success on every getMintInfo call including instant SWR cache hits, duplicating the cache layer's own store.mint_info.* hit/miss/fetch logs. Removed the redundant hook-level start/success debug logs (kept the error log). * nostr.client.npub_to_pubkey (30%): a debug log on every call of a pure decode invoked per-row across follower/contact lists. Removed (kept decode-failure). - Restore addQuery's useCallback (useRecentSearches) — consumed as a cross-file useEffect dep; deps [addSearch, surface] are stable. Context-provider value memos were investigated and deliberately left compiler-owned: restoring them introduced 7 net-new react-hooks/exhaustive-deps warnings (their callback deps aren't manually memoized), and the compiler already stabilizes them (596/596 compile) under the new coverage gate. See ADR 0007 "Second pass". Verified: tsc 0, eslint 0 errors, knip 0, jest 1664/1664, prettier clean, react-compiler-healthcheck 596/596.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
The
eslint-suppressions.jsonbulk-suppression file was a code smell — 193 lintviolations across 72 files swept under the rug rather than fixed. This PR
eliminates it entirely (the file is deleted, not emptied) and adopts a tier of
high-value bad-pattern rules surfaced by deep research into our stack (RN/Expo,
TypeScript type-aware, neverthrow, Nostr/Cashu, security).
Built on top of
chore/react-doctor-top-fixes(PR base).Suppression cleanup (193 → 0)
@typescript-eslint/no-explicit-anyredux/*.deprecated.ts→ scoped config exemption (live migration scaffolding reading arbitrary legacy persisted blobs); the rest → concrete types (ElementRef,StyleProp, structuralMeasurable, typed records/streams) or documented inline disables where genuinely load-bearing (typedUpdate impl behind typed overloads, gorhom/image-colors 3rd-party shapes, caught errors)no-restricted-syntax(custom)INVARIANT_WHITE/INVARIANT_BLACK/newLIKE_ACCENT·AMBER_ACCENT·WALLPAPER_PLACEHOLDERtokens; colour-definition homes (map/categories,useThemeColor,RowStatsAccent) → exempt list; localized invariants (notification palette, native-tab theme) → documented disables. No rendered colour changes — every swap maps a literal to a constant of the same value@typescript-eslint/consistent-type-assertionsreduce<T>, typed.catchreturns,satisfies), dropped redundant casts, assert-the-variable for generic merges; 2 documented disablesno-restricted-importsAnimatedfiles → scoped override w/TODO(reanimated-migration);easeGradientMIT adaptation → inline disableno-restricted-globalsroutstr/api.tsrawfetch→ scoped override (every endpoint needs the rawResponsefor its status-aware error pipeline + SSE;fetchJsoncan't carry that contract)unused-imports/no-unused-importsLegitimate exceptions now live as documented, scoped config overrides or inline
eslint-disable+ one-line rationale — never as an opaque count in a baseline file.New rules adopted
error tier (backlog driven to zero, incl. hand-fixes):
await-thenable,prefer-optional-chain,import/no-duplicates,import/no-self-import, unicorn (no-thenable,no-instanceof-array,error-message,throw-new-error), sonarjs bug rules (no-identical-expressions,no-all-duplicated-branches,no-element-overwrite,no-collection-size-mischeck,non-existent-operator), security (detect-pseudoRandomBytes,detect-eval-with-expression,detect-bidi-characters).warn tier (visible, never suppressed — same posture as the existing
react-perf/react-compiler warns):
neverthrow/must-consume-result,no-secrets(tuned + off in fixtures),
import/no-cycle(gated),switch-exhaustiveness-check,prefer-nullish-coalescing.Deferred (follow-up, surfaced at
warnso they stay visible)switch-exhaustiveness-checkgapsprefer-nullish-coalescingpromotionno-secretspromotionwarnuntil the floor is zeroAnimated→Reanimated v4 rewritesAmountFormatter,SpriteView,Buttonripple — animation-behaviour risk, explicitly out of scopeVerification
0 errors(2431 warnings — pre-existing react-perf + the new warn-tier rules; CI does not fail on warnings)tsc --noEmit):0 errors(caught & fixed 15 type regressions where a removedany/cast was load-bearing)jest):1664 passed / 150 suiteseslint-suppressions.json: deleted (193 → 0)Notes
gpt-5.5); Codex flagged theButton.tsx3rd legacy-Animatedfile and thefetchJson-loses-status risk forroutstr, both confirmed and handled.
eslint-plugin-importis provided transitively byeslint-config-expo(used viaimport/*rule names), so no new direct dep was needed.🤖 Generated with Claude Code