refactor: eliminate any types in core (non-clis) files#886
Merged
Conversation
jackwener
force-pushed
the
claude/festive-williams
branch
3 times, most recently
from
67025fe to
c915959
Compare
Replace explicit `any` with `unknown` + narrowing or concrete types across
all core src/ files (non-`clis/**`). Core drops from ~60 `any` occurrences
to a handful of documented, unavoidable boundaries.
Mechanical error-handler cleanup (uses getErrorMessage() from errors.ts):
- cli.ts, cascade.ts, download/*.ts, external.ts, plugin.ts, doctor.ts
— catch (err: any) → catch (err) + getErrorMessage(err)
Pipeline steps — typed params with per-step interfaces:
- pipeline/steps/intercept.ts — InterceptParams, signature uses unknown
- pipeline/steps/tap.ts — TapParams
- pipeline/steps/download.ts — DownloadParams + DownloadedItem
(ytdlp_args is now coerced via String(v) for defence-in-depth)
Probe / boundary typing:
- cascade.ts — FetchProbeResponse interface; also fixes a latent bug
where result.success could be assigned undefined (masked by any)
by wrapping with !!(…)
Browser-side injected scripts — structural types at the TS boundary
(types stripped by tsc emit before .toString() runs, runtime unchanged):
- scripts/store.ts — PiniaStore / VuexModule / VueApp
- scripts/framework.ts — VueAppEl / FrameworkWindow
Runtime detection:
- runtime-detect.ts — BunGlobal interface; getRuntimeVersion reads Bun
into a local to avoid non-null assertions.
Test files — precise structural casts replacing `as any`:
- browser.test.ts — `{ _state: string }` cast for private
state; full DaemonStatus shape for
the getDaemonHealth mock
- browser/dom-helpers.test.ts — globalThis as Record<string, unknown>
- browser/cdp.test.ts — (...args: unknown[]) in mock handlers
- runtime-detect.test.ts — matches runtime-detect.ts BunGlobal
- output.test.ts — logSpy.mock.calls typed with unknown[]
- engine.test.ts, snapshotFormatter.test.ts, pipeline/executor.test.ts
— narrow structural casts / removed stale any casts
Verification:
- npx tsc --noEmit: clean
- npx vitest run (excluding e2e/smoke): 1415 passed, 1 skipped
jackwener
force-pushed
the
claude/festive-williams
branch
from
c915959 to
93de3af
Compare
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.
Summary
Replace explicit
anywithunknown+ narrowing or concrete types acrosssrc/core files (non-clis/**). Reduces coreanycount from ~51 → 9, where the remaining 9 are explicitly documented cross-boundary exceptions.Changes
Mechanical error-handler cleanup (uses existing
getErrorMessage()fromsrc/errors.ts):src/cli.ts,plugin.ts,external.ts,doctor.ts,cascade.ts,download/index.ts,download/media-download.ts—catch (err: any)→catch (err)+getErrorMessage(err)orinstanceof Errornarrowing.Pipeline steps — typed params with per-step interfaces:
src/pipeline/steps/intercept.ts—InterceptParamsinterface, signature usesunknown.src/pipeline/steps/tap.ts—TapParamsinterface.src/pipeline/steps/download.ts—DownloadParams+DownloadedItemtypes, items typed asArray<Record<string, unknown>>.Injected browser scripts — structural types at the TS boundary (erased at runtime, so
.toString()output is unchanged):src/scripts/store.ts—PiniaStore/VuexModule/VueApp.src/scripts/framework.ts—VueAppEl/FrameworkWindow.Test files — precise structural casts replacing
as any:src/browser.test.ts,engine.test.ts,snapshotFormatter.test.ts,pipeline/executor.test.ts.Incidental fix:
src/cascade.ts—result.success = resp?.ok && resp?.hasDatacould assignundefinedto abooleanfield (masked byany); now!!(...).Deferred (documented with
eslint-disable+ rationale)src/types.ts—IPage.evaluate/snapshot/tabs/...still returnPromise<any>. These cross the untyped-JS boundary (in-pageevalresults, extension responses). Tightening tounknowncascades into ~450 narrowing sites acrosssrc/clis/**.src/registry.ts—CommandArgs = Record<string, any>.kwargsare destructured across all adapters with the same cascade.Both now carry explicit comments so new
anycan't be added silently. Cleanup acrossclis/**is a separate follow-up.Test plan
npx tsc --noEmit— cleannpx vitest rununit + integration (excludingtests/e2e/**,tests/smoke/**) — 260/260 passclis/anyoccurrences remain beyond the 9 documented exceptions