feat: add /i18n route dispatch to HttpDispatcher#861
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Add handleI18n method supporting: - GET /i18n/locales → getLocales() - GET /i18n/translations/:locale → getTranslations(locale) - GET /i18n/labels/:object/:locale → getFieldLabels(object, locale) Includes fallback for deriving field labels from translations when getFieldLabels is not available on the service. Supports both path params and query params for locale. Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
|
@copilot 仔细检查是否已经完成了所有的开发并评估还有没有类似的问题 |
i18n 路由分发已完成（ 经过全面检查
这 4 个与 i18n 属于同类问题（discovery 中已广播、协议中有定义、客户端有调用，但 dispatcher 未实现），建议分别创建 cross-repo issue 跟踪。 |
There was a problem hiding this comment.
Pull request overview
Adds /i18n routing support to HttpDispatcher so i18n API requests no longer fall through to 404, and introduces a dedicated handleI18n() handler plus new test coverage for the i18n routing behavior.
Changes:
- Added
/i18nprefix routing inHttpDispatcher.dispatch(). - Implemented
handleI18n()with routes for locales, translations, and field labels (including a fallback whengetFieldLabelsis missing). - Added tests covering
handleI18n()and dispatch integration for/i18n.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/runtime/src/http-dispatcher.ts | Adds /i18n dispatch routing and implements handleI18n() route handler. |
| packages/runtime/src/http-dispatcher.test.ts | Adds a new handleI18n test suite covering routes, error cases, fallback behavior, and dispatch integration. |
| if (parts[0] === 'translations') { | ||
| const locale = parts[1] ? decodeURIComponent(parts[1]) : query?.locale; | ||
| if (!locale) return { handled: true, response: this.error('Missing locale parameter', 400) }; | ||
| const translations = i18nService.getTranslations(locale); |
There was a problem hiding this comment.
handleI18n currently ignores the namespace and keys query params defined by GetTranslationsRequestSchema and used by the client (keys is sent as a comma-separated list). This means callers can’t request a scoped subset of translations and will always receive the full locale payload. Consider parsing query.namespace / query.keys and filtering the returned translation data accordingly (or returning 400 if an unsupported option is provided).
| const translations = i18nService.getTranslations(locale); | |
| // Base translation bundle for the requested locale | |
| const fullTranslations = i18nService.getTranslations(locale) || {}; | |
| // Optional filtering via namespace / keys query params | |
| const rawNamespace = query?.namespace; | |
| const rawKeys = query?.keys; | |
| // Validate basic types for namespace | |
| if (rawNamespace !== undefined && typeof rawNamespace !== 'string' && !Array.isArray(rawNamespace)) { | |
| return { handled: true, response: this.error('Invalid namespace parameter', 400) }; | |
| } | |
| // Validate basic types for keys | |
| if (rawKeys !== undefined && typeof rawKeys !== 'string' && !Array.isArray(rawKeys)) { | |
| return { handled: true, response: this.error('Invalid keys parameter', 400) }; | |
| } | |
| // Normalize namespace(s) to a string array | |
| const namespaces: string[] = []; | |
| if (typeof rawNamespace === 'string') { | |
| for (const ns of rawNamespace.split(',')) { | |
| const trimmed = ns.trim(); | |
| if (trimmed) namespaces.push(trimmed); | |
| } | |
| } else if (Array.isArray(rawNamespace)) { | |
| for (const ns of rawNamespace) { | |
| if (typeof ns === 'string') { | |
| const trimmed = ns.trim(); | |
| if (trimmed) namespaces.push(trimmed); | |
| } | |
| } | |
| } | |
| // Normalize keys to a string array (client sends comma-separated list) | |
| const keyFilters: string[] = []; | |
| if (typeof rawKeys === 'string') { | |
| for (const k of rawKeys.split(',')) { | |
| const trimmed = k.trim(); | |
| if (trimmed) keyFilters.push(trimmed); | |
| } | |
| } else if (Array.isArray(rawKeys)) { | |
| for (const k of rawKeys) { | |
| if (typeof k === 'string') { | |
| const trimmed = k.trim(); | |
| if (trimmed) keyFilters.push(trimmed); | |
| } | |
| } | |
| } | |
| // If both filters are supplied, treat as unsupported to avoid ambiguity | |
| if (namespaces.length > 0 && keyFilters.length > 0) { | |
| return { handled: true, response: this.error('Cannot combine namespace and keys filters', 400) }; | |
| } | |
| let translations = fullTranslations; | |
| if (namespaces.length > 0) { | |
| const filtered: Record<string, any> = {}; | |
| for (const [key, value] of Object.entries(fullTranslations)) { | |
| for (const ns of namespaces) { | |
| if ( | |
| key === ns || | |
| key.startsWith(ns + '.') || | |
| key.startsWith(ns + ':') | |
| ) { | |
| filtered[key] = value; | |
| break; | |
| } | |
| } | |
| } | |
| translations = filtered; | |
| } else if (keyFilters.length > 0) { | |
| const filtered: Record<string, any> = {}; | |
| for (const k of keyFilters) { | |
| if (Object.prototype.hasOwnProperty.call(fullTranslations, k)) { | |
| filtered[k] = (fullTranslations as any)[k]; | |
| } | |
| } | |
| translations = filtered; | |
| } |
| // GET /i18n/locales | ||
| if (parts[0] === 'locales' && parts.length === 1) { | ||
| const locales = i18nService.getLocales(); | ||
| return { handled: true, response: this.success({ locales }) }; | ||
| } |
There was a problem hiding this comment.
PR description says the handler routes endpoints “per the API protocol spec”, but the response shapes here don’t match the spec schemas: e.g. GetLocalesResponseSchema expects locales with metadata objects ({code,label,isDefault}), while this returns the raw string[] from II18nService.getLocales(). Either align the handler output to the spec response types (and update the service contract if needed) or adjust the PR description to reflect the actual contract used by runtime/client.
| it('should fallback to deriving labels from translations when getFieldLabels is missing', async () => { | ||
| delete mockI18nService.getFieldLabels; | ||
| mockI18nService.getTranslations.mockReturnValue({ | ||
| 'o.contact.fields.first_name': 'First Name', | ||
| 'o.contact.fields.email': 'Email', | ||
| 'o.contact.label': 'Contact', | ||
| }); | ||
|
|
||
| const result = await dispatcher.handleI18n('/labels/contact/en', 'GET', {}, { request: {} }); | ||
| expect(result.handled).toBe(true); | ||
| expect(result.response?.status).toBe(200); | ||
| expect(result.response?.body?.data?.labels).toEqual({ | ||
| first_name: 'First Name', | ||
| email: 'Email', | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The fallback test for missing getFieldLabels uses flat translation keys like o.contact.fields.first_name and expects string values, but the default TranslationDataSchema / FileI18nAdapter returns nested objects (e.g. translations.objects.contact.fields.first_name.label). As written, this test can pass while the production fallback remains broken. Adjust the test fixture to use the real nested structure and assert the expected response shape.
| const prefix = `o.${objectName}.fields.`; | ||
| const labels: Record<string, string> = {}; | ||
| for (const [key, value] of Object.entries(translations)) { | ||
| if (key.startsWith(prefix)) { | ||
| labels[key.substring(prefix.length)] = value as string; | ||
| } | ||
| } |
There was a problem hiding this comment.
The /labels fallback derives labels by scanning for flat keys with prefix o.${objectName}.fields. and casting values to string. This won’t work with the repo’s TranslationDataSchema / FileI18nAdapter, where translations are nested under objects.{object}.fields.{field} and field values are objects (label/help/options). In production, this likely returns an empty labels map or incorrect values; consider extracting from translations.objects?.[objectName]?.fields and shaping the response accordingly.
| const prefix = `o.${objectName}.fields.`; | |
| const labels: Record<string, string> = {}; | |
| for (const [key, value] of Object.entries(translations)) { | |
| if (key.startsWith(prefix)) { | |
| labels[key.substring(prefix.length)] = value as string; | |
| } | |
| } | |
| const labels: Record<string, string> = {}; | |
| // Preferred: TranslationDataSchema / FileI18nAdapter nested structure: | |
| // translations.objects?.[objectName]?.fields?.[fieldName]?.label | |
| const objectTranslations = (translations as any)?.objects?.[objectName]; | |
| const fieldTranslations = objectTranslations && typeof objectTranslations === 'object' | |
| ? (objectTranslations as any).fields | |
| : undefined; | |
| if (fieldTranslations && typeof fieldTranslations === 'object') { | |
| for (const [fieldName, fieldTranslation] of Object.entries(fieldTranslations)) { | |
| const label = (fieldTranslation as any)?.label; | |
| if (typeof label === 'string') { | |
| labels[fieldName] = label; | |
| } | |
| } | |
| } else { | |
| // Legacy fallback: flat keys like "o.${objectName}.fields.${field}" | |
| const prefix = `o.${objectName}.fields.`; | |
| for (const [key, value] of Object.entries(translations as any)) { | |
| if (key.startsWith(prefix) && typeof value === 'string') { | |
| labels[key.substring(prefix.length)] = value; | |
| } | |
| } | |
| } |
HttpDispatcher.dispatch()had no/i18nprefix routing, so all i18n API requests (/api/v1/i18n/translations/:locale, etc.) fell through to 404. The route was already advertised in discovery info and defined in the API protocol spec, but never wired up.Changes
dispatch()method — Added/i18nprefix check alongside existing/analytics,/packages, etc.handleI18n()handler — Routes three endpoints per the API protocol:GET /locales→i18nService.getLocales()GET /translations/:locale→i18nService.getTranslations(locale)GET /labels/:object/:locale→i18nService.getFieldLabels(object, locale)with fallback that derives field labels from the full translation bundle whengetFieldLabelsis not implemented/translations/zh-CN) and query params (/translations?locale=zh-CN) for client SDK compatibilityOriginal prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.