Skip to content

Commit 1d4756e

Browse files
authored
fix(i18n)!: field labels emit the entry shape they declare, and stop discarding help/options (#3847) (#3859)
`GetFieldLabelsResponseSchema` has always declared each label as `{ label, help?, options? }`. Both serving surfaces emitted `Record<string, string>` — 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.<obj>.<fld>.<value>`) — 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.
1 parent b0e5a37 commit 1d4756e

6 files changed

Lines changed: 160 additions & 10 deletions

File tree

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/runtime": patch
4+
"@objectstack/service-i18n": patch
5+
---
6+
7+
fix(i18n)!: `/i18n/labels/:object/:locale` emits the entry shape it declares —
8+
and stops discarding `help`/`options` (#3847)
9+
10+
`GetFieldLabelsResponseSchema` has always declared each label as an object:
11+
12+
```ts
13+
labels: z.record(z.string(), z.object({
14+
label: z.string(),
15+
help: z.string().optional(),
16+
options: z.record(z.string(), z.string()).optional(),
17+
}))
18+
```
19+
20+
Both serving surfaces emitted `Record<string, string>` — a bare label per field.
21+
A client typed against `GetFieldLabelsResponse` read `labels[field].label` and
22+
got `undefined`, because the value was the string itself. The SDK's type was
23+
right the whole time; the servers were wrong.
24+
25+
The cost is not only the type mismatch. `FieldTranslationSchema` carries `help`
26+
and `options`, bundles populate them, and the endpoint threw them away. objectui
27+
needs exactly those — its `spec-translations.ts` transform reads `label` **and**
28+
`options` (as `fieldOptions.<obj>.<fld>.<value>`) — and gets them by pulling the
29+
whole bundle from `/i18n/translations/:locale` and resolving client-side. The
30+
per-object endpoint could not have served it even if it wanted to: the data was
31+
being dropped at the emit site.
32+
33+
Fixed at that emit site, `resolveObjectFieldLabels`, which both surfaces already
34+
share as of #3833 — so one change covers both. `help` and `options` are attached
35+
only when non-empty: an `options: {}` would claim a field has translated options
36+
and hand back none, and a `help: ''` would erase a caller's source help text.
37+
Fields with no non-empty `label` are still omitted entirely, which is what lets
38+
`ResolvedFieldLabel.label` be a required string.
39+
40+
**The response schema is unchanged** — this moves the implementation onto the
41+
contract, not the contract onto the implementation. Generated docs are
42+
byte-identical for that reason.
43+
44+
`placeholder` is deliberately left out. `FieldTranslationSchema` has it and the
45+
response schema does not, so emitting it would be widening the contract rather
46+
than satisfying it — and adding an optional response field later is additive and
47+
non-breaking, whereas guessing now is not.
48+
49+
The regression guard is the part worth keeping: a test that builds the response
50+
body from the shared helper and parses it with `GetFieldLabelsResponseSchema`.
51+
Nothing had ever put the emitted value and the declared contract in one
52+
assertion, which is precisely why a bare string could sit under an object schema
53+
unnoticed. Third and last of the declared ≠ enforced gaps on this endpoint
54+
family, after #3676 (request filters no server read) and #3833 (a derivation
55+
scanning a retired dialect).
56+
57+
BREAKING: `labels[field]` is now `{ label, help?, options? }` rather than a
58+
string. No consumer in this repo or objectui read it — objectui never calls this
59+
route, and in-repo use is the SDK method plus URL-shape tests — so the practical
60+
blast radius is nil, and this is the cheap moment to align it.

packages/runtime/src/http-dispatcher.test.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1962,6 +1962,7 @@ describe('HttpDispatcher', () => {
19621962
fields: {
19631963
first_name: { label: 'First Name' },
19641964
email: { label: 'Email', help: 'Primary address' },
1965+
status: { label: 'Status', options: { open: 'Open' } },
19651966
// No label — partial translation is the normal
19661967
// state, and a blank entry would overwrite the
19671968
// caller's source label with an empty string.
@@ -1975,9 +1976,13 @@ describe('HttpDispatcher', () => {
19751976
const result = await dispatcher.handleI18n('/labels/contact/en', 'GET', {}, { request: {} });
19761977
expect(result.handled).toBe(true);
19771978
expect(result.response?.status).toBe(200);
1979+
// Entries are objects carrying help/options, per
1980+
// `GetFieldLabelsResponseSchema` — not the bare strings both
1981+
// surfaces used to emit against it (#3847).
19781982
expect(result.response?.body?.data?.labels).toEqual({
1979-
first_name: 'First Name',
1980-
email: 'Email',
1983+
first_name: { label: 'First Name' },
1984+
email: { label: 'Email', help: 'Primary address' },
1985+
status: { label: 'Status', options: { open: 'Open' } },
19811986
});
19821987
});
19831988

packages/services/service-i18n/src/i18n-service-plugin.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,12 +259,16 @@ describe('I18nServicePlugin', () => {
259259
const res = createMockRes();
260260
await handler(req, res);
261261

262+
// Each entry is an object, not a bare string: that is what
263+
// `GetFieldLabelsResponseSchema` has always declared, and emitting a
264+
// string contradicted it while discarding the `help`/`options` the
265+
// bundle carries (#3847).
262266
expect(res._data).toEqual({
263267
success: true,
264268
data: {
265269
object: 'account',
266270
locale: 'en',
267-
labels: { name: 'Account Name' },
271+
labels: { name: { label: 'Account Name' } },
268272
},
269273
});
270274
});

packages/spec/api-surface.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1070,6 +1070,7 @@
10701070
"ResolveOptions (interface)",
10711071
"ResolvedBook (interface)",
10721072
"ResolvedEntry (interface)",
1073+
"ResolvedFieldLabel (interface)",
10731074
"ResolvedGroup (interface)",
10741075
"ResolvedSettingValue (type)",
10751076
"ResolvedSettingValueSchema (const)",

packages/spec/src/system/i18n-resolver.test.ts

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import { describe, it, expect } from 'vitest';
44
import { ObjectTranslationDataSchema, TranslationDataSchema, type TranslationBundle } from './translation.zod';
5+
import { GetFieldLabelsResponseSchema } from '../api/protocol.zod';
56
import {
67
resolveViewLabel,
78
resolveViewDescription,
@@ -1044,14 +1045,18 @@ describe('translateObject inline actions (objectstack#3370)', () => {
10441045
// resolveObjectFieldLabels — the `/i18n/labels/:object/:locale` body
10451046
// ==========================================
10461047

1047-
describe('resolveObjectFieldLabels (objectstack#3833)', () => {
1048+
describe('resolveObjectFieldLabels (objectstack#3833, #3847)', () => {
10481049
const data = TranslationDataSchema.parse({
10491050
objects: {
10501051
contact: {
10511052
label: 'Contact',
10521053
fields: {
10531054
first_name: { label: 'First Name' },
10541055
email: { label: 'Email', help: 'Primary address' },
1056+
status: {
1057+
label: 'Status',
1058+
options: { open: 'Open', closed: 'Closed' },
1059+
},
10551060
phone: { help: 'Mobile preferred' },
10561061
},
10571062
},
@@ -1061,14 +1066,35 @@ describe('resolveObjectFieldLabels (objectstack#3833)', () => {
10611066

10621067
it('enumerates the labels a locale actually translates', () => {
10631068
expect(resolveObjectFieldLabels(data, 'contact')).toEqual({
1064-
first_name: 'First Name',
1065-
email: 'Email',
1069+
first_name: { label: 'First Name' },
1070+
email: { label: 'Email', help: 'Primary address' },
1071+
status: { label: 'Status', options: { open: 'Open', closed: 'Closed' } },
10661072
});
10671073
});
10681074

1075+
it('carries the help and options the bundle holds (#3847)', () => {
1076+
// These are the translations the endpoint used to discard by emitting a
1077+
// bare string per field. objectui needs exactly them, and had to read the
1078+
// full-bundle route to get them.
1079+
const out = resolveObjectFieldLabels(data, 'contact');
1080+
expect(out.email?.help).toBe('Primary address');
1081+
expect(out.status?.options).toEqual({ open: 'Open', closed: 'Closed' });
1082+
});
1083+
1084+
it('omits help/options entirely rather than emitting empty ones', () => {
1085+
// An `options: {}` would claim the field has translated options and hand
1086+
// back none; `help: ''` would erase a caller's source help text.
1087+
const out = resolveObjectFieldLabels(data, 'contact');
1088+
expect(out.first_name).toEqual({ label: 'First Name' });
1089+
expect(Object.hasOwn(out.first_name!, 'help')).toBe(false);
1090+
expect(Object.hasOwn(out.first_name!, 'options')).toBe(false);
1091+
});
1092+
10691093
it('omits fields carrying no label rather than emitting a blank one', () => {
10701094
// Partial translation is the normal state (see ObjectTranslationDataSchema),
10711095
// and callers merge this over their source labels — a '' would erase them.
1096+
// `phone` has help but no label, so it yields no entry at all — which is
1097+
// what lets `ResolvedFieldLabel.label` be a required string.
10721098
expect(resolveObjectFieldLabels(data, 'contact')).not.toHaveProperty('phone');
10731099
});
10741100

@@ -1088,4 +1114,32 @@ describe('resolveObjectFieldLabels (objectstack#3833)', () => {
10881114
} as unknown as Parameters<typeof resolveObjectFieldLabels>[0];
10891115
expect(resolveObjectFieldLabels(flat, 'contact')).toEqual({});
10901116
});
1117+
1118+
/**
1119+
* The guard that would have caught #3847 on the day it was introduced:
1120+
* build the response the surfaces actually send and parse it with the schema
1121+
* that declares it. Both used to emit `Record<string, string>` against a
1122+
* schema declaring `Record<string, { label, help?, options? }>` — a
1123+
* mismatch no test compared, because none of them ever put the emitted
1124+
* value and the declared contract in the same assertion.
1125+
*/
1126+
it('produces a body that satisfies GetFieldLabelsResponseSchema (#3847)', () => {
1127+
const response = {
1128+
object: 'contact',
1129+
locale: 'en-US',
1130+
labels: resolveObjectFieldLabels(data, 'contact'),
1131+
};
1132+
const parsed = GetFieldLabelsResponseSchema.safeParse(response);
1133+
expect(parsed.success).toBe(true);
1134+
expect(parsed.data?.labels.status?.options?.open).toBe('Open');
1135+
});
1136+
1137+
it('an empty label map is still a valid response body', () => {
1138+
const parsed = GetFieldLabelsResponseSchema.safeParse({
1139+
object: 'account',
1140+
locale: 'en-US',
1141+
labels: resolveObjectFieldLabels(data, 'account'),
1142+
});
1143+
expect(parsed.success).toBe(true);
1144+
});
10911145
});

packages/spec/src/system/i18n-resolver.ts

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -746,18 +746,44 @@ function lookupObjectFieldAttr(
746746
*
747747
* Fields with no non-empty `label` are omitted rather than emitted blank:
748748
* partial translation is the normal state (see `ObjectTranslationDataSchema`),
749-
* and a caller merges what comes back over its source labels.
749+
* and a caller merges what comes back over its source labels. That rule is
750+
* also what lets `ResolvedFieldLabel.label` be required — an entry exists
751+
* precisely because a label was found, so a field carrying only `help` yields
752+
* no entry rather than one with a blank label.
753+
*
754+
* Each entry carries `help` and `options` alongside `label`, which is what
755+
* `GetFieldLabelsResponseSchema` has always declared. Both surfaces used to
756+
* emit a bare `Record<string, string>` instead, so the endpoint contradicted
757+
* its own response schema AND discarded translations the bundle already
758+
* carried — `FieldTranslationSchema` populates `help` and `options`, and
759+
* objectui reads exactly those (as `fieldOptions.<obj>.<fld>.<value>`) off the
760+
* full-bundle route, because this endpoint could not give them to it (#3847).
750761
*/
762+
export interface ResolvedFieldLabel {
763+
/** Translated field label. Required — an entry exists only because it has one. */
764+
label: string;
765+
/** Translated help text, when the bundle carries one. */
766+
help?: string;
767+
/** Option value → translated option label, when the bundle carries them. */
768+
options?: Record<string, string>;
769+
}
770+
751771
export function resolveObjectFieldLabels(
752772
data: TranslationData | undefined,
753773
objectName: string,
754-
): Record<string, string> {
774+
): Record<string, ResolvedFieldLabel> {
755775
const fields = data?.objects?.[objectName]?.fields;
756-
const labels: Record<string, string> = {};
776+
const labels: Record<string, ResolvedFieldLabel> = {};
757777
if (!fields) return labels;
758778
for (const [fieldName, field] of Object.entries(fields)) {
759779
const label = field?.label;
760-
if (typeof label === 'string' && label.length > 0) labels[fieldName] = label;
780+
if (typeof label !== 'string' || label.length === 0) continue;
781+
const entry: ResolvedFieldLabel = { label };
782+
if (typeof field.help === 'string' && field.help.length > 0) entry.help = field.help;
783+
// Only a non-empty map — an empty `options: {}` would claim the field has
784+
// translated options and hand back none.
785+
if (field.options && Object.keys(field.options).length > 0) entry.options = { ...field.options };
786+
labels[fieldName] = entry;
761787
}
762788
return labels;
763789
}

0 commit comments

Comments
 (0)