Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions .changeset/i18n-translations-request-drop-phantom-filters.md
Original file line numberDiff line numberDiff line change
@@ -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.
2 changes: 0 additions & 2 deletions content/docs/references/api/protocol.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 |


---
Expand Down
16 changes: 12 additions & 4 deletions packages/client/src/client.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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<unknown>)(
'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 () => {
Expand Down
13 changes: 7 additions & 6 deletions packages/client/src/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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<GetTranslationsResponse> => {
getTranslations: async (locale: string): Promise<GetTranslationsResponse> => {
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<GetTranslationsResponse>(res);
},
Expand Down
2 changes: 1 addition & 1 deletion packages/spec/src/api/plugin-rest-api.zod.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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,
Expand Down
9 changes: 8 additions & 1 deletion packages/spec/src/api/protocol.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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' } },
Expand Down
22 changes: 20 additions & 2 deletions packages/spec/src/api/protocol.zod.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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({
Expand Down
Loading