feat(h3,hono): add getDetectorLocale util
#43
Conversation
WalkthroughAdds async-capable locale detection helpers and a public Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App as H3/Hono App
participant Helper as getLocaleAndEventContext / getLocaleAndIntlifyContext
participant Detector as Locale Detector
participant Context as Event/Context Storage
User->>App: Request (with Accept-Language or ctx)
App->>Helper: getDetectorLocale(event/ctx)
activate Helper
Helper->>Context: Verify i18n initialized
alt Not initialized
Helper-->>App: Throw initialization error
else Initialized
Helper->>Context: Retrieve detector
Helper->>Detector: Invoke detector (sync or async)
Detector-->>Helper: Return detected locale
Helper->>Context: Set resolved locale on context
Helper-->>App: Return Intl.Locale
end
deactivate Helper
App-->>User: Response (may use locale for translations)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related issues
Possibly related PRs
Suggested labels
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)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/hono/README.md (2)
47-47: Fix incorrect import statement.The import statement references
@intlify/h3but this documentation is for the@intlify/honopackage. This will cause a module resolution error for users following this example.Apply this diff:
-import { getHeaderLocale } from '@intlify/h3' +import { getHeaderLocale } from '@intlify/hono'
62-62: Fix incorrect import statement.Same issue as above - the import references
@intlify/h3instead of@intlify/hono.Apply this diff:
-import { getQueryLocale } from '@intlify/h3' +import { getQueryLocale } from '@intlify/hono'
🧹 Nitpick comments (6)
packages/h3/docs/index.md (1)
21-21: Minor style inconsistency in description.The description "get a locale which is detected with locale detector." ends with a period, while most other function descriptions in the table do not (e.g., "get locale from cookie", "get language from header"). Consider removing the trailing period for consistency.
Apply this diff:
-| [getDetectorLocale](functions/getDetectorLocale.md) | get a locale which is detected with locale detector. | +| [getDetectorLocale](functions/getDetectorLocale.md) | get a locale which is detected with locale detector |packages/hono/docs/index.md (1)
16-16: Minor style inconsistency in description.The description "get a locale which is detected with locale detector." ends with a period, while most other function descriptions in the table do not (e.g., "get locale from cookie", "get language from header"). Consider removing the trailing period for consistency with the other entries.
Apply this diff:
-| [getDetectorLocale](functions/getDetectorLocale.md) | get a locale which is detected with locale detector. | +| [getDetectorLocale](functions/getDetectorLocale.md) | get a locale which is detected with locale detector |packages/h3/src/index.test.ts (1)
93-115: LGTM: Test coverage is comprehensive.The test properly exercises the
getDetectorLocalefunction with a realistic mock setup and verifies the returned locale.Note: There's significant setup duplication between this test and the
useTranslationtest (lines 47-72). Consider extracting a shared test helper function to create the mock event with bound locale detector. This would improve maintainability.Example refactor:
function createMockEventWithLocaleDetector(acceptLanguage: string) { const context = createCoreContext({ locale: detectLocaleFromAcceptLanguageHeader, messages: { en: { hello: 'hello, {name}' }, ja: { hello: 'こんにちは, {name}' } } }) const eventMock = { req: { headers: { get: _name => (_name === 'accept-language' ? acceptLanguage : '') } }, context: { [SYMBOL_I18N]: context as CoreContext } } as H3Event const _locale = context.locale as unknown const bindLocaleDetector = (_locale as LocaleDetector).bind(null, eventMock) // @ts-ignore ignore type error because this is test context.locale = bindLocaleDetector eventMock.context[SYMBOL_I18N_LOCALE] = bindLocaleDetector return { eventMock, context } }packages/hono/src/index.test.ts (1)
98-126: LGTM: Test coverage is comprehensive.The test properly exercises the
getDetectorLocalefunction with appropriate Hono context mocking and verifies the returned locale.Similar to the H3 tests, there's notable setup duplication between this test and the
useTranslationtest (lines 44-80). Consider extracting a shared helper function to create the mock context with the i18n setup. This would improve maintainability.Example refactor:
function createMockContextWithI18n(acceptLanguage: string) { const context = createCoreContext({ locale: detectLocaleFromAcceptLanguageHeader, messages: { en: { hello: 'hello, {name}' }, ja: { hello: 'こんにちは, {name}' } } }) const mockContext = { req: { raw: { headers: { get: _name => (_name === 'accept-language' ? acceptLanguage : '') } } }, get: (key: string) => { if (key === 'i18n') { return context } else if (key === 'i18nLocaleDetector') { const locale = context.locale as unknown return (locale as LocaleDetector).bind(null, mockContext) } } } as Context return mockContext }packages/hono/README.md (1)
120-120: Consider capitalizing "example" for consistency.Other section headings in this document capitalize "Example" (e.g., line 98's comment structure). Consider starting with a capital letter for consistency.
-example for detecting locale from url query, and get locale with `getDetectorLocale` util: +Example for detecting locale from url query, and get locale with `getDetectorLocale` util:packages/h3/src/index.ts (1)
345-346: Consider destructuring for slightly cleaner code (optional).The implementation stores the result then accesses
result[0]. You could destructure directly for slightly more concise code:- const result = await getLocaleAndEventContext(event) - return new Intl.Locale(result[0]) + const [locale] = await getLocaleAndEventContext(event) + return new Intl.Locale(locale)However, the current implementation is perfectly clear and may be preferred for explicitness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/h3/README.md(2 hunks)packages/h3/docs/functions/getDetectorLocale.md(1 hunks)packages/h3/docs/index.md(1 hunks)packages/h3/src/index.test.ts(2 hunks)packages/h3/src/index.ts(3 hunks)packages/hono/README.md(2 hunks)packages/hono/docs/functions/getDetectorLocale.md(1 hunks)packages/hono/docs/functions/useTranslation.md(1 hunks)packages/hono/docs/index.md(2 hunks)packages/hono/src/index.test.ts(2 hunks)packages/hono/src/index.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
packages/h3/src/index.test.ts (2)
packages/h3/src/index.ts (2)
detectLocaleFromAcceptLanguageHeader(241-242)getDetectorLocale(344-347)packages/h3/src/symbols.ts (2)
SYMBOL_I18N(10-10)SYMBOL_I18N_LOCALE(11-11)
packages/hono/src/index.test.ts (1)
packages/hono/src/index.ts (2)
detectLocaleFromAcceptLanguageHeader(193-194)getDetectorLocale(298-301)
packages/hono/src/index.ts (1)
packages/h3/src/index.ts (2)
intlify(122-126)getDetectorLocale(344-347)
packages/h3/src/index.ts (2)
packages/h3/src/symbols.ts (2)
SYMBOL_I18N(10-10)SYMBOL_I18N_LOCALE(11-11)packages/hono/src/index.ts (1)
getDetectorLocale(298-301)
🔇 Additional comments (10)
packages/hono/docs/functions/useTranslation.md (1)
28-28: LGTM: Documentation wording improvement.The simplified phrasing "use translation function in handler" is more accurate for Hono's context-based architecture.
packages/hono/docs/index.md (1)
29-29: LGTM: Consistent documentation update.The updated description aligns with the corresponding change in useTranslation.md.
packages/h3/README.md (1)
152-180: LGTM: Clear example of the new API.The README update provides a clear example of using
getDetectorLocaleto retrieve the detected locale asynchronously, showing both the import and practical usage pattern.packages/hono/docs/functions/getDetectorLocale.md (1)
33-41: LGTM: Clear usage example.The example clearly demonstrates the async usage pattern and how to access locale properties.
packages/h3/docs/functions/getDetectorLocale.md (1)
33-41: LGTM: Clear usage example.The example demonstrates the async usage pattern appropriately for H3 event handlers.
packages/hono/README.md (1)
123-151: LGTM! Clear demonstration of the new API.The example effectively demonstrates the new
getDetectorLocaleutility with proper async/await usage and shows how to access the locale properties. The variable rename frommiddlewaretoi18nMiddlewarealso improves clarity.packages/hono/src/index.ts (2)
196-217: Well-designed helper function with robust error handling.The helper effectively centralizes locale detection logic with clear error messages that guide users to the correct setup. The comment on lines 211-212 helpfully explains that awaiting works for both sync and async detectors, which is a subtle but important detail.
278-301: Clean implementation with excellent documentation.The function correctly leverages the shared helper and has comprehensive JSDoc including a practical example. The API is consistent with the h3 package implementation.
packages/h3/src/index.ts (2)
264-278: Solid helper implementation adapted for H3 architecture.The function correctly uses H3's symbol-based context pattern and includes appropriate error handling. The error message correctly references "plugin" (vs "middleware" in hono), which aligns with H3's terminology.
344-347: Implementation is correct with clear documentation.The function properly delegates to the helper and returns the expected
Intl.Localetype. The API is consistent with the hono package implementation.
| const locale = await localeDetector(event) | ||
| context[SYMBOL_I18N].locale = locale | ||
|
|
||
| const [locale, context] = await getLocaleAndEventContext(event) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Effective refactoring using the shared helper.
The refactoring centralizes locale detection logic and maintains the existing translation flow while improving code organization.
🤖 Prompt for AI Agents
In packages/h3/src/index.ts around line 302, replace the existing inline
locale-detection and context extraction with a single call to the shared helper
getLocaleAndEventContext and destructure its resolved tuple into [locale,
context]; ensure all subsequent uses reference these variables (remove the old
duplicated detection code paths), preserve the existing translation flow, and
add minimal defensive handling if the helper returns undefined/null so we don't
break downstream code.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/hono/src/index.ts (1)
254-255: Refactoring approved in previous review.The use of the shared helper eliminates duplication and centralizes error handling.
🧹 Nitpick comments (3)
packages/h3/docs/functions/getDetectorLocale.md (2)
25-25: Use "Returns" for consistency with documentation conventions.The return description should use "Returns" rather than "Return" to align with standard JSDoc and documentation conventions.
-Return an `Intl.Locale` instance representing the detected locale +Returns an `Intl.Locale` instance representing the detected locale
19-19: Fix grammar: "A H3 event" → "An H3 event".Use "An" before a vowel sound.
-| `event` | `H3Event` | A H3 event | +| `event` | `H3Event` | An H3 event |packages/hono/src/index.ts (1)
278-301: LGTM - Consistent implementation with H3 package.The function correctly retrieves the detected locale and returns an
Intl.Localeinstance. The JSDoc is comprehensive and includes a helpful example.Note:
new Intl.Locale(locale)will throw aRangeErrorif the locale string is invalid. This is likely intentional to catch configuration errors early, but consider documenting this behavior if it's not obvious to users.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/h3/docs/functions/getDetectorLocale.md(1 hunks)packages/h3/src/index.ts(3 hunks)packages/hono/docs/functions/getDetectorLocale.md(1 hunks)packages/hono/src/index.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/h3/src/index.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/hono/src/index.ts (1)
packages/h3/src/index.ts (2)
intlify(122-126)getDetectorLocale(344-347)
🔇 Additional comments (2)
packages/h3/docs/functions/getDetectorLocale.md (1)
33-40: Example usage is clear and well-demonstrated.The async/await pattern is correctly shown, and accessing the
locale.languageproperty provides a practical illustration of typical usage.packages/hono/src/index.ts (1)
196-216: LGTM - Well-designed helper function.The helper effectively centralizes locale/context retrieval with clear error messages and proper async handling for both sync and async detectors.
Description
Linked Issues
Additional context
Summary by CodeRabbit
New Features
Documentation
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.