Conversation
Bump typescript dependency from ^5.9.2 to ^6.0.0 across all JS packages (resolves to 6.0.2). No new type errors introduced by the upgrade. https://claude.ai/code/session_01XZPshGXakJHVJ1jpzZhfs7
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughTypeScript 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Remove unused @ts-expect-error for protocols in WebTransportOptions (now included in TS 6.0's built-in lib.dom.d.ts) - Add node-test.d.ts ambient declarations for node:test and node:assert to work with moduleResolution: "bundler" - Add bun.d.ts ambient declarations for bun:test module - Configure token/clock tsconfigs with types: ["node"] for full Node.js API support (node:fs, process, util) - Configure token tsconfig with module/moduleResolution: "nodenext" for node: protocol imports in cli.ts - Configure watch/publish tsconfigs with types: ["audioworklet"] for AudioWorklet globals - Add jsx: "preserve" and jsxImportSource: "solid-js" to demo tsconfig All packages now pass `tsc --noEmit` with zero errors. https://claude.ai/code/session_01XZPshGXakJHVJ1jpzZhfs7
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
js/common/node-test.d.ts (1)
1-11: Well-structured ambient declarations for node:test.Clear documentation of purpose, and the function signatures cover the common test patterns. One minor gap: Node's lifecycle hooks (
before,after,beforeEach,afterEach) also accept an optional name as the first parameter.🔧 Optional: Add overloads for named lifecycle hooks
export function before(fn: () => void | Promise<void>): void; + export function before(name: string, fn: () => void | Promise<void>): void; export function after(fn: () => void | Promise<void>): void; + export function after(name: string, fn: () => void | Promise<void>): void; export function beforeEach(fn: () => void | Promise<void>): void; + export function beforeEach(name: string, fn: () => void | Promise<void>): void; export function afterEach(fn: () => void | Promise<void>): void; + export function afterEach(name: string, fn: () => void | Promise<void>): void;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/common/node-test.d.ts` around lines 1 - 11, Add overloads for the lifecycle hooks so they accept an optional name: update the ambient declarations for the functions before, after, beforeEach, and afterEach to include an overload that takes (name: string, fn: () => void | Promise<void>) in addition to the existing (fn: ...) signature so callers can provide a descriptive name for the hook.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/common/bun.d.ts`:
- Line 11: Update the toThrow declaration in js/common/bun.d.ts: replace the
current narrow signature toThrow(expected?: string | RegExp | Error): void with
the broader Bun-compatible signature toThrow(expected?: unknown): void so it
accepts error constructors, error objects, strings, regexes and predicate
functions (i.e., match Bun 1.x). Locate the toThrow method in the d.ts and
change its parameter type to unknown.
---
Nitpick comments:
In `@js/common/node-test.d.ts`:
- Around line 1-11: Add overloads for the lifecycle hooks so they accept an
optional name: update the ambient declarations for the functions before, after,
beforeEach, and afterEach to include an overload that takes (name: string, fn:
() => void | Promise<void>) in addition to the existing (fn: ...) signature so
callers can provide a descriptive name for the hook.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1b12c658-5a7d-4ae0-b358-ff542c81f44d
📒 Files selected for processing (10)
js/clock/tsconfig.jsonjs/common/bun.d.tsjs/common/node-test.d.tsjs/demo/tsconfig.jsonjs/hang/tsconfig.jsonjs/lite/src/connection/connect.tsjs/lite/tsconfig.jsonjs/publish/tsconfig.jsonjs/token/tsconfig.jsonjs/watch/tsconfig.json
💤 Files with no reviewable changes (1)
- js/lite/src/connection/connect.ts
✅ Files skipped from review due to trivial changes (7)
- js/lite/tsconfig.json
- js/demo/tsconfig.json
- js/token/tsconfig.json
- js/watch/tsconfig.json
- js/publish/tsconfig.json
- js/clock/tsconfig.json
- js/hang/tsconfig.json
js/common/bun.d.ts
Outdated
| toBeLessThan(expected: number): void; | ||
| toBeGreaterThan(expected: number): void; | ||
| toBeCloseTo(expected: number, precision?: number): void; | ||
| toThrow(expected?: string | RegExp | Error): void; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In the official Bun 1.x bun:testTypeScript definitions, what is the exact signature ofexpect(...).toThrow(...), and which expected argument types are supported?
💡 Result:
In the official Bun 1.x bun:test TypeScript definitions, the signature of expect(...).toThrow(...) is toThrow(expected?: unknown): void. This is a method on the Matchers and MatchersBuiltin interfaces. The expected argument is optional (?) and of type unknown. When provided, it supports: - undefined or omitted: checks if anything throws - Error constructor: checks the class of the thrown Error - Error object: checks name and message properties - string or RegExp: checks the message property of the thrown error These behaviors are explicitly documented in the official Bun API reference.
Citations:
- 1: https://bun.com/reference/bun/test/Matchers/toThrow
- 2: https://bun.com/reference/bun/test/MatchersBuiltin
- 3: https://bun.com/reference/bun/test/MatchersBuiltin/toThrowError
- 4: https://bun.com/reference/bun/test/Expect
- 5: https://bun.com/reference/bun/test/Matchers
🏁 Script executed:
# Check if the file exists and examine the toThrow signature
if [ -f "js/common/bun.d.ts" ]; then
echo "=== Current content around line 11 ==="
sed -n '1,20p' js/common/bun.d.ts
echo ""
echo "=== Full file to understand context ==="
wc -l js/common/bun.d.ts
fiRepository: moq-dev/moq
Length of output: 1012
🏁 Script executed:
# Search for toThrow in the codebase to see if there are other definitions or usages
echo "=== Searching for toThrow definitions ==="
rg "toThrow" --type ts --type d.ts -B 2 -A 2Repository: moq-dev/moq
Length of output: 142
🏁 Script executed:
# Check bun version in package.json or similar files
echo "=== Checking project configuration ==="
find . -maxdepth 2 -name "package.json" -o -name "bunfig.toml" -o -name ".bun-version" 2>/dev/null | head -5 | xargs -I {} sh -c 'echo "File: {}"; cat "{}"'Repository: moq-dev/moq
Length of output: 1010
Update toThrow signature to accept error constructors and match Bun's type definition.
The current signature toThrow(expected?: string | RegExp | Error): void is too restrictive and rejects valid usage like toThrow(TypeError). Bun's official 1.x signature is toThrow(expected?: unknown): void, which supports error constructors, error objects, strings, regexes, and predicate functions. Update line 11 to match the official signature.
Suggested diff
- toThrow(expected?: string | RegExp | Error): void;
+ toThrow(expected?: unknown): void;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| toThrow(expected?: string | RegExp | Error): void; | |
| toThrow(expected?: unknown): void; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/common/bun.d.ts` at line 11, Update the toThrow declaration in
js/common/bun.d.ts: replace the current narrow signature toThrow(expected?:
string | RegExp | Error): void with the broader Bun-compatible signature
toThrow(expected?: unknown): void so it accepts error constructors, error
objects, strings, regexes and predicate functions (i.e., match Bun 1.x). Locate
the toThrow method in the d.ts and change its parameter type to unknown.
Use the installed @types/bun package instead of a hand-written bun.d.ts stub for bun:test types in js/watch. https://claude.ai/code/session_01XZPshGXakJHVJ1jpzZhfs7
Add biome-ignore comments for Function type usage (matches Node.js API) and fix formatting of single-line rejects overload. https://claude.ai/code/session_01XZPshGXakJHVJ1jpzZhfs7
…iles Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace node:test/node:assert with bun:test expect() in all test files - Delete custom node-test.d.ts, use bun-types for bun:test declarations - Remove explicit module/moduleResolution (defaults in TS 6.0) - Add "types": ["bun-types"] to base tsconfig - Cast expected values to branded types for strict bun-types generics Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
TS 6.0 defaults types to [] instead of auto-including @types/*, so each package with tests now declares @types/bun as a devDependency and "types": ["bun"] in its tsconfig. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
TS 6.0 requires rootDir to be explicitly set when outDir is specified. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Bump typescript dependency from ^5.9.2 to ^6.0.0 across all JS packages (resolves to 6.0.2). No new type errors introduced by the upgrade.
https://claude.ai/code/session_01XZPshGXakJHVJ1jpzZhfs7