diff --git a/.changeset/i18n-translations-request-drop-phantom-filters.md b/.changeset/i18n-translations-request-drop-phantom-filters.md new file mode 100644 index 0000000000..fec49970bf --- /dev/null +++ b/.changeset/i18n-translations-request-drop-phantom-filters.md @@ -0,0 +1,54 @@ +--- +"@objectstack/spec": minor +"@objectstack/client": minor +--- + +fix(spec,client)!: `GetTranslationsRequest` is locale-only — drop the +`namespace` / `keys` filters no server ever read (#3676) + +`GetTranslationsRequestSchema` declared two optional filters, and the endpoint +description promised one of them ("...for the specified locale and optional +namespace"). Neither serving surface read either: the dispatcher domain body +(`runtime/src/domains/i18n.ts`) takes `parts[1]` / `query.locale`, and +service-i18n (`i18n-service-plugin.ts`) takes `req.params.locale`. Both return +the locale's whole bundle. The SDK meanwhile put both on the query string, so a +caller who passed `keys` to shrink the response shrank nothing and got no +indication the filter was inert — Prime Directive #10's declared ≠ enforced, the +same shape #1475 trimmed out of the validation-rule types. + +Trimmed rather than implemented, on three counts: + +- **No consumer.** No call site in this repo or `objectui` passed either field. + The docs (`content/docs/api/client-sdk.mdx`, `skills/objectstack-i18n/SKILL.md`) + already documented `getTranslations(locale)` as a full-bundle snapshot, so the + schema was the outlier, not the docs. The one thing that did exercise them was + a client test asserting the query string got *built* — it pinned the phantom + rather than any behaviour, since no server read what it asserted was sent. It + is replaced here by its inverse: a regression test that the request carries no + filter query at all. +- **`keys` could not deliver what it advertises.** `II18nService.getTranslations` + (`contracts/i18n-service.ts`) takes only `locale`, so a filter could only be a + post-filter over an already-materialized bundle. `keys` reads as a payload + optimization; a post-filter saves wire bytes but none of the server work, and + widening the contract would break every implementer (`memory-i18n`, + `file-i18n-adapter`) for a capability with no caller. +- **`keys` has no defined meaning against the current bundle shape.** Under the + retired flat `o.`-dotted dialect, `keys: ['o.account.label']` was an obvious + pick. #3778 settled the tree on one nested `TranslationData` shape, where a + flat `string[]` is neither a path set nor a group set, and a filtered response + would have to be rebuilt as a sparse nested tree to stay schema-valid. That is + a design decision, and nothing is waiting on it. + +`namespace` is the one that got *easier* — it now lands exactly on +`TranslationData`'s top-level groups, which is what its own description already +said ("e.g., objects, apps, messages"). It is still trimmed here: re-adding an +optional request field is additive and non-breaking the day the Studio's +per-module views actually need it, whereas shipping an unexercised filter path +now means dead code with tests to match, and a declared-but-unread field is +precisely the exemplar the next author copies. + +BREAKING: the two schema fields and the `getTranslations(locale, options?)` +second parameter are removed with no deprecation cycle. Nothing worked through +them — a passed filter was silently ignored — so there is no behavior to +protect. Runtime impact is nil (the fields were optional and now strip); TS +callers passing them fail to compile, which is the intended signal. diff --git a/content/docs/references/api/protocol.mdx b/content/docs/references/api/protocol.mdx index a8953e7331..a4f123fdc4 100644 --- a/content/docs/references/api/protocol.mdx +++ b/content/docs/references/api/protocol.mdx @@ -711,8 +711,6 @@ const result = AiInsightsRequest.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | | **locale** | `string` | ✅ | BCP-47 locale code | -| **namespace** | `string` | optional | Translation namespace (e.g., objects, apps, messages) | -| **keys** | `string[]` | optional | Specific translation keys to fetch | --- diff --git a/packages/client/src/client.test.ts b/packages/client/src/client.test.ts index 1d2a0c6d4f..d0d16f1f80 100644 --- a/packages/client/src/client.test.ts +++ b/packages/client/src/client.test.ts @@ -656,15 +656,23 @@ describe('i18n namespace', () => { ); }); - it('i18n.getTranslations keeps namespace/keys as query params on the path form', async () => { + it('i18n.getTranslations sends no filter query — the server reads none (#3676)', async () => { const { client, fetchMock } = createMockClient({ success: true, data: { locale: 'zh-CN', translations: { hello: '你好' } } }); - await client.i18n.getTranslations('zh-CN', { namespace: 'common', keys: ['a', 'b'] }); - expect(String(fetchMock.mock.calls[0][0])).toBe( - 'http://localhost:3000/api/v1/i18n/translations/zh-CN?namespace=common&keys=a%2Cb', + // This used to accept `{ namespace, keys }` and append them as query + // params. Neither serving surface ever read them (the dispatcher takes + // parts[1]/query.locale, service-i18n takes params.locale), so the + // filter was inert and the caller got the full bundle either way. The + // predecessor of this test asserted the query string was BUILT, which + // pinned the phantom in place rather than the behaviour. + await (client.i18n.getTranslations as (l: string, o?: unknown) => Promise)( + 'zh-CN', { namespace: 'common', keys: ['a', 'b'] }, ); + const url = String(fetchMock.mock.calls[0][0]); + expect(url).toBe('http://localhost:3000/api/v1/i18n/translations/zh-CN'); + expect(url).not.toContain('?'); }); it('i18n.getFieldLabels puts both object and locale on the path (#3636)', async () => { diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index 6d33585942..3d9ace120c 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -3510,15 +3510,16 @@ export class ObjectStackClient { * The `?locale=` query form this used to send matched no route anywhere * and 404'd on the wire; the dispatcher's domain body accepts it, but * nothing ever routes a bare `/translations` to that body (#3636). + * + * Returns the locale's full bundle. The `options.namespace` / `options.keys` + * this used to accept rode the query string to a server that read neither, + * so the filter silently did nothing — trimmed with the request schema's + * fields in #3676. */ - getTranslations: async (locale: string, options?: { namespace?: string; keys?: string[] }): Promise => { + getTranslations: async (locale: string): Promise => { const route = this.getRoute('i18n'); - const params = new URLSearchParams(); - if (options?.namespace) params.set('namespace', options.namespace); - if (options?.keys) params.set('keys', options.keys.join(',')); - const query = params.toString(); const res = await this.fetch( - `${this.baseUrl}${route}/translations/${encodeURIComponent(locale)}${query ? `?${query}` : ''}`, + `${this.baseUrl}${route}/translations/${encodeURIComponent(locale)}`, ); return this.unwrapResponse(res); }, diff --git a/packages/spec/src/api/plugin-rest-api.zod.ts b/packages/spec/src/api/plugin-rest-api.zod.ts index fedf82524f..319f432694 100644 --- a/packages/spec/src/api/plugin-rest-api.zod.ts +++ b/packages/spec/src/api/plugin-rest-api.zod.ts @@ -1182,7 +1182,7 @@ export const DEFAULT_I18N_ROUTES: RestApiRouteRegistration = { category: 'i18n', public: false, summary: 'Get translations for a locale', - description: 'Returns translation strings for the specified locale and optional namespace', + description: "Returns the specified locale's full translation bundle", tags: ['i18n'], responseSchema: 'GetTranslationsResponseSchema', cacheable: true, diff --git a/packages/spec/src/api/protocol.test.ts b/packages/spec/src/api/protocol.test.ts index d8dd372f4a..00f981ffec 100644 --- a/packages/spec/src/api/protocol.test.ts +++ b/packages/spec/src/api/protocol.test.ts @@ -302,7 +302,14 @@ describe('ObjectStack Protocol', () => { { code: 'es-ES', label: 'Spanish (Spain)' }, ], }).success).toBe(true); - expect(GetTranslationsRequestSchema.safeParse({ locale: 'en-US', namespace: 'objects' }).success).toBe(true); + expect(GetTranslationsRequestSchema.safeParse({ locale: 'en-US' }).success).toBe(true); + // The request is locale-only. `namespace`/`keys` were declared here but read + // by no serving surface, so they were trimmed (#3676). Asserting on the + // PARSED OUTPUT is the point: `safeParse` still succeeds on a payload + // carrying them (z.object strips unknown keys), so a success-only assertion + // would keep passing green whether the fields were trimmed or not. + const trimmed = GetTranslationsRequestSchema.parse({ locale: 'en-US', namespace: 'objects', keys: ['a'] }); + expect(trimmed).toEqual({ locale: 'en-US' }); expect(GetTranslationsResponseSchema.safeParse({ locale: 'en-US', translations: { objects: { task: { label: 'Task', pluralLabel: 'Tasks' } }, messages: { save: 'Save' } }, diff --git a/packages/spec/src/api/protocol.zod.ts b/packages/spec/src/api/protocol.zod.ts index 3219e6e8c6..8cbe246a76 100644 --- a/packages/spec/src/api/protocol.zod.ts +++ b/packages/spec/src/api/protocol.zod.ts @@ -954,10 +954,28 @@ export const GetLocalesResponseSchema = lazySchema(() => z.object({ })).describe('Available locales'), })); +/** + * `locale` is the whole request — the endpoint returns that locale's full + * bundle, and there is no server-side filter to ask for. + * + * This once declared `namespace` and `keys` filters. Neither serving surface + * ever read them: the dispatcher domain body takes `parts[1]`/`query.locale` + * and service-i18n takes `req.params.locale`, so both returned the whole + * bundle while the SDK dutifully put both on the query string. A caller who + * passed `keys` to shrink the payload shrank nothing and was told nothing — + * Prime Directive #10's declared ≠ enforced, the shape #1475 trimmed out of + * the validation-rule types. Removed rather than implemented (#3676): no + * caller in this repo or objectui passed either one, `II18nService` + * (`contracts/i18n-service.ts`) takes only `locale` so a filter could only be + * a post-filter on an already-materialized bundle — which is precisely not + * the server-side work `keys` reads as saving — and against the nested + * `TranslationData` shape #3778 settled on, a flat `keys: string[]` has no + * defined meaning at all. Re-adding an optional filter is additive and + * non-breaking the day a consumer actually needs one; a declared-but-unread + * field is the exemplar the next author copies. + */ export const GetTranslationsRequestSchema = lazySchema(() => z.object({ locale: z.string().describe('BCP-47 locale code'), - namespace: z.string().optional().describe('Translation namespace (e.g., objects, apps, messages)'), - keys: z.array(z.string()).optional().describe('Specific translation keys to fetch'), })); export const GetTranslationsResponseSchema = lazySchema(() => z.object({