[#1494] : [Hook] onRequest server hook parameters are typed as any (Bun 1.3.0)
#1495
Conversation
WalkthroughThis PR bumps the @types/bun dependency to v1.3.0 and updates the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The changes span type system adjustments that affect public API exports. While the modifications are localized and repetitive (applying generic parameterization), they require careful verification of TypeScript type inference behavior and potential downstream impacts on consumers relying on these type definitions. Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
#1494 I tried to explain in detail in the issue 🥺 |
Komiriko
changed the title
anyany (Bun 1.3.0)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
package.json(1 hunks)src/universal/server.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/universal/server.ts (1)
src/types.ts (1)
IsAny(842-842)
🔇 Additional comments (2)
package.json (1)
199-199: Version bump aligns with Bun 1.3 typings.Upgrade looks good and matches the type changes in src/universal/server.ts. Please run the existing type test suite to confirm no regressions in downstream adapters. Based on learnings.
src/universal/server.ts (1)
125-126: Switch to Bun generics with unknown — LGTM and solves the any leak.Using BunServe.Options and BunServer (with IsAny fallback) tightens types and should fix onRequest params being any while staying backward compatible. Based on learnings.
To lock this in, add a tiny type test (using expect-type already in devDeps):
// test/type-system/on-request-server.d.ts import type { Server } from '../../src/universal/server' import { expectType } from 'expect-type' declare function onRequest(server: Server): void // Ensure Server is not any type IsAny<T> = 0 extends 1 & T ? true : false expectType<false>({} as IsAny<Server>)Run: bun run test:types
| @@ -1,4 +1,4 @@ | |||
| import type { Serve as BunServe, Server as BunServer } from 'bun' | |||
| import { Serve as BunServe, type Server as BunServer } from 'bun' | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Avoid runtime import of "bun" in universal types by using type-only module import.
Importing Serve as a value can survive emit in some setups (e.g., verbatimModuleSyntax) and accidentally pull "bun" at runtime in non-Bun targets. Prefer a pure type import for safety.
Apply:
-import { Serve as BunServe, type Server as BunServer } from 'bun'
+import type * as BunTypes from 'bun'And update aliases:
-export type Serve = IsAny<BunServe.Options<unknown>> extends false ? BunServe.Options<unknown> : ServeOptions
-export type Server = IsAny<BunServer<unknown>> extends false ? BunServer<unknown> : ServerOptions
+export type Serve = IsAny<BunTypes.Serve.Options<unknown>> extends false ? BunTypes.Serve.Options<unknown> : ServeOptions
+export type Server = IsAny<BunTypes.Server<unknown>> extends false ? BunTypes.Server<unknown> : ServerOptionsAlso applies to: 125-126
🏁 Script executed:
#!/bin/bash
# Verify we don't emit runtime imports of "bun" in built artifacts
# (run after your normal build)
rg -nP 'from\s+[\'"]bun[\'"]' -g 'dist/**/universal/*.js' -S || echo "OK: no runtime bun imports found in dist/universal"Length of output: 68
---
🏁 Script executed:
```shell
#!/bin/bash
# Examine the source file to understand current import structure
echo "=== Examining src/universal/server.ts ==="
head -20 src/universal/server.ts
echo ""
echo "=== Checking lines around 125-126 ==="
sed -n '120,130p' src/universal/server.ts
Length of output: 944
🏁 Script executed:
#!/bin/bash
# Check if dist/universal exists and search for bun imports using alternative escaping
if [ -d dist/universal ]; then
echo "=== Checking for bun imports in dist/universal ==="
rg -n 'from.*bun' dist/universal/ --type js 2>/dev/null || echo "No matches found or directory structure differs"
else
echo "dist/universal not found - build artifacts may not exist"
find . -name "*.js" -path "*/dist/*" -type f 2>/dev/null | head -5 || echo "No dist files found"
fiLength of output: 1140
🏁 Script executed:
#!/bin/bash
# Verify the overall structure and search for any bun-related imports in the source
echo "=== Source file import section ==="
rg -n 'import.*bun' src/universal/ --type ts 2>/dev/null || echo "No matches found"Length of output: 246
🏁 Script executed:
#!/bin/bash
# Check all usages of BunServe and BunServer in the file
echo "=== All usages of BunServe ==="
rg -n 'BunServe' src/universal/server.ts
echo ""
echo "=== All usages of BunServer ==="
rg -n 'BunServer' src/universal/server.tsLength of output: 517
Fix value import of "Serve" to prevent runtime "bun" imports in universal code.
The Serve import on line 1 is imported as a value but only used in type positions (lines 125–126). With compiler options like verbatimModuleSyntax, this will emit a runtime import of "bun" in the compiled output, breaking compatibility in non-Bun environments.
Apply:
-import { Serve as BunServe, type Server as BunServer } from 'bun'
+import type * as BunTypes from 'bun'Then update lines 125–126:
-export type Serve = IsAny<BunServe.Options<unknown>> extends false ? BunServe.Options<unknown> : ServeOptions
-export type Server = IsAny<BunServer<unknown>> extends false ? BunServer<unknown> : ServerOptions
+export type Serve = IsAny<BunTypes.Serve.Options<unknown>> extends false ? BunTypes.Serve.Options<unknown> : ServeOptions
+export type Server = IsAny<BunTypes.Server<unknown>> extends false ? BunTypes.Server<unknown> : ServerOptionsCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/universal/server.ts around line 1 and lines 125–126, the import currently
brings Serve as a runtime value which causes a runtime import of "bun"; change
the import to a type-only import (import type { Serve as BunServe, Server as
BunServer } from 'bun') and update the usages at lines 125–126 to be type
annotations only (remove any runtime references to BunServe/BunServer so they
are used purely as types).
1 participant
Summary by CodeRabbit
Chores
Refactor