feat(hono): support hono v4 #33
Conversation
WalkthroughRemoved Miniflare environment/dev-deps and updated Hono to v4 while adopting srvx for local servers. Added srvx to workspace catalog, adjusted playgrounds with explicit Hono types and srvx server bootstraps, changed getQueryLocale to accept raw requests, made i18n optional on context, and added an e2e test suite. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as e2e Test
participant Proc as spawned tsx
participant Server as srvx Server
participant App as Hono App
participant Client as curl
Test->>Proc: spawn playground/index.ts
Proc->>Server: initialize srvx(port 3000)
Server->>App: instantiate Hono (const app: Hono)
App->>Server: provide fetch handler
Server->>Server: await ready
Test->>Test: short wait
Test->>Client: HTTP GET / (with Accept-Language or ?lang=)
Client->>Server: request arrives
Server->>App: App handles request, calls getHeaderLocale/getQueryLocale(req.raw)
App->>Client: responds with locale text
Client->>Test: response captured
Test->>Proc: terminate process
Proc->>Server: cleanup
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (4)
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 |
commit: |
![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
♻️ Duplicate comments (2)
packages/hono/spec/e2e.spec.ts (2)
42-48: Same hardcoded delay issue applies here.This test also uses a fixed 2-second delay instead of actively checking server readiness.
50-58: Same hardcoded delay issue applies here.This test also uses a fixed 2-second delay instead of actively checking server readiness.
🧹 Nitpick comments (4)
packages/hono/playground/util-query/index.ts (1)
12-15: Consider parameterizing the port to avoid conflicts.Port 3000 is hardcoded across multiple playground files (basic, util-header, util-query). If multiple playgrounds are run simultaneously during development, this will cause port conflicts.
Consider reading the port from an environment variable with a fallback:
const server = serve({ - port: 3000, + port: Number(process.env.PORT) || 3000, fetch: app.fetch })packages/hono/playground/util-header/index.ts (1)
12-15: Consider parameterizing the port to avoid conflicts.This playground also uses port 3000, creating a potential conflict with other playgrounds (basic, util-query). Consider using an environment variable or a unique default port for each playground.
packages/hono/playground/basic/index.ts (1)
28-30: Port 3000 is hardcoded across multiple playgrounds.This playground also uses port 3000, consistent with util-header and util-query. While this is fine for running one playground at a time, it creates conflicts when running multiple simultaneously.
packages/hono/src/index.ts (1)
152-152: Consider removing the unnecessary type cast.The
i18nvariable is already typed asCoreContextfrom line 130, so the explicit cast seems redundant. If TypeScript requires this due to the optional property inContextVariableMap, consider refining the type definitions instead.Apply this diff if the cast is truly unnecessary:
- ctx.set('i18n', i18n as CoreContext) + ctx.set('i18n', i18n)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
knip.config.ts(1 hunks)package.json(1 hunks)packages/h3/README.md(2 hunks)packages/h3/package.json(1 hunks)packages/hono/package.json(1 hunks)packages/hono/playground/basic/index.ts(2 hunks)packages/hono/playground/global-schema/index.ts(1 hunks)packages/hono/playground/local-schema/index.ts(1 hunks)packages/hono/playground/typesafe-schema/index.ts(2 hunks)packages/hono/playground/util-header/index.ts(1 hunks)packages/hono/playground/util-query/index.ts(1 hunks)packages/hono/spec/e2e.spec.ts(1 hunks)packages/hono/spec/integration.spec.ts(2 hunks)packages/hono/src/index.test.ts(0 hunks)packages/hono/src/index.ts(2 hunks)pnpm-workspace.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- packages/hono/src/index.test.ts
🧰 Additional context used
🧬 Code graph analysis (3)
packages/hono/playground/util-query/index.ts (1)
packages/hono/src/index.ts (1)
getQueryLocale(22-22)
packages/hono/playground/util-header/index.ts (1)
packages/hono/src/index.ts (1)
getHeaderLocale(19-19)
packages/hono/spec/integration.spec.ts (1)
packages/hono/src/index.ts (1)
getQueryLocale(22-22)
🔇 Additional comments (22)
packages/h3/README.md (2)
79-89: Documentation enhancement for translation usage is clear and helpful.The addition of introductory guidance (line 79) about installing the plugin and using
useTranslationimproves the README's clarity for new users. The code example provides a complete, working demonstration of the plugin pattern.
70-75: No action required - README examples are correct and consistent with API signatures.The @intlify/utils functions
getQueryLocaleandgetCookieLocaleboth accept aRequestparameter (the Node.js http.IncomingMessage). The README examples correctly call these withevent.req, which is the underlying Node.js request object provided by h3 Events. This pattern is consistent throughout the codebase: test files, playground examples, and all existing usage demonstrate the same correct approach. No API signature changes are present in this PR.Likely an incorrect or invalid review comment.
package.json (1)
75-75: LGTM! Clean dependency removal.The removal of
miniflareandvitest-environment-miniflarealigns well with the migration to srvx-based local development.packages/hono/playground/util-query/index.ts (1)
1-17: LGTM! Clean playground implementation.The structure is clear and follows a consistent pattern with other playground files. The use of
c.req.rawto pass the native Request object togetQueryLocalealigns with the API design.packages/hono/playground/util-header/index.ts (1)
1-17: LGTM! Consistent playground structure.The implementation follows the same clean pattern as util-query, using
c.req.rawto pass the native Request object togetHeaderLocale.packages/hono/spec/e2e.spec.ts (2)
7-21: LGTM! Well-structured command execution helper.The helper properly wraps exec with timeout and error handling, merging environment variables correctly.
23-29: Server cleanup looks good, but consider additional safeguards.The afterEach hook ensures the spawned process is killed after each test, which is essential for test isolation.
knip.config.ts (1)
14-14: LGTM! Knip configuration updated consistently.Removing
miniflareandvitest-environment-miniflarefrom the ignore list aligns with their removal from dependencies, enabling proper dependency checking.pnpm-workspace.yaml (1)
12-12: LGTM! Catalog entry enables consistent dependency resolution.Adding
srvxto the catalog allows packages to reference it viacatalog:, ensuring version consistency across the workspace.packages/hono/package.json (3)
72-72: LGTM! Catalog-based dependency resolution.Using
catalog:forsrvxensures consistent versioning across the workspace as defined in pnpm-workspace.yaml.
61-61: LGTM! Simplified play script.The script now uses plain
nodeto execute the TypeScript file, which works well with the srvx-based server setup.
70-70: No compatibility issues found with Hono v4.10.6 upgrade.The codebase uses only stable Hono v4 APIs. The main context method used is
c.json()(the correct v4 API), and there is no usage of any deprecated v3 APIs such asc.jsonT(),c.stream(),c.streamText(),c.env(), orapp.handleEvent(). The middleware implements standard patterns (app.use(),app.get(),Context,MiddlewareHandler) that are unaffected by the v3→v4 breaking changes. No code modifications are required.packages/hono/playground/basic/index.ts (3)
2-2: LGTM! Migration to srvx for local serving.The addition of
srvxreplaces the previous Wrangler-based development workflow, simplifying local development.
21-21: LGTM! Explicit type annotation improves clarity.The explicit
Honotype annotation makes the code more readable and helps catch type errors early.
28-33: LGTM! Clean server bootstrap pattern.The srvx server setup is straightforward and follows the same pattern used in the util-* playground files. The
await server.ready()ensures the server is initialized before the script completes.packages/h3/package.json (1)
73-73: LGTM! Centralizing dependency version management.Moving to catalog-based dependency resolution is a good practice for monorepo management, ensuring consistent versions across packages.
packages/hono/playground/global-schema/index.ts (1)
28-28: LGTM! Explicit typing improves clarity.The explicit type annotation makes the code more readable and intention clearer, even though TypeScript would infer the type automatically.
packages/hono/src/index.ts (1)
52-52: LGTM! Making i18n optional aligns with Hono v4.This change properly reflects that the i18n context variable is only available after the middleware has run. The
useTranslationfunction correctly handles the null case by throwing an informative error.packages/hono/playground/local-schema/index.ts (1)
19-19: LGTM! Consistent explicit typing.This change aligns with the pattern established across other playground files, improving code clarity.
packages/hono/playground/typesafe-schema/index.ts (2)
3-3: LGTM! Necessary import restored.The
defineI18nMiddlewareimport is required for the middleware setup on line 13.
23-23: LGTM! Consistent explicit typing.This aligns with the explicit typing pattern used across all playground files.
packages/hono/spec/integration.spec.ts (1)
51-51: API signature change verified and consistent across the codebase.The verification confirms that all usages of
getQueryLocalehave been correctly updated to the new signature:
- Hono usages pass
ctx.req.raw- H3 usages pass
event.req- No old-style calls remain
The test file change at line 51 is correct and aligns with all other usages in the codebase.
| test('util-header', async () => { | ||
| const target = path.resolve(import.meta.dirname, '../playground/util-header/index.ts') | ||
| serve = spawn('pnpx', ['tsx', target]) | ||
| await delay(2000) // wait for server to start | ||
| const stdout = await runCommand( | ||
| `curl -H 'Accept-Language: ja,en-US;q=0.7,en;q=0.3' http://localhost:3000` | ||
| ) | ||
| expect(stdout).toContain(`ja`) | ||
| }) |
There was a problem hiding this comment.
Hardcoded delay makes tests brittle.
The 2-second delay assumes the server will be ready, but this is not guaranteed on slower systems or under load. The test could fail intermittently or waste time waiting longer than necessary.
Consider polling the server endpoint until it responds or times out:
async function waitForServer(url: string, timeout = 10000): Promise<void> {
const start = Date.now()
while (Date.now() - start < timeout) {
try {
await fetch(url)
return
} catch {
await delay(100)
}
}
throw new Error(`Server at ${url} did not become ready within ${timeout}ms`)
}Then replace the delay with:
serve = spawn('pnpx', ['tsx', target])
-await delay(2000) // wait for server to start
+await waitForServer('http://localhost:3000')🤖 Prompt for AI Agents
In packages/hono/spec/e2e.spec.ts around lines 32–40, the test uses a hardcoded
2s delay which is brittle; replace the fixed delay with a polling helper that
repeatedly attempts the endpoint until it responds or a timeout elapses, then
call that helper (e.g., waitForServer('http://localhost:3000', timeout)) before
running the curl assertion; implement the helper to loop with short sleeps, try
a simple fetch/HTTP request each iteration, return when successful and throw on
timeout, and remove the fixed delay so the test proceeds as soon as the server
is ready.
Description
Linked Issues
Additional context
Summary by CodeRabbit
New Features
Tests
Bug Fixes
Documentation
Chores