Files
software-mansion__argent/knip.jsonc
Hubert Gancarczyk 2740a3ec38 chore(knip): clear the parked dead-code backlog and drop the ceiling to 0 (#770)
> Built on top of #769, so this PR carries that commit too — the
workflows only run for PRs targeting `main`, which is why it is based
there rather than on the branch. #769 on its own is the minimal,
low-risk fix for the currently-red job; if it merges first this shrinks
to its own commit, and if this one merges first it closes #769 as well.

## What this does

#572 landed the gate with 215 findings parked under `--max-issues`. That
bounded the backlog but left it in place, and the first thing to touch
it (#663) pushed the total to 218 and turned the job red. This clears
the backlog and sets the ceiling to **0**, so the next unused export
fails the job instead of consuming headroom nobody knew was there.

| | before | after |
| --- | ---: | ---: |
| Unused exports | 47 | 0 |
| Unused exported types | 156 | 0 |
| Unused exported class members | 12 | 0 |
| `--max-issues` | 215 | **0** |

## Scope check first

Every workspace holding a finding is `private: true`.
`@swmansion/argent` — the one published package — had **zero**, so
nothing here moves a public API.

## Two shapes of fix

**Still referenced inside its own file → drop `export`** (146 types, 36
exports). The declaration stays put; it just leaves the module's public
surface. `tsc` keeps emitting these into the `.d.ts` as local
declarations that the exported signatures reference.

For the 146 **types** that is the whole story: they are erased, so the
emitted `.js` is byte-identical. For the 36 **values** it is not.
Dropping `export` from a `const`/`function`/`class` removes its
`exports.X` binding from the emitted `dist/*.js` — 36 declarations
across 26 files here. `packages/tool-server/dist/utils/device-info.js`
no longer carries `exports.REMOTE_PREFIX` or
`exports.VEGA_SERIAL_PREFIX`; `telemetry/dist/posthog.js` loses
`exports.POSTHOG_PROJECT_TOKEN`; in the ESM-emitting `argent-installer`,
`isTempRunnerPath` / `resolveLocalArgentDir` / `PACKAGE_ROOT` leave the
module namespace object entirely.

Nothing in the repo or in the `packages/argent-private` submodule at the
pinned revision reads any of them by path, so nothing breaks. Flagging
it because a `require()` of a built `dist/*.js` loses a dropped `export`
exactly the way it lost the deleted `__resetAndroidDevtoolsInstallCache`
this branch had to restore — so this half of the diff does deserve
runtime scrutiny, not a skim.

**Referenced nowhere → delete.** Orphaned test seams (`__test`,
`_testValidators`), stale compatibility re-exports, and a few real
orphans. The ones worth a reviewer's eye:

- **`localSimctl`** — the `SimctlBackend` strategy is only ever
constructed with `remoteSimctl`. What keeps the local iOS impl off it is
the handler signature: `buildIosLaunchHandler` /
`buildIosRestartHandler` read `services.nativeDevtools`, and
`ios-remote` is the only platform that declares that service, so the
local impl — which resolves native-devtools per device inside its own
handler — has nothing to hand them and shells out to `xcrun` itself.
- **`sendCharInsert`** — removing the `__sendCharInsert` test alias
exposed the underlying helper as unreferenced. It is superseded, not
unwired: the Chromium keyboard impl dispatches its own `type: "char"`
event inline (`keyboard/platforms/chromium.ts:58`), which is exactly
what this helper did. Chromium typing is unaffected.
- **Error metadata fields** — `ServiceNotFoundError.serviceId`,
`ToolNotFoundError.toolId` and their two siblings were never read. The
`e.serviceId` in `registry-error-events.test.ts` is a field on the event
object the handler pushes, not on the error class; the `err.toolId` in
`http.ts` is `NotImplementedOnPlatformError`, a different class that
keeps its fields. Flagging these in case they are wanted as deliberate
diagnostics.

Deleting `SourceMapsRegistry.toGeneratedPosition` / `findMatchingSource`
cascaded into their only helper (`buildSourceCandidates`) and then into
`projectRoot`, which had no remaining reader — so the constructor
parameter goes too. `StubSourceMapsRegistry` existed only to pass
`super("")`, so it goes with it.

## The three cross-workspace members

`ArtifactStore.register`, `ArtifactStore.list` and
`TypedEventEmitter.off` are all called from other workspaces — the
artifact route and screenshot tools in `tool-server`, and
`registry-listener` in `telemetry`. They are reported unused anyway, and
the reason is the **unbuilt tree** the gate runs against: every
workspace resolves `main`/`types` to a `dist/` that does not exist, so
`@argent/registry` resolves to nothing and the cross-workspace edge
never forms. Build the tree and all three findings disappear on their
own; a control member that really is dead is still reported in the built
run, so the pass is live either way. Nothing about this is specific to
`classMembers` — every cross-workspace reference is invisible the same
way.

Each is exempted at the declaration with a `@public` JSDoc tag rather
than by name in `knip.jsonc`, for one reason: **scope**. `ignoreMembers`
matches a name across the whole workspace, so it would also hide a
future dead `register`/`list`/`off` on any other class in
`packages/registry`; `@public` binds to the member it is written on.

What `@public` does **not** buy is a staleness check, and neither does
`ignoreMembers`. Measured on a clean unbuilt worktree: a bogus
`ignoreMembers` name and a redundant `@public` each leave the run at
exit 0 with no hint, while a bogus `ignoreDependencies` name exits 1
with `Remove from ignoreDependencies`. `treatConfigHintsAsErrors` is
live and simply has no producer for either exemption style — so when one
of the three stops being earned, nothing says so, and the JSDoc at each
member is the only record a re-audit has. Each one names its callers,
and `off` records how to re-derive them (rename the member and read the
`Property 'off' does not exist` errors; grepping `.off(` over-reports,
since 14 of the 38 call-site hits are Node `EventEmitter`s). One
correction to the commit message on that change: it says 35 hits where
the exact count is 38 — 14 Node `EventEmitter`, 24 `TypedEventEmitter`
(21 production, 3 in a test). The 14 and the 21 are right.

`knip.jsonc`, `CONTRIBUTING.md` step 5 and the workflow's
failure-explanation step all described a parked backlog and a ceiling to
stay under. All three are rewritten for a gate that must simply come
back empty, and all three now name the `@public` escape hatch — the
remedy for a symbol whose only caller is out of knip's reach.

## Verification

Run against a clean worktree with **no build output**, the way CI
counts:

- `npm run knip` — **exit 0, prints nothing**. Both passes empty, no
config hints.
- `npx tsc --build` — clean.
- `npm run lint` (`eslint . --max-warnings 0`) — clean.
- `npx prettier --check .` — clean.
- `npm test --workspaces` — tool-server 329 files / 3857 passed;
registry, telemetry, update-core, tools-client, mcp, cli green.
- `npm run typecheck:tests --workspaces` — 13 workspaces define the
script and all 13 are clean, but **the command itself exits 1**: npm
errors `Missing script: "typecheck:tests"` for the three that do not
define it rather than skipping them. Worth knowing before re-running
this bullet and reading the exit code as a failure.

End-to-end on a tool-server built from this branch, driven over its HTTP
API:

- **Chromium** (Electron smoke app) — `describe`, `screenshot`,
`gesture-tap` (counter advanced 0 → 1), `POST /api/clipboard/text` read
back out of the renderer, the WS `clipboardSync` command, and
`debugger-connect` / `debugger-status` / `debugger-evaluate`
(`sourceMapReady: true` with `StubSourceMapsRegistry` gone).
- **iOS** — `boot-device`, `launch-app`, `describe` via ax-service,
`screenshot`.
- **tvOS** — `boot-device`, `launch-app` on an Apple TV udid,
focus-driven `describe`. The registry snapshot then carries
`NativeDevtools:<tvOS udid>`, confirming injection **is** resolved on
tvOS through the local impl.
- **Android** — `boot-device` on a dedicated AVD, `describe` via
android-devtools, `gesture-tap`.
- **Metro source maps** — the registry driven against a real 12.5 MB
Metro `.map` over loopback, the way `Debugger.scriptParsed` drives it:
allowlist passes and `waitForPending()` resolves on both, while retained
heap after gc drops from 17.3 MB to 3.6 MB.


## Review follow-ups

Five commits on top, one per finding. The base is back on `main`, so the
workflows run again.

- **`docs(launch-app)`** — the services comment explained the empty
shape through `LaunchAppAndroidServices`, deleted here, and skipped
`LaunchAppVegaServices`, which is still there. Same edit in
`restart-app/types.ts`.
- **`docs(registry)`** — the `@public` tag on `ArtifactStore.register`
named 2 of 7 callers. Derived the full set the way the sibling tag on
`off` prescribes: renamed the member, read the 16 `TS2339` errors `tsc
--build` reports across flow-visual (5), screenshot (4),
native-profiler-stop (2), screenshot-diff (2), native-profiler-analyze,
react-profiler-analyze and screen-recording-stop. The re-derivation
recipe is recorded too — `.register(` greps as badly as `.off(`.
- **`docs(knip)`** — "the build, the tests and this gate all stay green
on a wrong delete" held for argent-private and not for the
cross-workspace case that justifies three of the four `@public` tags.
Deleting `ArtifactStore.register` gives 16 `TS2339` errors and `9 failed
| 2 passed` in `test/artifacts.test.ts`; only the gate stays green.
Split in the workflow comment, the failure annotation and
`CONTRIBUTING.md` step 5.
- **`perf(debugger)`** — `doRegister`'s `data:` branch still
base64-decoded and `JSON.parse`d a payload it dropped, inside a `catch`
that swallows. Both arms returned the same `void`, so the branch
collapses to an early return; `scriptUrl` / `scriptId` go with it, since
their last reader was the deleted `this.maps.push(...)` and `doRegister`
is private.
- **`test(debugger)`** — the allowlist check moved to a top-level early
return with nothing pinning it: delete the line and both SSRF files
still pass 17/17. Two tests now assert `doRegister` consults it — four
rejected URLs reach no `fetch`, and the loopback `*.map` Metro emits
still does.

Re-verified after: `npm run knip` exit 0 and empty on a `tsc --build
--clean` tree, `tsc --build`, `eslint . --max-warnings 0`, `prettier
--check .` all clean, and `npm test --workspaces` with tool-server at
329 files / 3854 passed. The one red test, `argent-installer`'s
`globalPath returns ~/.config/opencode/opencode.json`, fails identically
with these commits stashed — it reads the real `~/.config/opencode`,
which holds a `.jsonc` on this machine.

End-to-end on a tool-server run from this branch over its HTTP API, on
every platform:

- **Chromium** — `launch-app`, `describe`, `screenshot` (returned an
artifact handle, `GET /artifacts/<id>` served the 137542-byte PNG back),
`gesture-tap` (counter 0 → 1), `debugger-connect` / `debugger-status`
(`sourceMapReady: true`) / `debugger-evaluate` (read `taps: 1` out of
the page).
- **iOS** — `launch-app`, `describe` via native-devtools, `restart-app`,
`screenshot`.
- **tvOS** — `launch-app`, focus-driven `describe` (12 focusables in
TVSettings), `tv-remote`, `restart-app`.
- **Android** — `launch-app`, `describe` via android-devtools,
`restart-app`, `screenshot`.
- **Vega** — `boot-device` on the `tv` VVD, `describe` via the
automation toolkit.
- **Metro** — the real `debugger-connect` / `debugger-status` /
`debugger-evaluate` tools against a stand-in Metro (HTTP `/json/list` +
CDP WebSocket) emitting three `Debugger.scriptParsed` events: a loopback
`*.map` on a counting server, a 1.4 MB inline `data:` map, and
`http://169.254.169.254/latest.map`. `sourceMapReady: true`,
`loadedScripts: 3`, evaluate 42, the loopback map fetched exactly once
and the metadata URL never. A malformed inline payload gives
byte-identical output, which is the finding.

### Merge with `main`

Putting the base back on `main` made CI resolve the merge, and the Unit
Tests job went red on a conflict no side could see on its own: `#771`
fixed the same red gate this branch's first commit did, the other way
round — it made `component-names.test.ts` import `StrippedName`,
`ComponentAnnotation` and `ComponentNameResolution` instead of dropping
their `export`. Merged `main` in, reproduced it (`tsc --noEmit -p
tsconfig.test.json` → three `TS2459`s) and took main's resolution: the
three exports are back. The gate stays green because knip treats test
files as entry points, so a type a test imports is not an unused export.

All nine workflows are green on the head commit.
2026-08-25 17:27:12 +02:00

182 lines
10 KiB
JSON

{
// Anti-dead-code gate: unused files, exports and types nothing imports, and
// dependencies declared but never used (plus the inverse). Run it with
// `npm run knip`; .github/workflows/knip.yml enforces it.
//
// Every issue type knip reports is an error by default, so this file sets no
// `rules`. There is no parked backlog either: `--max-issues 0` means both
// passes must come back empty, and one new unused export fails the job.
//
// Empty is not the same as exhaustive. Two shapes get past both passes, and
// it is the invocation below that hides them, not knip. A dead member of a
// referenced `enum` needs `enumMembers` and `nsTypes` in the SAME run
// (`knip/dist/graph/analyze.js:72`); pass 1 has the first without the second,
// pass 2 the reverse. A dead specifier on a same-workspace `export … from`
// line is unreportable in pass 2, because naming `classMembers` turns
// `isSkipLibs` off and that enables the `hasExternalReferences` guard, for
// which the declaration in the re-exported module is itself a reference; pass
// 1 excludes `exports` and `types` outright. Both measured on a dist-free
// tree: a dead `ServiceState` member, and a dead `Cookie` type plus a dead
// `createTabsManager` value re-export, each leave `npm run knip` at exit 0,
// while a plain dead `export const` in the same tree fails it. So treat a
// green run as "the shape almost everyone writes is clean", not as proof the
// symbol you just orphaned still has a reader.
//
// `npm run knip` still runs knip twice because the two flags partition the
// issue types rather than layering. `--include` is restrictive, so the pass
// that names `classMembers`, `nsExports` and `nsTypes` — which it must, since
// they sit in knip's own `defaultExcludedIssueTypes` and are otherwise gated
// by neither pass — reports nothing else, and the first pass covers the rest.
// Naming `classMembers` also turns knip's `isSkipLibs` off, which moves the
// export and type counts, so a run that names it is not comparable to one
// that does not.
//
// Run it the way CI does, against a tree with no build output: whether a
// workspace resolves `main`/`types` to a file that exists changes what knip
// reaches, so a run after a local `tsc --build` is no evidence the job will
// pass.
"$schema": "https://unpkg.com/knip@5/schema.json",
// Makes a stale exemption here fail the job instead of printing a hint below
// the backlog — an `ignoreDependencies` name that suppresses nothing, an
// `entry` that matches nothing or is already reached, an `ignoreWorkspaces`
// path that does not exist. Mind that last one: if the submodule directory
// below goes missing the hint fires advising you to drop the entry, which is
// the one thing not to do. Restore the directory instead.
"treatConfigHintsAsErrors": true,
// Ships from its own repo with its own CI, and eslint and prettier skip it the
// same way. It becomes a workspace the moment `git submodule update --init`
// puts a package.json where the root `packages/*` glob matches it.
"ignoreWorkspaces": ["packages/argent-private"],
"workspaces": {
// Knip finds the root scripts on its own — via package.json `scripts`, the
// scripts/*.test.mjs files those run, and `run:` steps in .github/workflows.
// What its github-actions plugin cannot parse is a PowerShell body, so the
// Windows E2E driver, invoked from the windows-latest job where that is the
// default shell, is the one entry declared by hand.
//
// Knip analyzes JS and TS only, so the shell half of scripts/ — the whole
// scripts/e2e-full/ harness included — is outside the gate however it runs.
".": {
"entry": ["scripts/ci/windows-chromium-e2e.mjs"],
},
// @swmansion/argent is a thin dispatcher whose real payload is an esbuild
// bundle of the OTHER workspaces (see this workspace's
// scripts/bundle-tools.cjs); the hand-written dispatcher copied into bin/ at
// build time is an entry, not dead code.
//
// Knip cannot follow imports across that esbuild boundary, so a package
// consumed only by the bundled-in code reads as unused here. They are named
// one by one rather than waved through with a catch-all pattern:
// `ignoreDependencies` also suppresses `unlisted`, so a catch-all would take
// this workspace's own src/ and scripts/ out of that check — where an import
// with no matching declaration is a real bug, not a bundler artifact.
"packages/argent": {
"entry": ["scripts/argent-simulator-server.cjs"],
"ignoreDependencies": [
// dependencies
"@fails-components/webtransport",
// Not bundled-in code reaching it: @fails-components/webtransport
// imports this itself at runtime, so nothing first-party imports it.
"@fails-components/webtransport-transport-http3-quiche",
"@modelcontextprotocol/sdk",
"tree-sitter",
"tree-sitter-typescript",
// devDependencies
"@clack/prompts",
"@opentelemetry/api-logs",
"@opentelemetry/exporter-logs-otlp-http",
"@opentelemetry/resources",
"@opentelemetry/sdk-logs",
"picocolors",
"semver",
"smol-toml",
"yaml",
],
},
// Three members in `packages/registry` are reported unused even though
// tool-server calls all three. The cause is the unbuilt tree, the same one
// the note at the top of this file describes: every workspace resolves
// `main`/`types` to a `dist/` that is not there, so `@argent/registry`
// resolves to nothing and the edge from tool-server never forms. Build the
// tree first and all three findings go away on their own — which is why the
// exemption is needed at all, since CI never builds.
//
// Nothing about this is specific to class members. Every cross-workspace
// reference is invisible the same way; the other issue types simply have no
// finding to raise here. They are exempted at the declaration with a
// `@public` JSDoc tag rather than by name here, for one reason: scope.
// `ignoreMembers` matches a NAME across the whole workspace, so it would
// also hide a future dead `register`/`list`/`off` on any other class in
// `packages/registry`. `@public` binds to the one member it is written on.
//
// What it does NOT buy is a stale check. `treatConfigHintsAsErrors` cannot
// report either form: knip emits no hint for an `ignoreMembers` name that
// suppresses nothing, and it emits a tag hint only for tags in the exclude
// half of the `tags` option, which this repo does not set. Both were
// measured — a bogus `ignoreMembers` name and a redundant `@public` each
// leave the run at exit 0, while a bogus `ignoreDependencies` name fails it.
// So when one of these three stops being earned, nothing says so, and the
// JSDoc at each member is the only record a re-audit has.
//
// tool-server reaches a few packages in ways knip can't trace statically.
// `ignoreDependencies` works in both directions, so this one list silences
// two different issue classes; they are kept apart below so each entry's
// stated reason matches the finding it actually suppresses. Each entry
// exempts the NAME across the whole workspace, not the call site it
// documents — while `sharp` is listed, an undeclared import of it from any
// file here passes too — so the list holds only packages this workspace
// genuinely cannot declare.
"packages/tool-server": {
"ignoreDependencies": [
// --- Declared in package.json, loaded too dynamically to detect.
// Without these: "unused dependencies". ---
// Dynamic `_require("tree-sitter")` / `_require("tree-sitter-typescript")`
// in react-profiler/pipeline/.../ast-index.ts, behind a one-shot flag.
"tree-sitter",
"tree-sitter-typescript",
// Optional peers of @swmansion/argent-cloud-sdk, which leaves them to
// the host to declare. utils/moq-client.ts calls the SDK's
// `installNodeWebTransport()`, but the dynamic import of the polyfill
// sits inside the SDK's own /node entry, so no first-party file names
// it; the transport is a second hop, imported by the polyfill itself.
"@fails-components/webtransport",
"@fails-components/webtransport-transport-http3-quiche",
// --- Imported here but deliberately absent from this package.json.
// Without these: "unlisted dependencies". ---
// Lazy `require("sharp")` in chromium-server/screenshot.ts. Kept out of
// the manifest on purpose so the ~30 MB native binary isn't a cost every
// consumer pays; the caller degrades gracefully when it isn't installed.
"sharp",
// `require("electron")` twice, for two different reasons. In
// utils/preview-window.ts it resolves the Electron executable's path
// from the install root, where it is an optionalDependency of the
// published @swmansion/argent. In test/fixtures/electron-smoke-app it
// is the Electron main-process API; that fixture declares electron in
// its own package.json and CI installs it there, but knip attributes
// the file to this workspace's manifest and never reads the nested one.
"electron",
],
},
},
// Blind spot in the `files` check: test files are entry points, so a
// production module that nothing but its own test imports still counts as
// reached — which in this repo is most of them. What `files` reliably catches
// is a module with no test at all. A file orphaned inside test/ is invisible
// for the same reason, knip's node plugin registering those trees as entries.
// `knip --production` is the lens that reports the whole class; the repo is
// not clean under it yet, so expect findings there that this gate does not
// raise.
//
// Reachability has a wider hole still: whatever a workspace's entry re-exports
// is reached, so one `export { x } from "./dead-module";` in a src/index.ts
// takes that module out of `files` altogether. A clean run means nothing knip
// can see is dead — not that nothing is.
}