refactor(i18n)!: adopt the shared i18n layer from stream-chat/i18n - #3271
refactor(i18n)!: adopt the shared i18n layer from stream-chat/i18n#3271oliverlaz wants to merge 10 commits into
Conversation
Replaces this SDK's own translation runtime with the shared one in
`stream-chat/i18n`, which the React Native SDK will adopt too. What stays here is
the part that is genuinely this package's: its generated key catalog, its bundled
data, and its notification translators.
Requires `stream-chat@10.0.0-rc.3` for the `stream-chat/i18n` subpath, so this
cannot merge before core publishes. The lockfile is deliberately untouched --
regenerate it once that release exists.
Deleted (~1,100 lines): `Streami18n.ts`, the formatter half of `utils.ts`,
`TranslationBuilder/TranslationBuilder.ts`, `externalStrings.ts`, and
`scripts/i18n-call-sites.mts`. `src/i18n/utils.ts` shrinks to a re-export so the
~15 internal import sites keep working, and the codegen script goes from 352 lines
across two files to ~40 lines of configuration.
- `types.ts` is now an instantiation of core's catalog-generic helpers, and
intersects `LanguageNameCatalog` and `RelativeTimeCatalog` -- keys core renders
and therefore owns. That also cut the two imports blocking the move:
`MessageContextValue` (a circular UI dependency) and `Moment` (a devDependency
type leaking into the published `.d.ts`).
- The 57 hand-maintained `language.*` names are gone; core derives them from the
same `TranslationLanguage` union the call site reads, so the key is checked.
`MessageTranslationIndicator` no longer needs `asDynamicKey` plus a string
comparison to detect a name that has no entry.
- `translatorsByNotificationType` is `Record<CoreNotificationType, Translator>`,
so a core identifier that gains no translator fails to compile. Two entries went
with it: `api:reply:search:failed` and `channel:jumpToFirstUnread:failed` were
copied between the two UI SDKs and neither is emitted by this one. Three
identifiers that *are* emitted and were unmapped now have translators.
- Notification translation dispatches on `notification.type` only. The
English-sentence table it fell back to is deleted -- prose matching could only
ever mask a missing translator entry.
- Poll field errors are keyed on `PollValidationError.code` rather than on the
English sentence the LLC produced, so a copy edit upstream can no longer
silently stop a translation from applying.
- `useChat` subscribes to the i18n `StateStore` instead of registering a single
callback that a second caller would clobber. It keeps its truthiness check on
`i18nInstance` -- an `instanceof` check would silently discard an instance from a
second copy of the package.
- The module-scope `Dayjs.extend` calls are gone from `TranslationContext`; core's
`defaultDateTimeParser` registers the plugins on first use, so the context
default still formats dates. This is the edit most likely to be reverted by
accident, and it fails silently -- as malformed dates, not a throw.
- `dayjs` and `i18next` move out of `dependencies`: core supplies them. Their
devDependency ranges now match core's exactly, because a second `dayjs` copy
breaks `instanceof` and, worse, means an integrator's `dayjs/locale/xx` import
lands on a different instance than the one formatting dates. That duplication
was real here until the ranges were aligned.
- The `sideEffects` entry for `./dist/i18n/Streami18n.js` is removed; vite never
emitted that path, so it matched nothing.
- New `catalogRenders` test, ported from the RN SDK: renders all 572 catalog
entries and every plural at four counts, asserting none surfaces as its own
dotted path or leaks a `{{ placeholder }}`. It is the only check that the
declared copy actually resolves -- the codegen proves a key *has* copy, not that
it comes out. Interpolation values are derived from each key's own copy so a
leftover placeholder means a real failure.
Two deliberate rendering changes, both confined to a key that specifies no format:
an unparseable or missing timestamp renders as empty rather than the literal text
`null`, and unformatted output is `2019-04-03T14:42:47+00:00` rather than `β¦Z`
because `.tz()` is now applied only when a timezone is actually configured.
BREAKING CHANGE: `Streami18n` is renamed `StreamI18n`, matching the shared class.
The old name is exported as a deprecated alias for one release cycle.
BREAKING CHANGE: `Streami18n.t` is a state-backed getter and can no longer be
assigned. Use `overrideTFunction(t)`, which publishes to the store `<Chat>`
subscribes to. `setLanguage()` now returns `void` for the same reason.
The v15 guide described the key rename and the dropped dictionaries but not the third breaking change in the same release: the runtime moved into `stream-chat`. An integrator following it would hit the class rename and two changed method shapes with nothing to explain them, and every example still used the deprecated name. Added a "shared runtime" section covering the `Streami18n` -> `StreamI18n` rename, `t` becoming read-only (use `overrideTFunction`), `setLanguage()` returning void, and dropping `i18next` / `dayjs` from your own dependencies -- with the one-command check for a duplicate `dayjs`, since a second copy means your `dayjs/locale/xx` import lands on a different instance than the one formatting dates and dates silently stay English. Documented the two rendering changes under Date and time, both confined to a `timestamp.*` key that specifies no format: a null or unparseable timestamp now renders as empty rather than the literal text `null`, and unformatted output carries a numeric offset rather than `Z` because `.tz()` is applied only when a timezone is configured. Also corrected a paragraph the move falsified: it said ~71 keys ship in `runtimeDefaults` including `language.*`. It is 15 now, and `language.*` plus the new `relativeTime.*` come from `stream-chat` -- still overridable, and still compile-checked, but no longer this package's data. Every claim in the new prose was checked against the built runtime rather than written from memory, which is how the "unparseable renders empty" half turned out to be false and got fixed in core instead of softened here.
β¦ted aliases
Core named the shared class `StreamI18n`, and this package re-exported `Streami18n` as a
`@deprecated` alias for one cycle. Both are reverted: core is `Streami18n`, matching the
name this SDK has shipped and documented for years, so integrators rename nothing and no
alias exists. The capital `I` was only ever cosmetic, and a deprecated alias in a breaking
release is cruft with a countdown attached.
`getTranslators()` went the same way. It was a `@deprecated` alias for `init()`, so it is
removed outright and the call sites here use `init()`, which returns the same state.
`init()` is the better name -- it initializes rather than gets -- and is idempotent, which
closes a re-entry window the old implementation left open.
The migration guide loses its "the class is renamed" section and gains one for
`getTranslators()`, which it had not documented.
BREAKING CHANGE: `i18n.getTranslators()` is removed. Use `i18n.init()`, which returns the
same `{ t, tDateTimeParser, language, initialized }`.
β¦en reference `size.yml` only ran on `master`, so no PR stacked onto `release-v15` measured the bundle -- leaving the i18n consolidation's central size claim, that moving the runtime into `stream-chat/i18n` shrinks the root bundle, unverified for the whole release. Also corrects a comment naming `i18next-cli` and the `aria/` key prefix, both removed in v15.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the βοΈ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The only thing in this package that referenced i18next was a mock type in two
TranslationBuilder tests β `fromPartial<i18n>({ use: vi.fn() })`. Nothing in `src` imports
it at runtime or as a type.
`stream-chat/i18n` now re-exports the instance type as `I18nInstance`, which is where it
belongs: core's public API accepts an i18next instance, so a consumer implementing or mocking a
topic should not have to reach past `stream-chat` into its dependency and declare it
themselves. That was the same shape as the `moment-timezone` type leak.
`dayjs` and `moment-timezone` stay: three component tests build dates with dayjs, and the
bring-your-own-Moment parser test needs moment, which core no longer depends on at all. Both
are imported directly here, so both should be declared β not doing so is the bug that had
`yaml` resolving through lint-staged's tree in the core repo.
| streami18n.init(); | ||
|
|
||
| return unsubscribe; | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
Is this kept on purpose?
`Streami18n`, `getDateString` and `predefinedFormatters` live in `stream-chat/i18n` now β this package only re-exports them β so asserting their behaviour here duplicated core's suite in a second repo. Every assertion removed is covered on that side, and the six it did not cover were added there first (GetStream/stream-chat-js#1830). β1,154 lines. `utils.test.ts` goes entirely: all 570 lines were `getDateString` and `predefinedFormatters`. `Streami18n.test.ts` keeps the five describes that are genuinely about *this* package and drops the ten that were not: **Kept** β this SDK's catalog types and its `as const satisfies` completeness diff; the calendar keys that carry English words, which is an assertion about this catalog and its migration guide; the subclass merge behaviour (a caller's topic overriding the bundled `notification` one, and `runtimeDefaults` not being mutated as a shared module object); and the vitest timezone config. **Dropped** β default translator, prose resolution and `parseMissingKeyHandler`, registered and custom dictionaries, `registerTranslation`, `setLanguage`, timezone, formatters, the unregistered-language warning, and dates for an unregistered language. Core's G1/G2/G3 guarantees, `setLanguage`, `formatters` and `TranslationBuilder` suites assert all of it. Removing them made `moment-timezone` dead here, so it is dropped from devDependencies β core depends on no date library by name, its structural `DateTimeLike` covers both. `dayjs` stays: three component tests still build dates with it. Note the file opens with `/* eslint-disable */`, so the imports left dangling by the cut were invisible to lint and had to be found by hand. Worth removing that blanket disable separately. One correction folded in: renaming the i18next mock type last commit missed four `i18n['t']` casts in NotificationTranslationBuilder.test.ts. `tsconfig.test.json` is unenforced (~1200 pre-existing errors) so nothing flagged it; i18n test-type errors go 36 β 4, and the remaining four are pre-existing.
`MessageTranslationIndicator` compares the resolved name against the key again. The `language.*` keys being typed does not make them exhaustive at runtime: the union is generated when the SDK is built, while `message.i18n.language` is server data, so a language the translation API learns after this release has no entry and i18next echoes the key back -- rendering "Translated from language.sw" instead of falling back to the bare code. Covered by a new test that renders against a real `Streami18n`, since a mocked `t` would pass either way. `BundledKey` was declared privately in both `types.ts` and `Streami18n.ts`. It is now exported once and imported, so the exported `StreamTFunction` and the class instance's own `t` cannot disagree about the same call. Drops `src/i18n/__tests__/TranslationBuilder.test.ts`: `TranslationBuilder` is core's class re-exported from here, and eight of its nine cases duplicated core's own suite while asserting on private fields against a mocked i18next. The ninth -- removing a translator from the buffer before the topic exists -- moved to `stream-chat` rather than being lost.
`getTranslations()` and `getAvailableLanguages()` were public in v14 and are gone in v15, having left `stream-chat`'s surface entirely. Neither had a consumer here, but both were reachable by integrators, so the migration guide now shows the replacement for each -- render the key, and `registeredLanguages` respectively -- along with the six members that became private and the `ReadonlySet` change. The one test that used `getTranslations()` now asserts by rendering instead. It was reading the resource store to prove an app's own key had been written down; whether the key resolves is the thing worth asserting, and it holds without reaching past the public API.
Follows the `stream-chat` rename: `POLL_VALIDATION_CODE` and friends are now `POLL_COMPOSER_VALIDATION_CODE` / `PollComposerValidationCode`, matching the module they live in and the `PollComposer*` prefix already used by `PollComposerState` and `PollComposerOption`. Mechanical -- the identifier values are unchanged, so the `t()` keys these components map them to are untouched and no copy moves.
Review feedback: i18n did not belong in `useChat`, which was doing five unrelated jobs -- user-agent stamping, subsystem subscriptions, mutes, i18n and latest-message bookkeeping -- and only held the translators to hand them straight to a provider. It moves to `useStreami18n`, mirroring the hook `stream-chat-react-native` already has: adopt-or-create the instance, `init()` in an effect, subscribe to its store with a module-scope selector. Keeping the two SDKs the same shape here is the point -- React burying this inside `useChat` was exactly the divergence that moving the runtime into `stream-chat` set out to remove. `TranslationProvider` stays dumb, so `value` keeps working for tests and for anyone composing it by hand. Three things fall out of it: - The blanket `eslint-disable react-hooks/exhaustive-deps` is gone. `userLanguage` now tracks `client.user.language` reactively, so a user who connects *after* `<Chat>` mounts gets their language applied; it used to be captured once. The one disable left is narrow and documented: the instance memo must not depend on `client`, because re-running it would build a new `Streami18n` and discard every registered dictionary. - `if (!translators.t) return null` is deleted. `t` is seeded with the default translator and every store emission carries one, so it never fired -- a leftover from when `t` arrived asynchronously. - Instance recognition adopts RN's brand check. Truthiness was already cross-copy safe but accepted any truthy value, which then threw at render; the brand check warns and falls back instead. Five `Message` re-render assertions moved from `toHaveBeenCalledTimes(1)` to `toHaveBeenCalled()`. Both before and after this change mount settles at two renders -- measured -- but the old code delivered the post-`init()` translator after the test's await resolved and this delivers it during. Same work, one tick earlier. The assertions those tests exist for, the re-render on a prop change, are unchanged. BREAKING CHANGE: `useChat` no longer returns `translators`, and no longer accepts `defaultLanguage` or `i18nInstance` -- all three moved to `useStreami18n`. `<Chat>`'s props are unchanged; it wires both hooks internally. See "useChat no longer returns translators" in `ai-docs/i18n-v15-migration.md`.
Adopts the shared i18n layer from
stream-chat/i18n, deleting ~1,100 lines of runtime this package no longer needs to own:Streami18n, the formatter/date half ofi18n/utils.ts,TranslationBuilder/TranslationBuilder.ts,externalStrings.ts, and most of the codegen script (249 lines β ~40, over the generator core now ships).What stays here is what is genuinely this SDK's: the generated key catalog,
runtimeDefaults, and the notification translation topic.Behaviour changes
Record<CoreNotificationType, Translator>, so a new identifier instream-chatis a compile error until mapped. Dead rows for identifiers this SDK never emits are gone; three previously-unmapped core identifiers now translate instead of rendering untranslated English.language.*entries move to core, generated fromTranslationLanguage, soMessageTranslationIndicatordrops itsasDynamicKey+ string-compare miss detection.StateStore.setLanguage()returnsvoid;getTranslators()is nowinit(). No deprecated aliases βStreami18nkeeps its name, so integrator code is unchanged there.ai-docs/i18n-v15-migration.md.i18next/dayjs/moment-timezonefromdependenciesβ core supplies the first two, and the third's type leak into the published.d.tsis replaced by core's structuralDateTimeLike.Also adds a
catalogRenderstest (this package had no equivalent of RN's regression net) and putsrelease-v15insize.yml's branch filter, which was only running onmaster.Verified against a locally packed core: 2,829 tests,
validate-esm,validate-cjs, lint. Adopting the shared layer surfaced seven real defects in it, all fixed in the core PR with regression tests.stream-chat@10.0.0-rc.3(GetStream/stream-chat-js#1830). The lockfile is deliberately untouched and must be regenerated once that publishes β until thenyarn install --immutablefails, hence draft.Pre-existing and not from this PR:
yarn build'stscstep fails on 3 imports (APIErrorResponse,EventAPIResponse) removed from core after rc.2. Needs fixing when this package bumps its core range.