Skip to content

chore(code-health): resolve remaining react-doctor exceptions introduced by 0.9.3 cleanup (blob URLs, platform-split, try/finally) #363

Description

@masch

Pre-flight Checks

Problem Description

After the react-doctor 0.9.3 cleanup (17 findings → 100/100), apps/mobile/doctor.config.ts still contains 3 documented exceptions that mask real work. These are not the screen-refactor exceptions tracked in #147 — they are:

  1. no-create-object-url-without-revoke ×4 — blob URLs that are the live expo-audio web playback source
  2. deslop/unused-exportDeviceService (platform-split false positive)
  3. react-hooks-js/todo — React Compiler try/finally limitation (overlaps with chore(code-health): eliminate React Doctor exceptions from doctor.config.ts #147, detailed below)

Each exception is justified with evidence, but the goal is to eliminate them with real fixes, not keep them forever.

1. no-create-object-url-without-revoke ×4

File Line Context
src/hooks/use-track-download.ts 143 Web cache-validation path creates a blob URL for immediate playback
src/store/download-manager-store.ts 131 Cache hit path
src/store/download-manager-store.ts 151 Fallback path (no Cache Storage / no reader)
src/store/download-manager-store.ts 177 Normal download path

Why it is flagged: react-doctor only proves same-scope revocation (URL.revokeObjectURL syntactically in the same function as URL.createObjectURL).

Why the naive fix is wrong: On web these blob URLs ARE the audio source passed to expo-audio. Revoking in the creation scope kills playback immediately.

What was already done: lifecycle-correct revocation is implemented and behavior-preserving:

  • download-manager-store.ts: webObjectUrlRegistry (module-level map) registers URLs at all 3 creation sites; revoked in cancel (completed entries) and _failDownload.
  • use-track-download.ts: ownedWebObjectUrlRef — the hook owns the URL and revokes it on replacement, clear, and unmount cleanup.

Path to a real fix (pick one):

  • (a) Watch react-doctor/deslop for same-scope registry pattern support (revocation via a tracked registry proved from a single ownership point) and drop the exception when provable.
  • (b) Redesign web playback to avoid long-lived blob URLs (e.g. revoke-and-recreate on track switch is already handled; a future approach could stream/cache differently), then delete the exception.
  • (c) If neither lands, accept and keep the documented exception (current state).

2. deslop/unused-exportDeviceService

  • File: src/services/device-service.ts:7 (also device-service.web.ts)
  • Flagged: "Unused export: DeviceService"
  • Reality: DeviceService.getPlatformDeviceId is imported and used by src/storage/app-storage.ts:3,28 and app-storage.web.ts.
  • Root cause: deslop resolves platform-split files (.ts / .web.ts) inconsistently — it associates the import with the .web.ts variant and counts zero importers for device-service.ts. Same false-positive family already ignored for config-cache and translation-cache (pre-existing team decision).

Path to a real fix:

  • (a) Verify whether newer deslop/react-doctor releases resolve platform-split correctly; drop the exception when proven.
  • (b) Restructure so the analyzer can see the reference (e.g. a single non-split module re-exporting both platform implementations) — measure whether this reduces analyzer confusion without hurting Metro platform resolution.
  • (c) If it remains an analyzer limitation, keep the documented exception (current state).

3. react-hooks-js/todo — React Compiler try/finally

  • File: src/app/(tabs)/explore.tsx:101 (+ the same pattern in experiences.tsx, track-detail-view.tsx, trip-detail-view.tsx, track-map.tsx)
  • Flagged: (BuildHIR::lowerStatement) Handle TryStatement with a finalizer ("finally") clause — a known React Compiler limitation.
  • Current state: the exception predates this cleanup (team decision, already documented in doctor.config.ts); the rule name changed from react-compiler to react-hooks-js/todo between 0.5.x and 0.9.x and the override was updated to match.
  • Important: the finally block is not decorative — it guarantees setLoading(false) on both success and rejection (no-loading-flag-reset-outside-finally fix depends on it).
  • Note: this overlaps with chore(code-health): eliminate React Doctor exceptions from doctor.config.ts #147, which explicitly calls out the react-compiler exception; track resolution there and remove the react-hooks-js/todo entry from the override when the compiler supports it.

Path to a real fix:

Verification & Current State

  • react-doctor 0.9.3 (repo-pinned, bun run doctor): 100/100, No issues found with the documented exceptions.
  • 70 Jest suites / 508 tests pass; tsc --noEmit clean; prettier clean on touched files.
  • Related cleanup work (this PR set): useMemo removed in use-instructions-audio.ts (compiler-managed), effect cleanups added to use-purchase.ts/use-track-download.ts, only-export-components fixed via icon-utils.ts + tw/factory.ts, finally fix in explore.tsx, cancelled-flag in use-purchase.ts.

Acceptance Criteria

  • Each of the 3 exception groups is either (a) resolved with a real code fix and its doctor.config.ts override removed, or (b) closed as an analyzer/upstream limitation with a link to the upstream issue.
  • bun run doctor stays at No issues found (or the removed exception is provably gone from the output).
  • All tests + typecheck remain green after any change.

Affected Area

Code quality / linting — apps/mobile/doctor.config.ts, src/hooks/use-track-download.ts, src/store/download-manager-store.ts, src/services/device-service.ts

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requesttype:choreMaintenance or tooling changes

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions