-
-
Notifications
You must be signed in to change notification settings - Fork 1
feat: support elysia #38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds a new package Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Elysia
participant Plugin as intlify
participant Detector
participant Core as `@intlify/core`
Client->>Elysia: HTTP request
Elysia->>Plugin: resolve / onRequest
Plugin->>Detector: detectLocale(ctx)
Detector-->>Plugin: locale
Plugin->>Core: prepare core(messages, options)
Plugin->>Elysia: attach ctx.locale & ctx.translate
Elysia->>Elysia: handler calls ctx.translate(key, opts)
Elysia->>Core: translate(key, opts, locale)
Core-->>Elysia: translated string
Elysia-->>Client: HTTP response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
🧹 Nitpick comments (1)
packages/elysia/spec/e2e.spec.ts (1)
8-10: Consider capturing server output for debugging.When E2E tests fail, it's difficult to diagnose issues without seeing the server's stdout/stderr. Consider capturing the spawned process output for better debugging experience.
Add output capture to help diagnose test failures:
let serve: ReturnType<typeof spawn> | null = null +let serverOutput: string[] = [] afterEach(() => { + if (serverOutput.length > 0) { + console.log('Server output:', serverOutput.join('\n')) + } + serverOutput = [] serve?.kill() })Then in each test, capture output:
serve = spawn('pnpx', ['tsx', target]) serve.stdout?.on('data', (data) => serverOutput.push(data.toString())) serve.stderr?.on('data', (data) => serverOutput.push(data.toString()))
📜 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 (21)
knip.config.ts(1 hunks)packages/elysia/CHANGELOG.md(1 hunks)packages/elysia/README.md(1 hunks)packages/elysia/package.json(1 hunks)packages/elysia/playground/basic/index.ts(1 hunks)packages/elysia/playground/global-schema/index.ts(1 hunks)packages/elysia/playground/global-schema/locales/en.ts(1 hunks)packages/elysia/playground/global-schema/locales/ja.ts(1 hunks)packages/elysia/playground/local-schema/index.ts(1 hunks)packages/elysia/playground/local-schema/locales/en.ts(1 hunks)packages/elysia/playground/local-schema/locales/ja.ts(1 hunks)packages/elysia/playground/util-header/index.ts(1 hunks)packages/elysia/playground/util-query/index.ts(1 hunks)packages/elysia/spec/e2e.spec.ts(1 hunks)packages/elysia/spec/fixtures/en.json(1 hunks)packages/elysia/spec/fixtures/ja.json(1 hunks)packages/elysia/spec/integration.spec.ts(1 hunks)packages/elysia/src/index.test.ts(1 hunks)packages/elysia/src/index.ts(1 hunks)packages/elysia/tsdown.config.ts(1 hunks)packages/elysia/typedoc.config.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
packages/elysia/playground/util-header/index.ts (1)
packages/elysia/src/index.ts (1)
getHeaderLocale(27-27)
packages/elysia/src/index.test.ts (1)
packages/elysia/src/index.ts (1)
detectLocaleFromAcceptLanguageHeader(215-216)
packages/elysia/playground/global-schema/index.ts (1)
packages/elysia/src/index.ts (3)
DefineLocaleMessage(79-79)intlify(125-186)detectLocaleFromAcceptLanguageHeader(215-216)
packages/elysia/playground/local-schema/index.ts (1)
packages/elysia/src/index.ts (2)
intlify(125-186)detectLocaleFromAcceptLanguageHeader(215-216)
packages/elysia/spec/integration.spec.ts (1)
packages/elysia/src/index.ts (4)
intlify(125-186)detectLocaleFromAcceptLanguageHeader(215-216)getQueryLocale(30-30)DefineLocaleMessage(79-79)
packages/elysia/playground/util-query/index.ts (1)
packages/elysia/src/index.ts (1)
getQueryLocale(30-30)
packages/elysia/spec/e2e.spec.ts (1)
packages/shared/src/utils.ts (2)
delay(21-23)runCommand(5-19)
packages/elysia/src/index.ts (2)
packages/elysia/playground/global-schema/index.ts (1)
DefineLocaleMessage(14-14)packages/shared/src/types.ts (1)
TranslationFunction(25-39)
packages/elysia/playground/basic/index.ts (1)
packages/elysia/src/index.ts (2)
intlify(125-186)detectLocaleFromAcceptLanguageHeader(215-216)
🪛 GitHub Actions: Typos
packages/elysia/README.md
[warning] 68-68: typo: 'traslation' should be 'translation'.
[warning] 96-96: typo: 'fucntion' should be 'function'.
packages/elysia/src/index.ts
[warning] 88-88: typo: 'fucntion' should be 'function'.
🪛 GitHub Check: Spell check with Typos
packages/elysia/README.md
[warning] 96-96:
"fucntion" should be "function".
[warning] 68-68:
"traslation" should be "translation".
packages/elysia/src/index.ts
[warning] 88-88:
"fucntion" should be "function".
🪛 LanguageTool
packages/elysia/README.md
[grammar] ~68-~68: Ensure spelling is correct
Context: ...gin. As a result, locale property and traslation function will be extended in Elysia Con...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~189-~189: Ensure spelling is correct
Context: ...ompletion resource keys completion has twe ways. ### Type parameter for intlify...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~196-~196: Ensure spelling is correct
Context: ...gnore github alert --> > [!NOTE] > The exeample code is [here](https://github.com/intli...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~226-~226: Using many exclamation marks might seem excessive (in this case: 4 exclamation marks for a text that’s 2539 characters long)
Context: ...E(kazupon): ignore github alert --> > [!NOTE] > The exeample code is [here](http...
(EN_EXCESSIVE_EXCLAMATION)
[grammar] ~227-~227: Ensure spelling is correct
Context: ...gnore github alert --> > [!NOTE] > The exeample code is [here](https://github.com/intli...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
packages/elysia/README.md
196-196: Link text should be descriptive
(MD059, descriptive-link-text)
227-227: Link text should be descriptive
(MD059, descriptive-link-text)
🔇 Additional comments (15)
packages/elysia/typedoc.config.mjs (2)
3-28: LGTM! Configuration follows TypeDoc best practices.The TypeDoc configuration is well-structured with consistent formatting options. All format options are set to 'table' which provides a uniform documentation style, and the plugin-specific options are appropriate for markdown output.
1-28: Now I need to verify that both typedoc and the plugin are properly defined in the workspace catalog:
The configuration is correct. Catalogs are a workspace feature for defining dependency version ranges as reusable constants, with constants later referenced in package.json files. The
"catalog:"value indicates thattypedoc-plugin-markdownis properly managed through the pnpm workspace catalog system (defined inpnpm-workspace.yaml), which is a standard practice in monorepos. Both typedoc (latest version 0.28.14) and typedoc-plugin-markdown (latest version 4.9.0) are available and actively maintained packages. No changes are needed.Likely an incorrect or invalid review comment.
packages/elysia/spec/fixtures/en.json (1)
1-3: LGTM!Valid JSON fixture with appropriate translation template.
packages/elysia/CHANGELOG.md (1)
1-1: LGTM!Appropriate pattern for monorepo changelog management.
packages/elysia/playground/local-schema/locales/ja.ts (1)
1-3: LGTM!Clean locale export following the expected pattern.
knip.config.ts (1)
11-14: LGTM!Workspace configuration follows the established pattern and correctly integrates the new Elysia package.
packages/elysia/playground/util-header/index.ts (1)
1-15: LGTM!Clean playground example demonstrating header-based locale detection. The code appropriately showcases the feature without unnecessary complexity.
packages/elysia/playground/util-query/index.ts (1)
1-15: LGTM!Clean playground example demonstrating query-based locale detection. Follows consistent pattern with other playground examples.
packages/elysia/tsdown.config.ts (1)
1-13: LGTM!Build configuration is well-structured with appropriate settings for the package.
packages/elysia/playground/global-schema/index.ts (1)
17-36: LGTM!The playground example correctly demonstrates the global schema pattern with module augmentation, locale detection, and server setup.
packages/elysia/playground/local-schema/index.ts (1)
1-30: LGTM!The playground example correctly demonstrates the local schema pattern using a type parameter, providing an alternative approach to the global schema augmentation.
packages/elysia/package.json (1)
1-36: LGTM!The package configuration is well-structured with appropriate metadata, exports mapping, scripts, and dependencies using the catalog system for version management.
Also applies to: 48-76
packages/elysia/src/index.ts (3)
52-79: LGTM!The type definitions are well-designed with comprehensive JSDoc documentation and practical examples for module augmentation.
125-186: LGTM!The
intlifyfunction implementation is well-structured:
- Properly handles different locale detector types (function, string, default)
- Uses Elysia's
decorateto extend context withlocaleandtranslate- Uses
resolvewithas: 'global'to run locale detection per request- Translation function correctly binds the detected locale and handles unresolved keys
188-216: LGTM!The locale detection helper is simple, well-documented, and correctly delegates to
getHeaderLocalefrom@intlify/utils.
| "main": "dist/index.mjs", | ||
| "module": "dist/index.mjs", | ||
| "exports": { | ||
| ".": { | ||
| "types": "./dist/index.d.mts", | ||
| "import": "./dist/index.mjs" | ||
| }, | ||
| "./dist/*": "./dist/*", | ||
| "./package.json": "./package.json" | ||
| }, | ||
| "types": "dist/index.d.ts", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify types file extension mismatch.
The exports["."].types field points to ./dist/index.d.mts (line 41), but the top-level types field points to dist/index.d.ts (line 47). This extension mismatch (.d.mts vs .d.ts) could cause TypeScript resolution issues.
Ensure this is intentional and that the build process generates both files, or align them to use the same extension.
Run the following script to check what declaration files are generated:
🏁 Script executed:
#!/bin/bash
# Check for generated type declaration files in dist after build
fd -e d.ts -e d.mts . packages/elysia/dist 2>/dev/null || echo "dist directory not found (expected before build)"Length of output: 48
I'll examine the actual package.json file to verify the complete exports configuration.
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Extract and examine the package.json file for Elysia
cat packages/elysia/package.json | head -60Length of output: 1273
Let me check the TypeScript and build configuration to understand what declaration files are expected to be generated.
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Check for tsconfig files
find packages/elysia -maxdepth 2 -name "tsconfig*.json" -o -name "tsdown.config*" | head -20Length of output: 32
🏁 Script executed:
#!/bin/bash
# Check the root tsconfig if exists
cat tsconfig.json 2>/dev/null | head -50Length of output: 1341
🏁 Script executed:
#!/bin/bash
# Look for any tsdown or build configuration references
grep -r "declaration\|declarationMap\|emitDeclarationOnly" packages/elysia --include="*.json" --include="*.ts" --include="*.js" 2>/dev/null | head -10Length of output: 0
Let me examine the tsdown configuration to see what declaration files are being generated.
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Read the tsdown configuration
cat packages/elysia/tsdown.config.tsLength of output: 250
Let me search for information about tsdown's behavior with TypeScript declaration files.
[/web_search]
🌐 Web query:
tsdown TypeScript declaration files .d.mts .d.ts ESM output
💡 Result:
Short answer: tsdown generates and bundles TypeScript declaration files via rolldown-plugin-dts and will emit ESM-appropriate declaration extensions (.d.mts) when your output is ESM (and .d.cts for CJS). It supports both the TS compiler and faster isolatedDeclarations/oxc flows — enabling isolatedDeclarations in tsconfig is recommended for best performance. See tsdown docs and rolldown-plugin-dts for options and behavior. [1][2][3]
Sources
- tsdown — Declaration Files / build details. [tsdown docs]. [1]
- rolldown-plugin-dts (plugin used by tsdown). [2]
- ts2mjs (example tool that maps .d.ts → .d.mts/.d.cts for ESM/CJS). [3]
Remove or correct the top-level types field in package.json.
Since this is an ESM-only package ("type": "module"), tsdown generates ESM-appropriate declaration extensions (.d.mts) when output is ESM. Therefore, ./dist/index.d.ts will not be generated. The top-level types field (line 47) referencing dist/index.d.ts is:
- Incorrect: Points to a file that won't exist
- Redundant: The
exports["."].typesfield (pointing to.d.mts) takes precedence in modern Node.js/TypeScript and makes the top-level field unnecessary
Either remove the top-level types field or update it to ./dist/index.d.mts for consistency.
🤖 Prompt for AI Agents
In packages/elysia/package.json around lines 37 to 47, the top-level "types"
field incorrectly points to "./dist/index.d.ts" for an ESM-only package; remove
the top-level "types" entry entirely or update it to "./dist/index.d.mts" so it
matches the ESM declaration output and does not conflict with the
exports["."].types entry.
| @@ -0,0 +1,8 @@ | |||
| export default { | |||
| hello: 'こんにちは、{name}', | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verify comma punctuation consistency across Japanese translations.
This file uses a full-width comma (、) in 'こんにちは、{name}', while other Japanese translations in the codebase use half-width commas (e.g., packages/elysia/spec/fixtures/ja.json and the inline messages in packages/elysia/playground/basic/index.ts use 'こんにちは, {name}'). While full-width punctuation is more authentic for Japanese text, the inconsistency across the codebase may cause confusion.
Consider standardizing on either half-width or full-width commas across all Japanese translations:
- hello: 'こんにちは、{name}',
+ hello: 'こんにちは, {name}',Or alternatively, update all other Japanese translations to use full-width commas for consistency.
📝 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.
| hello: 'こんにちは、{name}', | |
| hello: 'こんにちは, {name}', |
🤖 Prompt for AI Agents
In packages/elysia/playground/global-schema/locales/ja.ts around line 2, the
greeting uses a full-width comma ('こんにちは、{name}') which is inconsistent with
other Japanese translations in the repo that use a half-width comma ('こんにちは,
{name}'); standardize punctuation across the codebase by choosing one style and
updating this file (or update other files) so all Japanese translations use the
same comma type, and run a quick grep across locales to ensure global
consistency.
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
E2E test reliability: Fixed delay makes tests brittle.
The test uses a fixed 2000ms delay to wait for server startup without verifying the server actually started successfully. This approach is brittle and can lead to flaky tests - it may be too short on slow CI systems or unnecessarily long on fast systems.
Consider implementing a health check that polls the server until it's ready:
async function waitForServer(url: string, timeout = 5000, interval = 100): Promise<void> {
const start = Date.now()
while (Date.now() - start < timeout) {
try {
await runCommand(`curl -f ${url}`, { timeout: 1000 })
return
} catch {
await delay(interval)
}
}
throw new Error(`Server did not start within ${timeout}ms`)
}
test('util-header', async () => {
const target = path.resolve(import.meta.dirname, '../playground/util-header/index.ts')
serve = spawn('pnpx', ['tsx', target])
await waitForServer('http://localhost:3000')
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`)
})This same pattern should be applied to all three E2E tests (lines 13-21, 23-29, 31-39).
| translate as _translate, | ||
| createCoreContext, | ||
| DEFAULT_LOCALE, | ||
| NOT_REOSLVED, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify constant name from @intlify/core.
The constant NOT_REOSLVED appears to have a typo (should likely be NOT_RESOLVED). Since this is imported from @intlify/core, verify whether this is the actual constant name in the upstream library or if there's a typo in the import.
Also applies to: 180-180
🏁 Script executed:
#!/bin/bash
# Search for NOT_REOSLVED and NOT_RESOLVED in node_modules/@intlify/core
rg -n "NOT_RE[OS].*LVED" node_modules/@intlify/core/ 2>/dev/null || echo "Package not found in node_modules"Length of output: 33
[/web_search]
I'll also check the codebase for the package version to help narrow down the verification:
[/run_scripts]
---
🏁 Script executed:
```shell
#!/bin/bash
# Find package.json to check @intlify/core version
find . -name "package.json" -type f | head -5 | xargs grep -l "@intlify/core" 2>/dev/null | head -1 | xargs cat | grep -A2 "@intlify/core"
Length of output: 70
🌐 Web query:
@intlify/core NOT_RESOLVED constant exports
💡 Result:
Short answer: this error comes from mismatched/incompatible @intlify / vue-i18n packages in your dependency tree (built bundles import symbols that aren’t exported by the installed @intlify/core-* package). Fix by aligning/pinning compatible versions and cleaning the lockfile + node_modules. [1][2][3]
What to do (quick):
- Remove node_modules and lockfile (package-lock.json / yarn.lock / pnpm-lock.yaml) and reinstall.
- Ensure you don’t have multiple conflicting vue-i18n/@intlify versions (npm ls vue-i18n @intlify/core @intlify/shared).
- Pin compatible versions (upgrade to the patched releases if applicable) or add dependency overrides (pnpm/yarn resolutions) so @intlify/shared and @intlify/core-base match the vue-i18n version you use. Example: force @intlify/shared to a stable 11.x if an alpha was pulled in.
- If you’re using Nuxt, remove stray vue-i18n packages if you rely on @nuxtjs/i18n (some users solved by uninstalling vue-i18n).
- Clear any build cache used by Vite/Nuxt (node_modules/.cache, .vite, etc.) and rebuild.
References / examples of the same error and solutions: StackOverflow reporting missing exports (shows NOT_REOSLVED/other missing symbols) and fix by removing conflicting packages; analysis posts explaining version/exports mismatches and recommending pinning/overrides. [1][2][3]
Sources:
[1] StackOverflow — missing export / NOT_REOSLVED error report and fix (uninstall vue-i18n).
[2] Analysis of @intlify export/version mismatches and recommended overrides/pinning.
[3] Example bundle showing NOT_REOSLVED import from @intlify/core-base.
[/web_search]
🌐 Web query:
@intlify/core NOT_REOSLVED source code github
💡 Result:
You can find the @intlify/core source in the intlify/vue-i18n GitHub repository — core code lives under packages/core (and core-base under packages/core-base). See the repo and the npm package pages for quick access. [1][2]
Sources:
- intlify/vue-i18n GitHub repository. [1]
- @intlify/core npm page. [2]
[/web_search]
🌐 Web query:
intlify core "NOT_RESOLVED" OR "NOT_REOSLVED" constant enum
💡 Result:
Short answer: There is no exported NOT_RESOLVED (nor a correctly spelled NOT_REOSLVED) constant/enum in the Intlify core packages. The Intlify/vue-i18n repo (which contains @intlify/core) is the canonical source. [1] A changelog entry also mentions a prior typo fix in the I18n error/enum names (i.e. they fixed typos in I18nErrorCodes), which suggests such misspellings are not part of the public API. [2]
Sources:
- intlify/vue-i18n GitHub repository (packages include @intlify/core). [1]
- @intlify/core-base changelog mentioning a fix for a typo in I18nErrorCodes. [2]
[/run_scripts]
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the actual import statement and usage of NOT_REOSLVED
rg -n "NOT_REOSLVED|NOT_RESOLVED" packages/elysia/src/index.ts -C3
Length of output: 443
Remove invalid import of NOT_REOSLVED from @intlify/core.
The constant NOT_REOSLVED does not exist in @intlify/core. This import (line 16) and its usage (line 180) will cause the code to fail. Verify the correct constant name in the @intlify/core library and update both the import and the comparison accordingly.
🤖 Prompt for AI Agents
packages/elysia/src/index.ts around line 16 (and usage at line 180): the import
of NOT_REOSLVED from @intlify/core is invalid and will break the build; locate
the correct exported constant name in @intlify/core (likely NOT_RESOLVED or
similarly spelled), replace the faulty import at line 16 with the correct
identifier or remove it if not needed, then update the comparison at line 180 to
use that correct constant; finally run type-check/tests to confirm no remaining
references to the misspelled symbol.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/elysia/README.md (2)
227-227: Fix typo: "exeample" should be "example".-> The exeample code is [here](https://github.com/intlify/elysia/tree/main/playground/global-schema) +> The example code is [here](https://github.com/intlify/elysia/tree/main/playground/global-schema)
189-189: Fix typo: "twe" should be "two".-resource keys completion has twe ways. +resource keys completion has two ways.
🧹 Nitpick comments (1)
packages/elysia/README.md (1)
196-196: Improve link text descriptiveness.Using generic "here" as link text is less helpful for accessibility and context. Consider using more descriptive text that indicates what the link points to.
For example:
- Line 196:
[local schema example](...)instead of[here](...)- Line 227:
[global schema example](...)instead of[here](...)Also applies to: 227-227
📜 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 (5)
packages/elysia/README.md(1 hunks)packages/elysia/playground/global-schema/index.ts(1 hunks)packages/elysia/src/index.ts(1 hunks)packages/h3/playground/global-schema/index.ts(1 hunks)packages/hono/playground/global-schema/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/h3/playground/global-schema/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/elysia/src/index.ts
- packages/elysia/playground/global-schema/index.ts
🧰 Additional context used
🪛 LanguageTool
packages/elysia/README.md
[grammar] ~189-~189: Ensure spelling is correct
Context: ...ompletion resource keys completion has twe ways. ### Type parameter for intlify...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~226-~226: Using many exclamation marks might seem excessive (in this case: 4 exclamation marks for a text that’s 2540 characters long)
Context: ...E(kazupon): ignore github alert --> > [!NOTE] > The exeample code is [here](http...
(EN_EXCESSIVE_EXCLAMATION)
[grammar] ~227-~227: Ensure spelling is correct
Context: ...gnore github alert --> > [!NOTE] > The exeample code is [here](https://github.com/intli...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
packages/elysia/README.md
196-196: Link text should be descriptive
(MD059, descriptive-link-text)
227-227: Link text should be descriptive
(MD059, descriptive-link-text)
| // you can put the type extending with `declare module` as global resource schema | ||
| declare module '../../src/index.ts' { | ||
| // please use `declare module '@intlifly/hono'`, if you want to use global resource schema in your project. | ||
| // please use `declare module '@intlify/hono'`, if you want to use global resource schema in your project. | ||
| export interface DefineLocaleMessage extends ResourceSchema {} | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Align all comments to use the corrected @intlify/hono package name
The new guidance in the module augmentation comment (Line 16) correctly uses @intlify/hono, but the earlier inline comment on the import (Line 6) still says @inlify/hono. This inconsistency can confuse users copying example code.
Suggested fix:
-} from '../../src/index.ts' // in your project, `import { ... } from '@inlify/hono'`
+} from '../../src/index.ts' // in your project, `import { ... } from '@intlify/hono'`Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/hono/playground/global-schema/index.ts around lines 14 to 18, the
inline comment earlier in the file mis-spells the package as `@inlify/hono`
while the module augmentation comment correctly uses `@intlify/hono`; update the
earlier comment(s) to use the corrected `@intlify/hono` package name so both
comments are consistent and accurate for users copying the example code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/elysia/src/index.ts (1)
12-19:NOT_REOSLVEDimport and sentinel check look invalid and will likely break.
NOT_REOSLVEDis imported from@intlify/coreand compared against the translate result (Line 181), but this identifier appears misspelled and does not exist in the public@intlify/coreAPI. Keeping this import and comparison will cause build/runtime failures once the bundler resolves@intlify/core.Consider either:
- Replacing it with the correct public sentinel (if one exists and is documented), or
- Dropping the sentinel entirely and falling back based on the value type, e.g.:
-import { - translate as _translate, - createCoreContext, - DEFAULT_LOCALE, - NOT_REOSLVED, - // @ts-expect-error -- NOTE(kazupon): internal function - parseTranslateArgs -} from '@intlify/core' +import { + translate as _translate, + createCoreContext, + DEFAULT_LOCALE, + // @ts-expect-error -- NOTE(kazupon): internal function + parseTranslateArgs +} from '@intlify/core' @@ - const result = Reflect.apply(_translate, null, [ + const result = Reflect.apply(_translate, null, [ intlify, key, arg2, { // bind to request locale locale, ...options } ]) - return NOT_REOSLVED === result ? key : (result as string) + return typeof result === 'string' ? result : keyThis keeps the “return key on not resolved” contract without depending on a non-public constant.
To confirm, please grep the installed
@intlify/coresources for anyNOT_REOSLVED/NOT_RESOLVEDexport and adjust accordingly.Also applies to: 170-182
🧹 Nitpick comments (3)
packages/elysia/src/index.ts (3)
87-90: Remove duplicatedtranslatebullet in JSDoc.The JSDoc lists the
translateproperty twice with identical descriptions. This is redundant and may confuse readers. Suggest keeping a single bullet:- * - `translate`: A {@link TranslationFunction | translate function}, that translates based on the `messages` options specified in the plugin options and the detected locale. - * - `translate`: A {@link TranslationFunction | translate function}, that translates based on the `messages` options specified in the plugin options and the detected locale. + * - `translate`: A {@link TranslationFunction | translate function}, that translates based on the `messages` options specified in the plugin options and the detected locale.
216-217: Guard againstgetHeaderLocalereturning a non-value before calling.toString().If
getHeaderLocale(ctx.request)can ever returnnull/undefined(e.g., missingAccept-Languageheader), then calling.toString()will throw. Consider adding a fallback, for example:-export const detectLocaleFromAcceptLanguageHeader = (ctx: ElysiaContext): Locale => - getHeaderLocale(ctx.request).toString() +export const detectLocaleFromAcceptLanguageHeader = (ctx: ElysiaContext): Locale => { + const locale = getHeaderLocale(ctx.request) + return (locale ?? DEFAULT_LOCALE).toString() +}(or whatever default locale behavior is appropriate for this package).
Please double‑check
@intlify/utils’sgetHeaderLocalecontract to ensure this guard is actually needed.
155-185: InternalparseTranslateArgsusage is fragile but acceptable if you’re pinning@intlify/core.You’re importing
parseTranslateArgswith a@ts-expect-errorand using it to normalize translation arguments before delegating to_translate. This is a clean way to mirror vue-i18n’s argument handling, but it depends on a non-public API that may change without semver guarantees.If you want to make this more robust in the long term, consider:
- Either inlining a minimal, stable version of the parse logic locally, or
- Providing a thin helper in
@intlify/corethat is officially exported and used by all adapters (elysia, hono, h3, etc.).Not blocking if you control
@intlify/core, but it’s worth tracking as a tech‑debt item.
📜 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 (1)
packages/elysia/src/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/elysia/src/index.ts (4)
packages/hono/playground/global-schema/index.ts (1)
DefineLocaleMessage(17-17)packages/h3/playground/global-schema/index.ts (1)
DefineLocaleMessage(18-18)packages/elysia/playground/global-schema/index.ts (1)
DefineLocaleMessage(14-14)packages/shared/src/types.ts (1)
TranslationFunction(25-39)
🔇 Additional comments (1)
packages/elysia/src/index.ts (1)
126-137: The review comment's premise is technically incorrect.Shadowing a function name with a local const declaration does not cause a redeclaration error in TypeScript. The function name
intlifyexists in module scope, whileconst intlifyis in the function's block scope—these are different scopes, and redeclaration errors only occur when the same identifier is declared in the same scope.The code as written will compile successfully with no
"Identifier 'intlify' has already been declared"error, even with strict TypeScript settings enabled (as shown in tsconfig.json).That said, the shadowing is poor practice and does create confusion—renaming the variable for clarity remains a valid suggestion for code quality, just not for the stated reason of avoiding a compilation error.
Likely an incorrect or invalid review comment.
Description
Linked Issues
Additional context
Summary by CodeRabbit
New Features
Tests
Documentation