feat(hono): support async locale detector #35
Conversation
WalkthroughAdds async locale-detector support to Hono: makes useTranslation asynchronous, wires a detector into Hono context (i18nLocaleDetector), enables on-demand and parallel message loading, updates examples/playgrounds/docs, adjusts tests, and updates some package dependency entries to catalog placeholders. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Middleware
participant Detector as Locale Detector
participant Loader as Resource Loader
participant Route as Route Handler
participant Translator as useTranslation
Note over Client,Translator: Async locale detection + on-demand loading flow
Client->>Middleware: HTTP request
Middleware->>Detector: detectLocale(ctx) (async)
Detector->>Loader: load messages if missing (async)
Loader-->>Detector: messages
Detector-->>Middleware: detected locale
Middleware->>Middleware: set ctx.i18nLocaleDetector
Middleware-->>Client: continue to route
Client->>Route: invoke route handler (async)
Route->>Translator: await useTranslation(ctx)
Translator->>Detector: call detector from ctx (once)
Detector-->>Translator: locale
Translator->>Translator: set i18n.locale, parse args
Translator-->>Route: translation function
Route-->>Client: response with translated text
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🔇 Additional comments (7)
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: 3
📜 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 (14)
packages/h3/package.json(1 hunks)packages/h3/spec/integration.spec.ts(3 hunks)packages/hono/README.md(4 hunks)packages/hono/package.json(1 hunks)packages/hono/playground/basic/index.ts(1 hunks)packages/hono/playground/global-schema/index.ts(1 hunks)packages/hono/playground/local-schema/index.ts(1 hunks)packages/hono/spec/fixtures/en.json(1 hunks)packages/hono/spec/fixtures/ja.json(1 hunks)packages/hono/spec/integration.spec.ts(3 hunks)packages/hono/src/index.test-d.ts(1 hunks)packages/hono/src/index.test.ts(3 hunks)packages/hono/src/index.ts(5 hunks)pnpm-workspace.yaml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/hono/src/index.ts (1)
packages/h3/src/index.ts (1)
detectLocaleFromAcceptLanguageHeader(244-245)
packages/hono/spec/integration.spec.ts (1)
packages/hono/src/index.ts (3)
defineI18nMiddleware(126-167)CoreContext(36-36)DefineLocaleMessage(89-89)
🔇 Additional comments (23)
packages/h3/spec/integration.spec.ts (1)
56-56: LGTM! Improved test titles.The test titles are now more descriptive and clearly communicate what each test validates: basic detection, async loading, and parallel async loading scenarios.
Also applies to: 91-91, 154-154
packages/hono/playground/local-schema/index.ts (1)
21-27: LGTM! Correctly implements async translation pattern.The route handler has been properly updated to async/await the
useTranslationcall, aligning with the PR's objective to support async locale detection.packages/hono/playground/basic/index.ts (1)
23-24: LGTM! Consistent async implementation.The async/await pattern is correctly applied, consistent with other playground examples in this PR.
packages/hono/playground/global-schema/index.ts (1)
30-31: LGTM! Async pattern with global schema.The async/await pattern is correctly applied while maintaining the global schema type safety approach.
packages/hono/README.md (3)
104-108: LGTM! Documentation updated for async API.The basic usage example correctly demonstrates the new async/await pattern for
useTranslation.
151-188: Excellent documentation for async locale detection.This new section provides a comprehensive and practical example demonstrating:
- Async locale detector signature with CoreContext parameter
- Resource lazy loading pattern
- Proper type imports and usage
This addresses the core feature of the PR and will help users understand how to implement async locale detection.
285-293: LGTM! All examples consistently updated.The type-safe resources and global schema examples have been properly updated to use the async/await pattern, maintaining consistency throughout the documentation.
Also applies to: 321-326
packages/hono/spec/fixtures/ja.json (1)
1-3: LGTM! Valid translation fixture.The Japanese translation fixture is correctly formatted and will support async loading test scenarios.
packages/h3/package.json (1)
66-66: Catalog configuration is properly defined.The verification confirms that
@intlify/coreis correctly defined in the pnpm workspace catalog with version^11.1.12. The change to use"@intlify/core": "catalog:"in the package.json is valid and consistent with the catalog configuration.packages/hono/src/index.test-d.ts (1)
19-19: No issues found. This is a deprecation update.
toMatchTypeOfhas been deprecated since expect-type v1.2.0 and the recommended replacement istoExtend..toExtendis identical to.toMatchTypeOf, so this change is a straightforward API update with no change to the actual type relationship being tested. The middleware type continues to extendMiddlewareHandleras before.packages/hono/spec/fixtures/en.json (1)
1-3: LGTM!The fixture file is correctly formatted and provides the necessary test data for English locale translations.
packages/hono/package.json (1)
65-65: LGTM!The switch to catalog-based dependency resolution is consistent with the monorepo approach and aligns with the workspace catalog entry.
packages/hono/src/index.test.ts (2)
43-79: LGTM!The test has been correctly updated to handle async translation:
- Test is now async and awaits
useTranslation- Mock context properly provides both
i18nandi18nLocaleDetector- Locale detector binding logic is correct
81-94: LGTM!The error handling test is correctly updated to async pattern with
rejects.toThrowError.packages/hono/spec/integration.spec.ts (2)
34-36: LGTM!The route handler correctly uses async/await pattern with
useTranslation.
48-80: LGTM!The basic detection test correctly demonstrates synchronous locale detection with the new async translation API.
packages/hono/src/index.ts (6)
12-17: Note: External typo in imported constant.Line 15 imports
NOT_REOSLVEDwhich appears to be a typo in the@intlify/corelibrary (should beNOT_RESOLVED). Since this is an internal function from the external library, there's nothing to fix here, but it's worth noting.
58-58: LGTM!The addition of
i18nLocaleDetectorto the context variable map correctly exposes the locale detector for use inuseTranslation.
147-153: LGTM!The updated
getLocaleDetectorcorrectly binds bothctxandi18nparameters to the locale detector function, enabling access to the i18n context for async locale loading scenarios.
155-166: LGTM!The middleware correctly:
- Creates and stores the locale detector in context
- Sets the detector as a function on
i18n.localefor later evaluation- Cleans up context on exit
347-367: LGTM!The async implementation correctly:
- Returns a Promise for the translation function
- Retrieves the locale detector from context with proper error handling
- Awaits the detector call, which handles both sync and async detectors transparently
- Sets the resolved locale on the i18n context
369-384: Verify _translate signature from @intlify/core to confirm arg2 duplication pattern.The pattern extracts
arg2directly from the originalargsarray while also passing parsedoptionsfromparseTranslateArgs. This creates potential duplication—ifarg2is a number (plural), string (default), or object (named), it may be passed twice: as rawarg2and again within parsedoptions. Without access to the_translatefunction signature from@intlify/core, I cannot confirm whether this duplication is intentional or if it correctly handles all overloads (plural, defaultMsg, list, named).The pattern is consistently used in both
packages/hono/src/index.ts(line 370-371) andpackages/h3/src/index.ts(line 421-422), suggesting it may be the intended design, but manual verification against the external library's API is required.pnpm-workspace.yaml (1)
6-6: ---@intlify/core ^11.1.12 version is valid, but parseTranslateArgs is not an async locale-detection feature.
@intlify/core v11.1.12 exists, confirming the catalog entry is valid. However, parseTranslateArgs is a synchronous helper for parsing translation arguments and is not an async locale-detection mechanism. Async locale detection is provided by higher-level tooling (e.g., Nuxt/i18n's locale detector hooks) and those detector APIs are typically synchronous. If your implementation requires async locale detection, ensure it's handled via appropriate detector hooks or application-level async message loading, not through parseTranslateArgs.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/hono/spec/integration.spec.ts (3)
47-80: Good basic custom detector; consider also asserting the fallback pathThe custom detector correctly pulls
localefrom the query string and falls back todefaultLocaleon error. Currently the spec only exercises the happy path (/?locale=ja); if you want fuller coverage of the detector’s behavior, consider adding a second assertion (or a small extra test) for the fallback case whenlocaleis missing or invalid.
82-135: Async loading test may not actually exercise the dynamic loaderIn this spec you define a
messagesloader map and only load when!i18n.messages[locale], but you also pass bothenandjamessages intodefineI18nMiddleware, soi18n.messages[locale]is likely already populated and the loader branch may never run. If the intent is to validate async message loading as well as async locale detection, consider either:
- Seeding only the default locale (e.g., just
en) inmessagesand relying on the loader forja, or- Instrumenting the loader with
vi.fn()and asserting it was called.That would make the test’s name (“detect with async loading”) more tightly aligned with what it actually verifies.
137-194: Parallel async loading test looks solid; you can avoid the@ts-ignorewith a small rewriteThe parallel test nicely exercises concurrent async detection and dynamic loading across
en/jarequests. To keep types clean and remove the@ts-ignore, you can rewrite thePromise.allas anasyncmapper:- const resList = await Promise.all( - ['en', 'ja'].map(locale => - app - .request(`/?locale=${locale}`) - // @ts-ignore - .then(res => res.json()) - ) - ) + const resList = await Promise.all( + ['en', 'ja'].map(async locale => { + const res = await app.request(`/?locale=${locale}`) + return res.json() + }) + )Behavior stays the same, but the types line up without suppressing the checker.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/hono/spec/integration.spec.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/hono/spec/integration.spec.ts (1)
packages/hono/src/index.ts (3)
defineI18nMiddleware(126-167)CoreContext(36-36)DefineLocaleMessage(89-89)
🔇 Additional comments (2)
packages/hono/spec/integration.spec.ts (2)
2-12: Imports updated correctly for async detection testsThe added vitest helpers and CoreContext / DefineLocaleMessage types match how they’re used later in the async locale detector specs; nothing concerning here.
34-36: AsyncuseTranslationusage is consistent with the new APISwitching the route handler to
asyncand awaitinguseTranslation(c)before callingt('hello', …)aligns with an async translation acquisition model and keeps the test behavior clear.
Description
Linked Issues
resolve #28
Additional context
Summary by CodeRabbit
New Features
Documentation
Tests
Chores