From 62a9365f8570686e54af529280c1175eba70fd5f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 08:55:29 +0000 Subject: [PATCH] =?UTF-8?q?fix(spec,client)!:=20`GetTranslationsRequest`?= =?UTF-8?q?=20is=20locale-only=20=E2=80=94=20drop=20the=20`namespace`/`key?= =?UTF-8?q?s`=20filters=20no=20server=20reads=20(#3676)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The request schema 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 takes `parts[1]`/`query.locale`, service-i18n takes `req.params.locale`, and both return the locale's whole bundle. The SDK put both on the query string, so a caller passing `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. Trimmed rather than implemented. No call site in this repo or objectui passed either field, and the docs already described `getTranslations(locale)` as a full-bundle snapshot, so the schema was the outlier. `keys` could not deliver what it advertises anyway: `II18nService.getTranslations` takes only `locale`, so a filter could only post-filter an already-materialized bundle — saving wire bytes but none of the server work the name implies — and widening that contract would break every implementer for a capability with no caller. `keys` also has no defined meaning against the nested `TranslationData` shape #3778 settled on, where a flat `string[]` is neither a path set nor a group set. `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. Still trimmed: re-adding an optional request field is additive the day the Studio's per-module views need it, whereas shipping an unexercised filter path now means dead code with tests to match. The client test that exercised the fields asserted the query string was BUILT — pinning the phantom rather than any behaviour, since no server read what it asserted was sent. Replaced by its inverse: a regression test that the request carries no filter query at all. The spec test is likewise rewritten to assert the PARSED OUTPUT, because `safeParse` still succeeds on a payload carrying the old fields (z.object strips unknown keys), so a success-only assertion would stay green whether or not the trim happened. BREAKING: the two schema fields and the `getTranslations(locale, options?)` second parameter are removed with no deprecation cycle. A passed filter was silently ignored, so there is no behaviour to protect; runtime impact is nil and TS callers passing them now fail to compile, which is the intended signal. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0196gwfwMK5vW8RToPyzGMJo --- ...anslations-request-drop-phantom-filters.md | 54 +++++++++++++++++++ content/docs/references/api/protocol.mdx | 2 - packages/client/src/client.test.ts | 16 ++++-- packages/client/src/index.ts | 13 ++--- packages/spec/src/api/plugin-rest-api.zod.ts | 2 +- packages/spec/src/api/protocol.test.ts | 9 +++- packages/spec/src/api/protocol.zod.ts | 22 +++++++- 7 files changed, 102 insertions(+), 16 deletions(-) create mode 100644 .changeset/i18n-translations-request-drop-phantom-filters.md 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({