From 59c64fc3a110bafbcbe6ecc823b630f8f84bc977 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 12:00:00 +0000 Subject: [PATCH] fix(i18n)!: field labels emit the entry shape they declare, and stop discarding help/options (#3847) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `GetFieldLabelsResponseSchema` has always declared each label as `{ label, help?, options? }`. Both serving surfaces emitted `Record` — a bare label per field. A client typed against `GetFieldLabelsResponse` read `labels[field].label` and got undefined, because the value was the string itself. The SDK's type was right the whole time; the servers were wrong. The cost is not only the type mismatch. `FieldTranslationSchema` carries `help` and `options`, bundles populate them, and the endpoint threw them away. objectui needs exactly those — its `spec-translations.ts` transform reads `label` AND `options` (as `fieldOptions...`) — and gets them by pulling the whole bundle from `/i18n/translations/:locale` and resolving client-side. The per-object endpoint could not have served it even if it wanted to: the data was dropped at the emit site. Fixed at that emit site, `resolveObjectFieldLabels`, which both surfaces already share as of #3833, so one change covers both. `help` and `options` attach only when non-empty: an `options: {}` would claim a field has translated options and hand back none, and a `help: ''` would erase a caller's source help text. Fields with no non-empty `label` are still omitted entirely, which is what lets `ResolvedFieldLabel.label` be a required string. The response schema is UNCHANGED — this moves the implementation onto the contract, not the contract onto the implementation. Generated docs are byte-identical for that reason. `placeholder` is deliberately left out: `FieldTranslationSchema` has it and the response schema does not, so emitting it would widen the contract rather than satisfy it, and adding an optional response field later is additive. The regression guard is the part worth keeping: a test that builds the response body from the shared helper and parses it with `GetFieldLabelsResponseSchema`. Nothing had ever put the emitted value and the declared contract in one assertion, which is precisely why a bare string sat under an object schema unnoticed. Third and last of the declared != enforced gaps on this endpoint family, after #3676 and #3833. BREAKING: `labels[field]` is now `{ label, help?, options? }` rather than a string. No consumer in this repo or objectui read it — objectui never calls this route, and in-repo use is the SDK method plus URL-shape tests — so the practical blast radius is nil. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0196gwfwMK5vW8RToPyzGMJo --- .../i18n-field-labels-emit-declared-shape.md | 60 +++++++++++++++++++ packages/runtime/src/http-dispatcher.test.ts | 9 ++- .../src/i18n-service-plugin.test.ts | 6 +- packages/spec/api-surface.json | 1 + .../spec/src/system/i18n-resolver.test.ts | 60 ++++++++++++++++++- packages/spec/src/system/i18n-resolver.ts | 34 +++++++++-- 6 files changed, 160 insertions(+), 10 deletions(-) create mode 100644 .changeset/i18n-field-labels-emit-declared-shape.md diff --git a/.changeset/i18n-field-labels-emit-declared-shape.md b/.changeset/i18n-field-labels-emit-declared-shape.md new file mode 100644 index 0000000000..bec5351b96 --- /dev/null +++ b/.changeset/i18n-field-labels-emit-declared-shape.md @@ -0,0 +1,60 @@ +--- +"@objectstack/spec": minor +"@objectstack/runtime": patch +"@objectstack/service-i18n": patch +--- + +fix(i18n)!: `/i18n/labels/:object/:locale` emits the entry shape it declares — +and stops discarding `help`/`options` (#3847) + +`GetFieldLabelsResponseSchema` has always declared each label as an object: + +```ts +labels: z.record(z.string(), z.object({ + label: z.string(), + help: z.string().optional(), + options: z.record(z.string(), z.string()).optional(), +})) +``` + +Both serving surfaces emitted `Record` — a bare label per field. +A client typed against `GetFieldLabelsResponse` read `labels[field].label` and +got `undefined`, because the value was the string itself. The SDK's type was +right the whole time; the servers were wrong. + +The cost is not only the type mismatch. `FieldTranslationSchema` carries `help` +and `options`, bundles populate them, and the endpoint threw them away. objectui +needs exactly those — its `spec-translations.ts` transform reads `label` **and** +`options` (as `fieldOptions...`) — and gets them by pulling the +whole bundle from `/i18n/translations/:locale` and resolving client-side. The +per-object endpoint could not have served it even if it wanted to: the data was +being dropped at the emit site. + +Fixed at that emit site, `resolveObjectFieldLabels`, which both surfaces already +share as of #3833 — so one change covers both. `help` and `options` are attached +only when non-empty: an `options: {}` would claim a field has translated options +and hand back none, and a `help: ''` would erase a caller's source help text. +Fields with no non-empty `label` are still omitted entirely, which is what lets +`ResolvedFieldLabel.label` be a required string. + +**The response schema is unchanged** — this moves the implementation onto the +contract, not the contract onto the implementation. Generated docs are +byte-identical for that reason. + +`placeholder` is deliberately left out. `FieldTranslationSchema` has it and the +response schema does not, so emitting it would be widening the contract rather +than satisfying it — and adding an optional response field later is additive and +non-breaking, whereas guessing now is not. + +The regression guard is the part worth keeping: a test that builds the response +body from the shared helper and parses it with `GetFieldLabelsResponseSchema`. +Nothing had ever put the emitted value and the declared contract in one +assertion, which is precisely why a bare string could sit under an object schema +unnoticed. Third and last of the declared ≠ enforced gaps on this endpoint +family, after #3676 (request filters no server read) and #3833 (a derivation +scanning a retired dialect). + +BREAKING: `labels[field]` is now `{ label, help?, options? }` rather than a +string. No consumer in this repo or objectui read it — objectui never calls this +route, and in-repo use is the SDK method plus URL-shape tests — so the practical +blast radius is nil, and this is the cheap moment to align it. diff --git a/packages/runtime/src/http-dispatcher.test.ts b/packages/runtime/src/http-dispatcher.test.ts index 19076d5ceb..70e5d021c8 100644 --- a/packages/runtime/src/http-dispatcher.test.ts +++ b/packages/runtime/src/http-dispatcher.test.ts @@ -1932,6 +1932,7 @@ describe('HttpDispatcher', () => { fields: { first_name: { label: 'First Name' }, email: { label: 'Email', help: 'Primary address' }, + status: { label: 'Status', options: { open: 'Open' } }, // No label — partial translation is the normal // state, and a blank entry would overwrite the // caller's source label with an empty string. @@ -1945,9 +1946,13 @@ describe('HttpDispatcher', () => { const result = await dispatcher.handleI18n('/labels/contact/en', 'GET', {}, { request: {} }); expect(result.handled).toBe(true); expect(result.response?.status).toBe(200); + // Entries are objects carrying help/options, per + // `GetFieldLabelsResponseSchema` — not the bare strings both + // surfaces used to emit against it (#3847). expect(result.response?.body?.data?.labels).toEqual({ - first_name: 'First Name', - email: 'Email', + first_name: { label: 'First Name' }, + email: { label: 'Email', help: 'Primary address' }, + status: { label: 'Status', options: { open: 'Open' } }, }); }); diff --git a/packages/services/service-i18n/src/i18n-service-plugin.test.ts b/packages/services/service-i18n/src/i18n-service-plugin.test.ts index 3f2952e8ff..2ee4012cc3 100644 --- a/packages/services/service-i18n/src/i18n-service-plugin.test.ts +++ b/packages/services/service-i18n/src/i18n-service-plugin.test.ts @@ -259,12 +259,16 @@ describe('I18nServicePlugin', () => { const res = createMockRes(); await handler(req, res); + // Each entry is an object, not a bare string: that is what + // `GetFieldLabelsResponseSchema` has always declared, and emitting a + // string contradicted it while discarding the `help`/`options` the + // bundle carries (#3847). expect(res._data).toEqual({ success: true, data: { object: 'account', locale: 'en', - labels: { name: 'Account Name' }, + labels: { name: { label: 'Account Name' } }, }, }); }); diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index 60f37f30c3..917da82f62 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -1068,6 +1068,7 @@ "ResolveOptions (interface)", "ResolvedBook (interface)", "ResolvedEntry (interface)", + "ResolvedFieldLabel (interface)", "ResolvedGroup (interface)", "ResolvedSettingValue (type)", "ResolvedSettingValueSchema (const)", diff --git a/packages/spec/src/system/i18n-resolver.test.ts b/packages/spec/src/system/i18n-resolver.test.ts index 15d6ee981f..ac02889393 100644 --- a/packages/spec/src/system/i18n-resolver.test.ts +++ b/packages/spec/src/system/i18n-resolver.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from 'vitest'; import { ObjectTranslationDataSchema, TranslationDataSchema, type TranslationBundle } from './translation.zod'; +import { GetFieldLabelsResponseSchema } from '../api/protocol.zod'; import { resolveViewLabel, resolveViewDescription, @@ -1044,7 +1045,7 @@ describe('translateObject inline actions (objectstack#3370)', () => { // resolveObjectFieldLabels — the `/i18n/labels/:object/:locale` body // ========================================== -describe('resolveObjectFieldLabels (objectstack#3833)', () => { +describe('resolveObjectFieldLabels (objectstack#3833, #3847)', () => { const data = TranslationDataSchema.parse({ objects: { contact: { @@ -1052,6 +1053,10 @@ describe('resolveObjectFieldLabels (objectstack#3833)', () => { fields: { first_name: { label: 'First Name' }, email: { label: 'Email', help: 'Primary address' }, + status: { + label: 'Status', + options: { open: 'Open', closed: 'Closed' }, + }, phone: { help: 'Mobile preferred' }, }, }, @@ -1061,14 +1066,35 @@ describe('resolveObjectFieldLabels (objectstack#3833)', () => { it('enumerates the labels a locale actually translates', () => { expect(resolveObjectFieldLabels(data, 'contact')).toEqual({ - first_name: 'First Name', - email: 'Email', + first_name: { label: 'First Name' }, + email: { label: 'Email', help: 'Primary address' }, + status: { label: 'Status', options: { open: 'Open', closed: 'Closed' } }, }); }); + it('carries the help and options the bundle holds (#3847)', () => { + // These are the translations the endpoint used to discard by emitting a + // bare string per field. objectui needs exactly them, and had to read the + // full-bundle route to get them. + const out = resolveObjectFieldLabels(data, 'contact'); + expect(out.email?.help).toBe('Primary address'); + expect(out.status?.options).toEqual({ open: 'Open', closed: 'Closed' }); + }); + + it('omits help/options entirely rather than emitting empty ones', () => { + // An `options: {}` would claim the field has translated options and hand + // back none; `help: ''` would erase a caller's source help text. + const out = resolveObjectFieldLabels(data, 'contact'); + expect(out.first_name).toEqual({ label: 'First Name' }); + expect(Object.hasOwn(out.first_name!, 'help')).toBe(false); + expect(Object.hasOwn(out.first_name!, 'options')).toBe(false); + }); + it('omits fields carrying no label rather than emitting a blank one', () => { // Partial translation is the normal state (see ObjectTranslationDataSchema), // and callers merge this over their source labels — a '' would erase them. + // `phone` has help but no label, so it yields no entry at all — which is + // what lets `ResolvedFieldLabel.label` be a required string. expect(resolveObjectFieldLabels(data, 'contact')).not.toHaveProperty('phone'); }); @@ -1088,4 +1114,32 @@ describe('resolveObjectFieldLabels (objectstack#3833)', () => { } as unknown as Parameters[0]; expect(resolveObjectFieldLabels(flat, 'contact')).toEqual({}); }); + + /** + * The guard that would have caught #3847 on the day it was introduced: + * build the response the surfaces actually send and parse it with the schema + * that declares it. Both used to emit `Record` against a + * schema declaring `Record` — a + * mismatch no test compared, because none of them ever put the emitted + * value and the declared contract in the same assertion. + */ + it('produces a body that satisfies GetFieldLabelsResponseSchema (#3847)', () => { + const response = { + object: 'contact', + locale: 'en-US', + labels: resolveObjectFieldLabels(data, 'contact'), + }; + const parsed = GetFieldLabelsResponseSchema.safeParse(response); + expect(parsed.success).toBe(true); + expect(parsed.data?.labels.status?.options?.open).toBe('Open'); + }); + + it('an empty label map is still a valid response body', () => { + const parsed = GetFieldLabelsResponseSchema.safeParse({ + object: 'account', + locale: 'en-US', + labels: resolveObjectFieldLabels(data, 'account'), + }); + expect(parsed.success).toBe(true); + }); }); diff --git a/packages/spec/src/system/i18n-resolver.ts b/packages/spec/src/system/i18n-resolver.ts index 28af462cb8..cfbd9c6c1a 100644 --- a/packages/spec/src/system/i18n-resolver.ts +++ b/packages/spec/src/system/i18n-resolver.ts @@ -746,18 +746,44 @@ function lookupObjectFieldAttr( * * Fields with no non-empty `label` are omitted rather than emitted blank: * partial translation is the normal state (see `ObjectTranslationDataSchema`), - * and a caller merges what comes back over its source labels. + * and a caller merges what comes back over its source labels. That rule is + * also what lets `ResolvedFieldLabel.label` be required — an entry exists + * precisely because a label was found, so a field carrying only `help` yields + * no entry rather than one with a blank label. + * + * Each entry carries `help` and `options` alongside `label`, which is what + * `GetFieldLabelsResponseSchema` has always declared. Both surfaces used to + * emit a bare `Record` instead, so the endpoint contradicted + * its own response schema AND discarded translations the bundle already + * carried — `FieldTranslationSchema` populates `help` and `options`, and + * objectui reads exactly those (as `fieldOptions...`) off the + * full-bundle route, because this endpoint could not give them to it (#3847). */ +export interface ResolvedFieldLabel { + /** Translated field label. Required — an entry exists only because it has one. */ + label: string; + /** Translated help text, when the bundle carries one. */ + help?: string; + /** Option value → translated option label, when the bundle carries them. */ + options?: Record; +} + export function resolveObjectFieldLabels( data: TranslationData | undefined, objectName: string, -): Record { +): Record { const fields = data?.objects?.[objectName]?.fields; - const labels: Record = {}; + const labels: Record = {}; if (!fields) return labels; for (const [fieldName, field] of Object.entries(fields)) { const label = field?.label; - if (typeof label === 'string' && label.length > 0) labels[fieldName] = label; + if (typeof label !== 'string' || label.length === 0) continue; + const entry: ResolvedFieldLabel = { label }; + if (typeof field.help === 'string' && field.help.length > 0) entry.help = field.help; + // Only a non-empty map — an empty `options: {}` would claim the field has + // translated options and hand back none. + if (field.options && Object.keys(field.options).length > 0) entry.options = { ...field.options }; + labels[fieldName] = entry; } return labels; }