Skip to content

chore(rivetkit): typecheck driver test fixtures - #5885

Draft
eersnington wants to merge 1 commit into
stack/fix-rivetkit-type-conn-state-from-createconnstate-urqpmypmfrom
stack/chore-rivetkit-typecheck-driver-test-fixtures-nxzslxvl
Draft

eersnington wants to merge 1 commit into
stack/fix-rivetkit-type-conn-state-from-createconnstate-urqpmypmfrom
stack/chore-rivetkit-typecheck-driver-test-fixtures-nxzslxvl

Conversation

@eersnington

@eersnington eersnington commented Oct 18, 2026 •

Copy link
Copy Markdown
Member

Typecheck the driver test fixtures in check-types, so type regressions in actor() show up in CI.

  • Add fixtures/**/*.ts to include in rivetkit-typescript/packages/rivetkit/tsconfig.json.

  • Add a rivetkit/unstable/migrations path alias for fixtures.

  • Fix the type errors that already exist in the fixtures:

    Fixture Fix
    actor-db-raw.ts Type the lifecycle helper client as () => Client<typeof registry>. Pass onReject to promiseWithResolvers
    conn-preflight-visibility.ts Read other connections' state as unknown, because preflight contexts type it that way
    telemetry.ts Use c.client<typeof registry>() in the workflow step
  • Seven fixtures still use // @ts-nocheck and stay unchecked.


This is part 2 of 2 in a stack:

@claude

claude Bot commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Review

This PR is small and looks good. It adds fixtures/**/*.ts to the check-types include list and fixes the type errors that exposed.

  • onReject callbacks: promiseWithResolvers requires onReject (src/utils.ts:210). The new warn-logging callbacks match the existing usage in sleep.ts and utils.ts, so the repo pattern is followed.
  • Path alias: src/unstable/migrations.ts exists, so the new rivetkit/unstable/migrations alias resolves.
  • telemetry.ts: replacing c.client<any>() with c.client<typeof registry>() is a real typing gain. The import type is erased at runtime, so it adds no circular import.
  • conn-preflight-visibility.ts: the as ConnState | undefined cast is unavoidable given the unknown state type. The comment explains why. The Map<unknown, ...> to Map<string, ...> change is a tighter and more accurate key type.

Nits (non-blocking):

  • actor-db-raw.ts: recordLifecycleEvent takes a hand-written structural context type. If the framework exports a context type, using it would avoid drift. This is fine as is.
  • The PR notes that seven fixtures still use // @ts-nocheck. A follow-up issue or todo to burn those down would be useful.

I couldn't confirm locally that check-types passes. CI should be the gate for that. No test, security or performance concerns.

This branch has not been deployed

No deployments
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