Skip to content

refactor: make vestigial runner-client/app-log dynamic imports static#1214

Open
thymikee wants to merge 1 commit into
mainfrom
fix/ineffective-dynamic-imports
Open

refactor: make vestigial runner-client/app-log dynamic imports static#1214
thymikee wants to merge 1 commit into
mainfrom
fix/ineffective-dynamic-imports

Conversation

@thymikee

Copy link
Copy Markdown
Member

Problem

pnpm build warned:

[INEFFECTIVE_DYNAMIC_IMPORT] src/daemon/app-log.ts is dynamically imported by
src/daemon/request-platform-providers.ts but also statically imported by
session-observability.ts, session-teardown.ts — dynamic import will not move
module into another chunk.

Investigation

app-log.ts is await import()ed in request-platform-providers.ts but also
statically imported elsewhere, so the dynamic import splits nothing. Digging in,
this is a whole class of vestigial dynamic imports — rolldown only surfaces a
subset at a time, so fixing one reveals the next (runner-provider, then
runner-client).

Every flagged module is already eager in the daemon entry chunk
(internal/daemon.js) through static importers, so the await import() calls
defer nothing:

Module Why it's already eager Fix
app-log.ts static in session-observability / session-teardown static import
runner-provider.ts static via runner-client static import
runner-client.ts static in ~20 daemon modules (session lifecycle, screenshot, selectors, record-trace, teardown) static import

runner-client's await import()s lived in dispatch, dispatch-interactions,
transport, daemon-runtime, http-server (two of them re-importing on every
poll-loop iteration).

Why this is safe

  • No import cyclerunner-client's 97-module static closure never reaches
    dispatch/server, so static imports can't introduce an init-order/TDZ bug.
  • Byte-identical eager closures for every entry — the CLI stays at
    487,422 B with runner-client absent; only the daemon entry contains it,
    exactly as before.
  • Forcing runner-client into its own chunk was considered and rejected: it
    dragged the full ~97 KB into the CLI's eager closure by defeating tree-shaking
    of one small constant the CLI references.

Result

  • Build is warning-free (was 1 reported, ≥3 latent).
  • Behavior unchanged: these modules were already loaded at daemon start; the
    no-op dynamic imports are just removed.
  • pnpm typecheck, oxlint, and dispatch / runner / provider-scope /
    server-shutdown tests all pass.

Diff: 6 files, +20/−19.

`pnpm build` emitted INEFFECTIVE_DYNAMIC_IMPORT for `app-log.ts`: it is
`await import()`ed in `request-platform-providers.ts` but also statically
imported by `session-observability`/`session-teardown`, so the dynamic import
splits nothing. Investigating revealed a whole class of these — rolldown only
reports a subset at a time.

Every flagged module is already eager in the daemon entry chunk
(`internal/daemon.js`) via static importers, so the `await import()` calls defer
nothing:

- `app-log.ts`, `runner-provider.ts` — daemon-only, eager via
  session/teardown/runner-client. Made static in `request-platform-providers.ts`.
- `runner-client.ts` — a hub statically imported by ~20 daemon modules
  (session lifecycle, screenshot, selectors, record-trace, teardown), so it is
  baked into the daemon entry. The `await import()`s in dispatch / dispatch-
  interactions / transport / daemon-runtime / http-server (two of them looping
  per poll iteration) deferred nothing. Made static.

Verified safe:
- No import cycle (runner-client's 97-module static closure never reaches
  dispatch/server), so static imports can't introduce an init-order bug.
- Byte-identical eager closures for every entry — CLI stays 487,422 B with
  runner-client absent; only the daemon entry contains it, as before. (Forcing
  runner-client into its own chunk was rejected: it pulled the full ~97 KB into
  the CLI's eager closure by defeating tree-shaking of one small constant.)
- Build is now warning-free; typecheck, lint, and dispatch/runner/provider/
  server-shutdown tests pass.
@github-actions

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.7 MB 1.6 MB -44.2 kB
JS gzip 531.1 kB 515.3 kB -15.8 kB
npm tarball 640.9 kB 623.6 kB -17.2 kB
npm unpacked 2.3 MB 2.2 MB -54.7 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 29.1 ms 28.6 ms -0.5 ms
CLI --help 57.7 ms 57.4 ms -0.3 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/internal/daemon.js +74.9 kB +22.4 kB
dist/src/client.js +58.2 kB +18.1 kB
dist/src/android-input-ownership.js +17.7 kB +6.3 kB
dist/src/android.js +17.0 kB +5.9 kB
dist/src/debug-symbols.js +14.2 kB +5.3 kB

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant