Lint the RedisClient class in redis.d.ts against the valkey.classes.ts tables - #39271
Conversation
…s tables packages/bun-types/redis.d.ts mirrors the proto table in src/runtime/valkey_jsc/valkey.classes.ts by hand and nothing compared the two; psubscribe, pubsub, punsubscribe, script and select have been registered since the first Bun.redis commit and are still undeclared. The new source lint imports valkey.classes.ts, collects the members the codegen installs (proto, klass, constructor), parses the members declared in the class body of redis.d.ts, and requires the two sets to be equal in both directions. The five names currently missing are listed in a pending table keyed by the PR that declares each; the lint also fails once a listed name is declared or unregistered, so the table cannot go stale. source-lints.yml now triggers on *.classes.ts, class-definitions.ts and redis.d.ts, the files this lint reads.
|
Updated 7:37 PM PT - Aug 15th, 2026
✅ @robobun, your commit e1843f25d86a9f3418fe1323760008464c00542a passed in 🧪 To try this PR locally: bunx bun-pr 39271That installs a local version of the PR into your bun-39271 --bun |
|
Status: merged as f0f6b2c. Reproduced on main by comparing the The five declarations themselves are owned by #39208 ( |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan includes up to 5 reviews per rolling hour; 0 remain after this review. WalkthroughChangesRedisClient synchronization
Suggested reviewers: Merge Risk: ⚪ Minimal · up to This PR adds a focused lint that keeps RedisClient type declarations aligned with the runtime command table and updates the checks that run it. No actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — test-infrastructure only, no runtime code changes.
What was reviewed:
- Ran the new lint locally against the checked-out tree: 4 pass;
class-definitions.tshas no imports so the bare-checkout constraint holds. - Checked
redis.d.tsindentation and the\\n }\\nterminator: lines 43/51 precede the class open at 53, soindexOfafterbodyStartlands on 3309 (the class close). define()defaultsklass = {}, so iteratingdefinition.klasson RedisClient (which sets none) is a no-op, not a crash.- Workflow path additions mirror across
push/pull_requestand cover exactly the files the lint reads/imports.
Extended reasoning...
Overview
Adds a source-lint test (test/internal/source-lints/redis-client-types.test.ts) that imports valkey.classes.ts, collects the members the codegen would install on RedisClient.prototype, parses the member names from the class RedisClient body in packages/bun-types/redis.d.ts, and asserts the two sets match. The five currently-undeclared commands are held in a pendingDeclarations table with a self-pruning test. The remaining hunks are a two-line pointer comment in valkey.classes.ts, three workflow paths: triggers (mirrored push/PR), and a README paragraph documenting the trigger-path rule.
Security risks
None. Reads two repo files, imports a codegen-input module with zero external deps, no network, no subprocess, no writes.
Level of scrutiny
Low-to-medium. No shipped code changes — the valkey.classes.ts hunk is a comment above proto:. The interesting part is the d.ts body parser: it anchors on the exact two-space-indented export class RedisClient { line, slices to the next \\n }\\n, strips comments, and matches four-space-indented lines whose first non-space char isn't a continuation closer. I checked the actual file: the only two-space } lines before the class are at 43/51 (before bodyStart), so the terminator search finds the class close at 3309. The test's own "unrecognized shape" assertion fails loudly if a future d.ts edit introduces a member line the regex can't read, so silent drift is prevented rather than assumed away.
Other factors
- Ran
bun test test/internal/source-lints/redis-client-types.test.tson this checkout: 4 pass, 191ms. Fits the directory's speed budget and its no-install constraint (class-definitions.tshas no imports;valkey.classes.tsimports only that). define()defaultsklass = {}, so the[definition.klass, "static "]loop entry is safe on RedisClient which has noklassblock.- The
pendingDeclarationstable introduces intentional cross-PR coupling with #39208/#29339/#35521; the failure message names the entry to delete, which is the standard suppression-list pattern already used elsewhere in this directory. - Follows the established
test/internal/source-lints/conventions (relative imports,toEqual([])on sorted diffs so the offending names print).
|
This is good but honestly we should find a much proper solution for this in the future with some actual generator between the two. I bet there are so many other problems we could be fixing to have these match up |
|
Agreed. To put numbers on the second half, I ran the same comparison by exact name over all 30
On a generator: the tables only know names, arity and getter/setter-ness; every signature, overload and doc comment exists only in the d.ts (the codegen already emits the untyped skeleton as |
The lint added in #39271 lists script() as declared by this PR, so the entry goes away here.
2 participants
Problem
class RedisClientinpackages/bun-types/redis.d.tsis a hand-written copy of theprototable insrc/runtime/valkey_jsc/valkey.classes.ts, and nothing compares the two. A command added to the table and not to the d.ts works at runtime and fails to type-check (Property 'pubsub' does not exist on type 'RedisClient').psubscribe,pubsub,punsubscribe,script,select. All five have been registered since the commit that introducedBun.redis(ec87a27, IntroduceBun.redis- a builtin Redis client for Bun #18812) and stayed undeclared through the 14 redis.d.ts commits since, including the 614-line batch in Many more Redis commands #23116, so the gap is systematic rather than a one-off.pubsubandselectby types: declare RedisClient.pubsub() and RedisClient.select() #39208,scriptby redis: add bitmap, HLL, geo, scripting, server, and stream commands #29339,psubscribe/punsubscribeby redis: implement psubscribe/punsubscribe with listener routing #35521 (which also adds the listener routing they are missing; todaypsubscribe()drops every pmessage). This PR adds the check, not the declarations.Fix
test/internal/source-lints/redis-client-types.test.tsimportsvalkey.classes.ts(the module the codegen reads;class-definitions.tshas no dependencies), collects the members the codegen installs (protoentries by name,klassentries asstatic name,constructorwhenconstructis set, skippinginternal/privateSymbol/publicSymbolentries, which the codegen does not install under an identifier), parses the member names declared in theclass RedisClientbody ofredis.d.ts([Symbol.x]maps onto the table's@@xspelling), and requires the two sets to be equal. Each direction is its own test, so a table entry without a declaration and a declaration without a table entry are both reported by name.pendingDeclarationstable keyed by the PR that declares each. A fourth test fails as soon as a listed name is declared or unregistered, so the entry gets deleted when its PR lands (whichever of this PR and types: declare RedisClient.pubsub() and RedisClient.select() #39208/redis: add bitmap, HLL, geo, scripting, server, and stream commands #29339/redis: implement psubscribe/punsubscribe with listener routing #35521 lands second trips it on rebase, and the message says which entry to delete). A name that is neither declared nor listed fails the lint outright..github/workflows/source-lints.ymlgainssrc/**/*.classes.ts,src/codegen/class-definitions.tsandpackages/bun-types/redis.d.tsas triggers: the workflow is path-filtered and those are the files this lint reads, so without them the edits it guards would not run it. The directory README now states that rule; a comment on theprototable points at the lint.*.classes.tsneeds a class-to-declaration mapping and inheritance handling and is a separate project.bun test test/internal/source-lints/redis-client-types.test.tspasses on this branch (4 tests); the whole directory is 170 green.pendingDeclarationsemptied, the lint fails against main's files naming exactlypsubscribe,pubsub,punsubscribe,script,select(output below).@@asyncDispose+ aklassentry with and without declarations,internal/privateSymbolentries); each fails the intended test or passes as intended.valkey.classes.tshunk is a comment and produces identical codegen output. This PR has no src/packages diff for a fail-before run to strip; the fail-before evidence is the emptied-pending-table run above.Background
*.classes.tsfiles are the input ofsrc/codegen/generate-classes.ts.define({ proto, klass, construct })describes a native class:protoentries become properties of the prototype (fnmethods,getter/setteraccessors),klassentries become statics, andconstruct: truemakes it newable. Entries withinternal,privateSymbolorpublicSymbolare installed under private names orSymbol.for()symbols (or not at all), and keys spelled@@xare installed under the well-known symbolSymbol.x.packages/bun-typesis the published@types/bunsurface; it is hand-written, not generated from the class definitions, which is why it can drift.test/internal/source-lints/holds tests that only read the source tree;.buildkite/ci.mjsexcludes the directory from the binary lanes andsource-lints.ymlruns it against a released bun on a bare checkout, so tests there may only import built-ins and relative paths.Lint output against main's files with the pending table emptied, and the simulated drift shapes