feat(h3): support h3 v2 #32
Conversation
|
Warning Rate limit exceeded@kazupon has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 24 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThe PR migrates the packages/h3 package to H3 v2: replaces middleware-based initialization with a plugin-based API and srvx server runner, introduces symbol-keyed event context fields, adds/exports a plugin and I18nPluginOptions, adapts defineI18nMiddleware to return onRequest/onResponse hooks, updates docs/playgrounds/tests to H3 class + srvx usage, and adjusts workspace/devDependencies (including knip.config.ts change). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant H3App as H3 v2 App
participant I18nPlugin as I18n Plugin (onRequest/onResponse)
participant Route as Route Handler
participant Context as Event Context (symbols)
Client->>H3App: HTTP Request
H3App->>I18nPlugin: onRequest(event)
I18nPlugin->>Context: set SYMBOL_I18N, SYMBOL_I18N_LOCALE on event.context
H3App->>Route: invoke route handler (event)
Route->>Context: useTranslation reads/writes SYMBOL_I18N / SYMBOL_I18N_LOCALE
Route-->>Client: response (translated)
H3App->>I18nPlugin: onResponse(event, response)
I18nPlugin-->>Client: finalize/modify response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related issues
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/h3/README.md (1)
114-140: Fix Nitro plugin example to use correct middleware export name.The README example at lines 123 and 138 destructures
onAfterResponse, but the middleware exportsonResponse(per H3 v2 changes). The Nitro hook name'afterResponse'is correct, but the callback variable should beonResponse:const { onRequest, onResponse } = defineI18nMiddleware({ // ... }) nitroApp.hooks.hook('request', onRequest) nitroApp.hooks.hook('afterResponse', onResponse)packages/h3/src/index.ts (1)
213-235: Fix JSDoc example name fordetectLocaleFromAcceptLanguageHeaderThe JSDoc example still references
detectLocaleWithAcceeptLanguageHeader(with typos in both the verb “From” vs “With” and “Accept” spelling), but the exported function isdetectLocaleFromAcceptLanguageHeader.To avoid confusion for users following the docs, I’d align the example with the actual export name, e.g.:
- * import { defineI18nMiddleware, detectLocaleWithAcceeptLanguageHeader } from '@intlify/h3' + * import { + * defineI18nMiddleware, + * detectLocaleFromAcceptLanguageHeader + * } from '@intlify/h3' ... - * locale: detectLocaleWithAcceeptLanguageHeader + * locale: detectLocaleFromAcceptLanguageHeaderThe implementation itself (
getHeaderLocale(event.req).toString()) looks correct for H3 v2’sH3Event.Also applies to: 241-242
🧹 Nitpick comments (6)
packages/h3/playground/util-header/index.ts (1)
8-9: Add error handling for locale detection.The code assumes getHeaderLocale always returns a valid locale. Consider adding error handling for cases where the locale header is missing or invalid.
Apply this diff to add basic error handling:
app.get('/', event => { const locale = getHeaderLocale(event.req) - return locale.toString() + return locale?.toString() ?? 'en' })packages/h3/playground/util-query/index.ts (1)
8-9: Add error handling for locale detection.Similar to util-header, the code assumes getQueryLocale always returns a valid locale. Consider adding error handling for cases where the locale query parameter is missing or invalid.
Apply this diff to add basic error handling:
app.get('/', event => { const locale = getQueryLocale(event.req) - return locale.toString() + return locale?.toString() ?? 'en' })packages/h3/spec/e2e.spec.ts (1)
31-59: Consider adding server readiness check instead of fixed delay.The tests use a fixed 1000ms delay for server startup, which could be flaky in slow CI environments or unnecessarily slow in fast ones.
Consider implementing a retry-based readiness check:
const waitForServer = async (port: number, maxRetries = 10, delayMs = 100) => { for (let i = 0; i < maxRetries; i++) { try { await runCommand(`curl --fail http://localhost:${port}/health || exit 0`) return } catch { await delay(delayMs) } } throw new Error(`Server not ready after ${maxRetries * delayMs}ms`) }Alternatively, if the playground servers support it, parse stdout for a "ready" message instead of a fixed timeout.
packages/h3/spec/integration.spec.ts (1)
91-152: Verify async locale detector behavior withuseTranslationand bound functionsThe
asyncandasync paralleltests correctly model async locale detectors that:
- Derive the locale via
getQueryLocale(event.req), and- Lazily populate
i18n.messages[locale]through dynamic imports before returning the locale.However, in
defineI18nMiddleware(insrc/index.ts), locale detectors are stored viagetLocaleDetector(event, i18n), which uses.bind(...)on the originallocalefunction.useTranslationthen checkslocaleDetector.constructor.name === 'AsyncFunction'to decide whether toawaitit.In JavaScript, bound functions typically report
constructor.name === 'Function'even if the original was an async function, so an async detector wrapped with.bindmay no longer be detectable via this heuristic. If that happens, the async branch inuseTranslationwon’t run, and these tests would be relying on implementation quirks rather than a guaranteed behavior.Please verify in your runtime that:
- A detector declared as
async (event: H3Event, i18n: CoreContext<...>) => {...}and then wrapped bygetLocaleDetectoris still recognized as async byuseTranslation, or- Adjust
useTranslation/defineI18nMiddlewareto track “async-ness” before binding (or alwaysawaitthe detector result) so async detectors behave predictably.Also applies to: 153-218
packages/h3/src/index.ts (2)
183-197: Optional: clear locale detector from context on response for symmetry
defineI18nMiddlewarecorrectly:
- Creates a shared
CoreContext,- Stores a request-specific locale detector in
event.context[SYMBOL_I18N_LOCALE],- Assigns
i18n.localeandevent.context[SYMBOL_I18N]on request, and- Resets
i18n.localeand deletesevent.context[SYMBOL_I18N]on response.For completeness and to keep per-request state fully cleaned up, you might also delete
event.context[SYMBOL_I18N_LOCALE]in theonResponsehook:- onResponse: onResponse((_, event) => { - i18n.locale = orgLocale - delete event.context[SYMBOL_I18N] - }) + onResponse: onResponse((_, event) => { + i18n.locale = orgLocale + delete event.context[SYMBOL_I18N] + delete event.context[SYMBOL_I18N_LOCALE] + })This doesn’t affect correctness in typical H3 lifecycles but keeps the context shape tidy.
Also applies to: 199-209
404-407: Error message could mention plugin-based setupThe error thrown when middleware is not initialized currently says:
“please setup
onRequestandonResponseoptions ofH3with the middleware obtained withdefineI18nMiddleware”Now that you expose a
pluginfor the common path, it might be clearer to mention both options, e.g., “either register the@intlify/h3plugin on yourH3instance or wiredefineI18nMiddleware’sonRequest/onResponsehooks manually.”This is just a wording tweak but will better guide users toward the preferred plugin-based setup.
📜 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 (24)
knip.config.ts(1 hunks)packages/h3/README.md(9 hunks)packages/h3/docs/functions/defineI18nMiddleware.md(0 hunks)packages/h3/docs/functions/detectLocaleFromAcceptLanguageHeader.md(3 hunks)packages/h3/docs/functions/useTranslation.md(3 hunks)packages/h3/docs/index.md(2 hunks)packages/h3/docs/interfaces/DefineLocaleMessage.md(0 hunks)packages/h3/docs/interfaces/I18nMiddleware.md(1 hunks)packages/h3/docs/type-aliases/I18nPluginOptions.md(1 hunks)packages/h3/docs/variables/plugin.md(1 hunks)packages/h3/package.json(1 hunks)packages/h3/playground/basic/index.ts(1 hunks)packages/h3/playground/global-schema/index.ts(2 hunks)packages/h3/playground/local-schema/index.ts(1 hunks)packages/h3/playground/typesafe-schema/index.ts(3 hunks)packages/h3/playground/util-header/index.ts(1 hunks)packages/h3/playground/util-query/index.ts(1 hunks)packages/h3/spec/e2e.spec.ts(2 hunks)packages/h3/spec/integration.spec.ts(7 hunks)packages/h3/src/index.test-d.ts(1 hunks)packages/h3/src/index.test.ts(5 hunks)packages/h3/src/index.ts(8 hunks)packages/h3/src/symbols.ts(1 hunks)pnpm-workspace.yaml(2 hunks)
💤 Files with no reviewable changes (2)
- packages/h3/docs/interfaces/DefineLocaleMessage.md
- packages/h3/docs/functions/defineI18nMiddleware.md
🧰 Additional context used
🧬 Code graph analysis (9)
packages/h3/spec/integration.spec.ts (1)
packages/h3/src/index.ts (2)
detectLocaleFromAcceptLanguageHeader(241-242)getQueryLocale(30-30)
packages/h3/playground/util-header/index.ts (1)
packages/h3/src/index.ts (1)
getHeaderLocale(27-27)
packages/h3/playground/global-schema/index.ts (1)
packages/h3/src/index.ts (1)
detectLocaleFromAcceptLanguageHeader(241-242)
packages/h3/src/index.test.ts (1)
packages/h3/src/symbols.ts (2)
SYMBOL_I18N(10-10)SYMBOL_I18N_LOCALE(11-11)
packages/h3/playground/util-query/index.ts (1)
packages/h3/src/index.ts (1)
getQueryLocale(30-30)
packages/h3/playground/basic/index.ts (1)
packages/h3/src/index.ts (1)
detectLocaleFromAcceptLanguageHeader(241-242)
packages/h3/playground/typesafe-schema/index.ts (1)
packages/h3/src/index.ts (1)
defineI18nMiddleware(170-210)
packages/h3/playground/local-schema/index.ts (1)
packages/h3/src/index.ts (1)
detectLocaleFromAcceptLanguageHeader(241-242)
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 (5)
CoreContext(31-31)defineI18nMiddleware(120-159)detectLocaleFromAcceptLanguageHeader(188-189)getHeaderLocale(19-19)DefineLocaleMessage(83-83)
🔇 Additional comments (28)
packages/h3/src/index.test-d.ts (1)
35-37: The simplified eventMock is sufficient for type testing.The
.test-d.tsfile performs compile-time type checking only (not runtime execution). The mock with{ context: {} } as H3Eventcorrectly satisfies the type requirements because vitest'sexpectTypeOfassertions verify that the return value ofuseTranslation()is correctly inferred asstring. The symbol-keyed properties (SYMBOL_I18NandSYMBOL_I18N_LOCALE) are optional context extensions and are not needed for type inference verification. Runtime behavior is tested separately inindex.test.tswith a more complete mock setup.packages/h3/playground/typesafe-schema/index.ts (3)
2-2: LGTM! Correct h3 v2 import.The import change from
createAppto{ H3 }correctly reflects the h3 v2 API update.
13-13: LGTM! Correct destructuring pattern.The destructuring of
{ onRequest, onResponse }correctly matches the updateddefineI18nMiddlewareAPI that returns separate request and response handlers.
23-25: Code is correct and follows documented h3 conventions. No changes needed.The middleware registration at lines 24-25 is verified to be correct. The h3 source code explicitly documents that
onRequestmust be registered beforeonResponse, and the playground follows this pattern exactly. This matches the documented approach in the h3 library itself (see packages/h3/src/index.ts lines 160-161). Other playgrounds don't register middleware because they demonstrate different h3 features (plugins, utilities) rather than middleware handling—this is expected variance, not an inconsistency.packages/h3/docs/variables/plugin.md (1)
1-48: LGTM!The plugin documentation is clear and provides a comprehensive example demonstrating the new plugin-based architecture for H3 v2.
knip.config.ts (1)
6-7: LGTM!Adding srvx to ignoreDependencies is appropriate since it's used in playground files (already excluded via the ignore pattern).
pnpm-workspace.yaml (2)
5-8: LGTM!Catalog reordering improves alphabetical consistency with no functional impact.
21-21: Adding msw to built dependencies.The addition of msw to onlyBuiltDependencies suggests it requires native compilation. This is correct if the project uses msw for mocking in tests.
packages/h3/docs/functions/useTranslation.md (1)
30-30: LGTM!Documentation updates correctly reflect the H3 v2 plugin-based architecture and consistent capitalization of "H3" throughout.
Also applies to: 43-43, 66-66, 71-77
packages/h3/src/symbols.ts (1)
1-11: LGTM!Using unique symbols for context keys is an excellent pattern that provides type safety and prevents key collisions. The implementation is clean and well-documented.
packages/h3/package.json (2)
73-73: srvx dependency version is current.The latest published version of srvx is 0.9.6 (published Nov 14, 2025), which matches the version specified in the package.json. The caret syntax allows for compatible updates and is appropriate for this dependency.
71-71: Confirm intentional use of h3 v2.0.1-rc.5 over stable v1.15.4.The latest stable h3 release is v1.15.4, but this package specifies v2.0.1-rc.5 (release candidate). No stable v2 release exists yet. Please confirm this RC version is intentional and document any known compatibility or stability considerations for your use case.
packages/h3/docs/type-aliases/I18nPluginOptions.md (1)
1-21: LGTM! Documentation is clear and well-structured.The new
I18nPluginOptionstype alias documentation correctly describes the type parameters and their defaults. The format is consistent with typical API documentation standards.packages/h3/docs/functions/detectLocaleFromAcceptLanguageHeader.md (1)
30-47: LGTM! Documentation updated to reflect H3 v2 API.The example code correctly demonstrates:
- Import of
H3constructor instead ofcreateApp- Plugin-based middleware initialization with
onRequestandonResponsehooks- Proper chaining with
.use()methodsThe changes align with the broader migration to h3 v2's plugin-based architecture.
packages/h3/src/index.test.ts (2)
16-22: LGTM! Event mock updated for H3 v2.The event mock structure correctly reflects the H3 v2 API with
req.headers.getfor header access. This is more aligned with the Web Request API pattern.
58-71: LGTM! Symbol-based context access implemented correctly.The test correctly demonstrates:
- Symbol-based context keys (
SYMBOL_I18N,SYMBOL_I18N_LOCALE) for storing i18n state- Proper binding of locale detector to the event context
- Updated event mock structure consistent with H3 v2
This approach improves encapsulation by preventing naming conflicts in the event context.
packages/h3/spec/e2e.spec.ts (1)
27-29: LGTM! Better test isolation withafterEach.Switching from
afterAlltoafterEachensures each test has a clean server state, which is the correct approach for isolated e2e tests. This prevents test interference and makes failures easier to diagnose.packages/h3/playground/global-schema/index.ts (1)
21-43: LGTM! Clean migration to H3 v2 plugin-based architecture.The playground correctly demonstrates:
- H3 constructor with
pluginsarray for dependency injection- Plugin-based i18n configuration with locale detection and messages
- Direct route definition with
app.getinstead of separate router- Modern server startup with
serve({ fetch: app.fetch })andawait server.ready()This is a significant improvement over the middleware-based approach and aligns well with h3 v2's design patterns.
packages/h3/docs/interfaces/I18nMiddleware.md (1)
9-16: LGTM! Interface documentation updated for H3 v2.The documentation correctly reflects the API changes:
- Updated to reference H3 (capitalized)
onRequestandonResponsehooks properly documented with links to h3.dev- Removal of
onAfterResponsealigns with h3 v2's standardized hook namingpackages/h3/playground/local-schema/index.ts (1)
24-30: LGTM! Demonstrates local schema pattern effectively.This playground correctly shows an alternative to global type declarations:
- Inline
ResourceSchematype definition within the route handler- Type parameter passed to
useTranslation<ResourceSchema>(event)- Enables type-safe translation keys without global module augmentation
This is a valid pattern for applications that prefer localized type scoping.
packages/h3/README.md (3)
7-12: LGTM! Clear communication about H3 v2 migration.The README properly:
- Uses consistent "H3" capitalization throughout
- Clearly notes this is for H3 v2 with a link to the H3 v1 repository
- Sets appropriate expectations for users
44-75: LGTM! Excellent usage examples for utility functions.The new examples clearly demonstrate:
- Direct locale detection from headers with
getHeaderLocale(event.req)- Query-based locale detection with
getQueryLocale(event.req)- Clean H3 constructor and route definition patterns
These examples provide clear, actionable guidance for common use cases.
79-112: LGTM! Comprehensive translation example with plugin pattern.The example effectively demonstrates:
- H3 constructor with
pluginsarray- Plugin-based i18n configuration
useTranslationusage in route handlers- Complete working server setup with node:http
This provides a complete, copy-pasteable example for users migrating to h3 v2.
packages/h3/docs/index.md (1)
9-14: Docs accurately reflect the new plugin-based APIThe added Variables, Functions, Interfaces, and Type Aliases entries are consistent with the exported
plugin, locale helpers,I18nMiddleware, andI18nPluginOptionsinsrc/index.ts. I don’t see inconsistencies or broken links here.Also applies to: 19-33, 40-40, 47-47
packages/h3/spec/integration.spec.ts (2)
21-53: Translation test coverage for Accept-Language looks solidThe
translationtest now exercises the plugin-based setup (new H3({ plugins: [i18n(...)] }),app.get, andapp.fetchwith anAccept-Languageheader) and validates the expected English message. This is a good smoke test for the default header-based locale detection path.
55-89: Custom query-based locale detector test is well wiredThe
localeDetectorthat callsgetQueryLocale(event.req).toString()and the corresponding plugin configuration verify that a synchronous detector using query parameters behaves as expected. This nicely validates the publicgetQueryLocalehelper together with the plugin.packages/h3/src/index.ts (2)
20-22: Symbol-based H3 context augmentation is appropriateUsing
SYMBOL_I18NandSYMBOL_I18N_LOCALEonH3EventContextis a good way to avoid property name collisions and keeps the i18n state internal to this package. The module augmentation for'h3'and the imports from./symbols.tslook consistent and type-safe.Also applies to: 56-63
72-83: Plugin surface and middleware types look aligned; confirm against h3 v2 typingsThe updated
I18nMiddleware(withMiddleware-typedonRequest/onResponse), theI18nPluginOptionsalias, and thepluginexport that wiresdefineI18nMiddlewareintodefinePluginall match the intended H3 v2 plugin model (new H3({ plugins: [plugin] })andh3.use(...)).To be safe, please double-check against the exact
h3@2.xtypings that:
onRequest/onResponsefrom'h3'indeed return something assignable to yourMiddlewaretype, andh3.use(onRequest/onResponse)is the recommended way to register these hooks for plugins.Also applies to: 86-97, 125-129
| import { defineI18nMiddleware } from '@intlify/h3' | ||
| import { createApp } from 'h3' | ||
| import { H3 } from 'h3' | ||
| import en from './locales/en.ts' | ||
|
|
||
| // define resource schema, as 'en' is master resource schema | ||
| type ResourceSchema = typeof en | ||
|
|
||
| const middleware = defineI18nMiddleware<[ResourceSchema], 'en' | 'ja'>({ | ||
| const i18nMiddleware = defineI18nMiddleware<[ResourceSchema], 'en' | 'ja'>({ | ||
| messages: { | ||
| en: { hello: 'Hello, {name}' } | ||
| } | ||
| // something options | ||
| // ... | ||
| }) | ||
|
|
||
| const app = createApp({ ...middleware }) | ||
| const app = new H3() | ||
| app.use(i18nMiddleware.onRequest) | ||
| app.use(i18nMiddleware.onResponse) | ||
|
|
||
| // something your implementation code ... | ||
| // ... | ||
| ``` |
There was a problem hiding this comment.
Inconsistency: Example still uses old middleware pattern.
The type-safe resources example hasn't been fully migrated to the H3 v2 plugin-based pattern. It still shows:
defineI18nMiddleware(old API)app.use(i18nMiddleware.onRequest)patternnew H3()without plugins array
Update the example to align with the H3 v2 plugin pattern shown in other sections:
-const i18nMiddleware = defineI18nMiddleware<[ResourceSchema], 'en' | 'ja'>({
- messages: {
- en: { hello: 'Hello, {name}' }
- }
- // something options
- // ...
-})
-
-const app = new H3()
-app.use(i18nMiddleware.onRequest)
-app.use(i18nMiddleware.onResponse)
+const app = new H3({
+ plugins: [
+ i18n<[ResourceSchema], 'en' | 'ja'>({
+ messages: {
+ en: { hello: 'Hello, {name}' },
+ ja: { hello: 'こんにちは、{name}' }
+ }
+ // something options
+ // ...
+ })
+ ]
+})Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/h3/README.md around lines 249 to 270, the example uses the old
middleware API (defineI18nMiddleware, new H3(), app.use(...)) which is
inconsistent with H3 v2 plugin-based pattern; replace it with the plugin pattern
by importing/creating the i18n plugin (type-safe resources preserved),
instantiate H3 with a plugins array that includes the i18n plugin, and remove
any app.use(i18nMiddleware.onRequest/onResponse) calls; ensure types for
ResourceSchema and supported locales remain and show registering messages via
the plugin options.
| expectTypeOf<string>(t('test')).toExtend<string>() | ||
| expectTypeOf<string>(t('foo')).toExtend<string>() | ||
| expectTypeOf<string>(t('bar.buz.baz')).toExtend<string>() |
There was a problem hiding this comment.
Incorrect type assertion syntax - not testing the intended behavior.
The current syntax expectTypeOf<string>(t('test')).toExtend<string>() places the generic <string> on expectTypeOf, which means it's checking if string extends string (a tautology) rather than verifying that t('test') actually returns a string type.
Apply this diff to fix the type assertions:
- expectTypeOf<string>(t('test')).toExtend<string>()
- expectTypeOf<string>(t('foo')).toExtend<string>()
- expectTypeOf<string>(t('bar.buz.baz')).toExtend<string>()
+ expectTypeOf(t('test')).toMatchTypeOf<string>()
+ expectTypeOf(t('foo')).toMatchTypeOf<string>()
+ expectTypeOf(t('bar.buz.baz')).toMatchTypeOf<string>()Alternatively, use the more concise .toBeString() matcher:
- expectTypeOf<string>(t('test')).toExtend<string>()
- expectTypeOf<string>(t('foo')).toExtend<string>()
- expectTypeOf<string>(t('bar.buz.baz')).toExtend<string>()
+ expectTypeOf(t('test')).toBeString()
+ expectTypeOf(t('foo')).toBeString()
+ expectTypeOf(t('bar.buz.baz')).toBeString()📝 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.
| expectTypeOf<string>(t('test')).toExtend<string>() | |
| expectTypeOf<string>(t('foo')).toExtend<string>() | |
| expectTypeOf<string>(t('bar.buz.baz')).toExtend<string>() | |
| expectTypeOf(t('test')).toMatchTypeOf<string>() | |
| expectTypeOf(t('foo')).toMatchTypeOf<string>() | |
| expectTypeOf(t('bar.buz.baz')).toMatchTypeOf<string>() |
🤖 Prompt for AI Agents
In packages/h3/src/index.test-d.ts around lines 49 to 51, the type assertions
incorrectly place the generic on expectTypeOf (making the checks tautological).
Update the assertions to assert the type of the call result instead — e.g. call
expectTypeOf(t('test')) and either use .toEqualTypeOf<string>() /
.toExtendTypeOf<string>() with the generic on the matcher or simply use
.toBeString() to assert the returned type is string for t('test'), t('foo') and
t('bar.buz.baz').
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/h3/src/index.ts (1)
212-243: Fix JSDoc example name fordetectLocaleFromAcceptLanguageHeaderThe example imports and uses
detectLocaleWithAcceeptLanguageHeader, but the actual export isdetectLocaleFromAcceptLanguageHeader, and “Acceept” is misspelled. This could confuse users copying the snippet.Suggested change:
- * import { defineI18nMiddleware, detectLocaleWithAcceeptLanguageHeader } from '@intlify/h3' + * import { defineI18nMiddleware, detectLocaleFromAcceptLanguageHeader } from '@intlify/h3' ... - * locale: detectLocaleWithAcceeptLanguageHeader + * locale: detectLocaleFromAcceptLanguageHeaderpackages/h3/playground/global-schema/index.ts (1)
16-19: Fix package name typo in global schema commentThe guidance comment uses
@intlifly/h3instead of@intlify/h3, which could trip up users copying it.Suggested change:
- // please use `declare module '@intlifly/h3'`, if you want to use global resource schema in your project. + // please use `declare module '@intlify/h3'`, if you want to use global resource schema in your project.
♻️ Duplicate comments (2)
packages/h3/README.md (2)
66-66: Fix typo in import statement.This is a duplicate of earlier feedback. The package name should be
@intlify/h3not@inltify/h3.-import { getQueryLocale } from '@inltify/h3' +import { getQueryLocale } from '@intlify/h3'
249-270: Example still uses old middleware pattern instead of H3 v2 plugin pattern.This is a duplicate of earlier feedback. The type-safe resources example should be updated to use the H3 v2 plugin-based pattern for consistency with other examples in the document (see lines 79–112).
The example should migrate from:
const i18nMiddleware = defineI18nMiddleware<[ResourceSchema], 'en' | 'ja'>({...}) const app = new H3() app.use(i18nMiddleware.onRequest) app.use(i18nMiddleware.onResponse)To:
const app = new H3({ plugins: [ i18n<[ResourceSchema], 'en' | 'ja'>({...}) ] })
🧹 Nitpick comments (3)
packages/h3/README.md (2)
232-232: Fix typo: "exeample" → "example".User-facing documentation has duplicate typo.
- > The exeample code is [here] + > The example code is [here]Also applies to: 318-318
311-311: Fix typo: "twe" → "two".-resource keys completion has twe ways. +resource keys completion has two ways.packages/h3/src/index.ts (1)
20-22: Plugin + middleware wiring looks solid; optionally clear locale detector on responseThe new
I18nMiddlewareshape,I18nPluginOptions, andpluginexport align well with an H3 v2-style plugin:definePluginreceives options, andh3.use(onRequest/onResponse)wires the symbol-based context correctly viadefineI18nMiddleware.One small cleanup you might consider:
onResponseresetsi18n.localeand deletesSYMBOL_I18N, but leavesSYMBOL_I18N_LOCALEon the event context. Even though events are short-lived, deleting both keeps per-request state symmetrical and avoids dangling references to user-defined detectors.Possible tweak:
onResponse: onResponse((_, event) => { i18n.locale = orgLocale - delete event.context[SYMBOL_I18N] + delete event.context[SYMBOL_I18N] + delete event.context[SYMBOL_I18N_LOCALE] })Please double‑check this still matches the expectations of the H3 plugin API and your tests around context cleanup.
Also applies to: 56-62, 71-83, 92-129, 170-209
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/h3/README.md(9 hunks)packages/h3/playground/basic/index.ts(1 hunks)packages/h3/playground/global-schema/index.ts(2 hunks)packages/h3/playground/local-schema/index.ts(1 hunks)packages/h3/playground/typesafe-schema/index.ts(3 hunks)packages/h3/playground/util-header/index.ts(1 hunks)packages/h3/playground/util-query/index.ts(1 hunks)packages/h3/src/index.ts(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/h3/playground/util-query/index.ts
- packages/h3/playground/util-header/index.ts
- packages/h3/playground/typesafe-schema/index.ts
🧰 Additional context used
🧬 Code graph analysis (4)
packages/h3/playground/global-schema/index.ts (1)
packages/h3/src/index.ts (1)
detectLocaleFromAcceptLanguageHeader(241-242)
packages/h3/playground/basic/index.ts (1)
packages/h3/src/index.ts (1)
detectLocaleFromAcceptLanguageHeader(241-242)
packages/h3/playground/local-schema/index.ts (1)
packages/h3/src/index.ts (1)
detectLocaleFromAcceptLanguageHeader(241-242)
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 (4)
CoreContext(31-31)defineI18nMiddleware(120-159)detectLocaleFromAcceptLanguageHeader(188-189)getHeaderLocale(19-19)
🔇 Additional comments (3)
packages/h3/src/index.ts (1)
383-407: Async locale detection handling inuseTranslationis now robustSwitching
useTranslationto:
- Read
event.context[SYMBOL_I18N]/SYMBOL_I18N_LOCALE, and- Always
await localeDetector(event),removes the brittle
constructor.name === 'AsyncFunction'check and correctly supports both sync and async detectors (awaiton a non‑promise is a no‑op). Updating the error message to referenceonRequest/onResponsesetup viadefineI18nMiddlewarealso matches the new API.This addresses the prior concern about async detectors being misdetected after
bindand looks good.Please re‑run the integration/e2e tests that cover both sync and async locale detectors to confirm behavior remains correct across your supported H3 versions.
Also applies to: 410-415, 420-427
packages/h3/playground/local-schema/index.ts (1)
1-37: Local-schema playground wiring matches the new plugin APIThe example (
new H3({ plugins: [i18n(...)] }),app.getwithuseTranslation<ResourceSchema>, andserve({ port, fetch: app.fetch })) is consistent with the updated H3 + i18n plugin design and looks correct.Please run this playground against your current
h3,srvx, and@intlify/h3versions to ensure the server boots and translations resolve as expected.packages/h3/playground/basic/index.ts (1)
1-35: Basic playground: plugin-based H3 + i18n example looks correctThis snippet cleanly demonstrates the intended H3 v2 usage: plugin-based i18n configuration,
app.getwithuseTranslation, and serving viasrvx. The inline comment correctly references@intlify/h3.As a sanity check, please run this playground to confirm startup and translation behavior with your current dependency versions.
Description
Linked Issues
resolve #19
Additional context
Summary by CodeRabbit
New Features
Documentation
Tests