feat(i18n): add @clerk/i18n reactive i18n engine#8861
Open
alexcarpenter wants to merge 10 commits into
Open
Conversation
🦋 Changeset detectedLatest commit: b4e6475 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR introduces the Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant createI18n
participant locale_atom as locale atom
participant Fetcher as options.get
participant MessageStore
App->>createI18n: createI18n($locale, { get, overrides, cache })
createI18n-->>App: i18n factory + i18n.loading
App->>createI18n: i18n('namespace', baseMessages)
createI18n-->>App: MessageStore (computed)
locale_atom->>createI18n: locale changed to 'fr'
createI18n->>Fetcher: get('fr') via task()
createI18n->>MessageStore: loading = true
Fetcher-->>createI18n: namespace data resolved
createI18n->>MessageStore: merge overrides and rebuild entries
createI18n->>MessageStore: loading = false
MessageStore-->>App: updated messages snapshot
sequenceDiagram
participant RSC as Server Component
participant loadTranslations
participant translationsLoading
participant i18n_loading as i18n.loading
RSC->>loadTranslations: loadTranslations(messages)
loadTranslations->>translationsLoading: translationsLoading(messages.i18n)
translationsLoading->>i18n_loading: subscribe until false
i18n_loading-->>translationsLoading: loading becomes false
translationsLoading-->>loadTranslations: resolved
loadTranslations->>loadTranslations: messages.get()
loadTranslations-->>RSC: translation snapshot
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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 |
New package: createI18n, params, count, messageFormat, defineLocalization, loadTranslations, messagesToJSON, and React bindings (useStore, Message). Built on nanostores; engine ships zero locale data.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
packages/i18n/src/define-localization/index.test.ts (1)
60-71: ⚡ Quick winAdd a regression test for dangerous keys (
__proto__/constructor/prototype).Current tests cover shape conversion well, but they don’t protect against prototype-polluting inputs. Add one malicious-key case so this behavior can’t regress.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/i18n/src/define-localization/index.test.ts` around lines 60 - 71, The test file for defineLocalization lacks regression coverage for prototype pollution vulnerabilities. After the existing test `'passes non-string override values through verbatim'`, add a new test case that verifies the defineLocalization function properly handles dangerous keys like __proto__, constructor, and prototype by ensuring these keys do not pollute the prototype chain. The test should pass malicious-key inputs to defineLocalization and assert that the resulting object structure does not exhibit prototype pollution behavior (for example, verify that attempting to access inherited properties through the polluted keys fails or returns undefined rather than contaminating the prototype).packages/i18n/src/messages-to-json/index.test.ts (1)
34-40: ⚡ Quick winAdd an edge-case test for
__proto__keys in namespace/message keys.Given serialization writes dynamic keys, this suite should include a malicious-key case to prevent future prototype-pollution regressions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/i18n/src/messages-to-json/index.test.ts` around lines 34 - 40, Add a new test case to the test suite in the messagesToJSON test file that covers the edge-case for prototype pollution prevention. The test should invoke the messagesToJSON function with `__proto__` as a key in either the namespace or message objects, and verify that the function safely handles this malicious key without causing prototype pollution or unintended side effects on the object prototype chain.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/i18n/package.json`:
- Around line 51-64: The pnpm-lock.yaml file is out of sync with the dependency
changes made in packages/i18n/package.json. Run pnpm install to regenerate the
lockfile with the updated specifiers for nanostores, `@testing-library/react`, and
other dependencies, then commit the updated pnpm-lock.yaml file to resolve the
CI blockage from the frozen-lockfile check.
- Around line 25-34: The `./react` subpath exports defined in package.json
(lines 25-34) are inconsistent with the documented contract which mentions
`formatToParts`, while the actual react entry point implementation only exposes
`useStore`, `Message`, `useMessage`, and `formatToReact`. Align the three
sources of truth: either update the react entry point implementation to export
`formatToParts` if that is the documented contract, or update the documentation
to accurately reflect what the react entry point actually exports (`useStore`,
`Message`, `useMessage`, `formatToReact`), ensuring all three align
consistently.
In `@packages/i18n/README.md`:
- Around line 1-5: Add a new "Compatibility" or "Compatibility Matrix" section
to the `@clerk/i18n` README.md that documents the supported versions for Node.js,
React, TypeScript, runtime environments, and any relevant package constraints.
This section should be added after the existing content to comply with the
coding guidelines that require maintaining compatibility matrices in package
READMEs, and should clearly specify which versions are supported and tested.
In `@packages/i18n/src/create-i18n/index.ts`:
- Around line 72-95: The issue is that Promise.resolve(get(locale)) evaluates
get(locale) synchronously, and if it throws synchronously, the attached catch()
and finally() handlers won't execute, leaving inFlight unredeemed and $loading
stuck at true. To fix this in the pending[locale] task function, replace
Promise.resolve(get(locale)) with Promise.resolve().then(() => get(locale)) to
defer the execution of get(locale) to the next microtask, ensuring that
synchronous errors are properly caught and the finally() handler always
decrements inFlight.
- Around line 78-80: The shallow merge performed by Object.assign({}, ...data)
on line 78 causes loss of translation keys when the same namespace appears in
multiple chunks of the data array. Replace the shallow merge logic with a deep
merge implementation that properly combines nested objects by namespace key,
ensuring that when multiple chunks contain the same namespace (like signIn),
their properties are merged together rather than one completely replacing the
other. The merged variable should contain all translation keys from all chunks
across their respective namespaces.
In `@packages/i18n/src/define-localization/index.ts`:
- Around line 28-37: This code is vulnerable to prototype pollution attacks
because unsanitized dynamic keys from user input are being written directly into
plain objects. At packages/i18n/src/define-localization/index.ts lines 28-37,
replace the for...in loop with Object.entries iteration, initialize the output
object and nested namespace objects using Object.create(null) to create
null-prototype maps, and validate both the namespace key and leaf key to reject
dangerous properties like __proto__, constructor, and prototype before any
assignment. At packages/i18n/src/messages-to-json/index.ts lines 42-46,
similarly replace the for...in loop with Object.entries or Object.keys
iteration, write into null-prototype maps created with Object.create(null), and
block the dangerous keys (__proto__, constructor, prototype) before assigning
values to prevent prototype pollution vulnerabilities in both locations.
In `@packages/i18n/src/index.ts`:
- Around line 1-45: The index.ts file in packages/i18n/src is functioning as a
barrel file with all re-exports, which violates the repository's guidelines
against barrel files due to circular dependency risks. Instead of maintaining a
single catch-all index.ts that re-exports everything (including all exports from
atom, browser, formatter, localeFrom, params, count, messageFormat, createI18n,
defineLocalization, loadTranslations, translationsLoading, messagesToJSON, and
their associated types), refactor to use explicit entry modules or narrowly
scoped export files. Consider creating separate entry points at the package root
level (e.g., packages/i18n/browser.ts, packages/i18n/formatter.ts, etc.) that
consumers can import from directly, or organize the current exports into
logical, narrowly-scoped files that group related functionality together rather
than aggregating everything in a single index.ts.
---
Nitpick comments:
In `@packages/i18n/src/define-localization/index.test.ts`:
- Around line 60-71: The test file for defineLocalization lacks regression
coverage for prototype pollution vulnerabilities. After the existing test
`'passes non-string override values through verbatim'`, add a new test case that
verifies the defineLocalization function properly handles dangerous keys like
__proto__, constructor, and prototype by ensuring these keys do not pollute the
prototype chain. The test should pass malicious-key inputs to defineLocalization
and assert that the resulting object structure does not exhibit prototype
pollution behavior (for example, verify that attempting to access inherited
properties through the polluted keys fails or returns undefined rather than
contaminating the prototype).
In `@packages/i18n/src/messages-to-json/index.test.ts`:
- Around line 34-40: Add a new test case to the test suite in the messagesToJSON
test file that covers the edge-case for prototype pollution prevention. The test
should invoke the messagesToJSON function with `__proto__` as a key in either
the namespace or message objects, and verify that the function safely handles
this malicious key without causing prototype pollution or unintended side
effects on the object prototype chain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 7639b0ee-338a-4cc9-bc01-60f8ab0c9877
📒 Files selected for processing (39)
packages/i18n/.gitignorepackages/i18n/README.mdpackages/i18n/package.jsonpackages/i18n/src/atom/index.test.tspackages/i18n/src/atom/index.tspackages/i18n/src/browser/index.test.tspackages/i18n/src/browser/index.tspackages/i18n/src/count/index.test.tspackages/i18n/src/count/index.tspackages/i18n/src/create-i18n/index.test.tspackages/i18n/src/create-i18n/index.tspackages/i18n/src/define-localization/index.test.tspackages/i18n/src/define-localization/index.tspackages/i18n/src/formatter/index.test.tspackages/i18n/src/formatter/index.tspackages/i18n/src/index.tspackages/i18n/src/index.type.test.tspackages/i18n/src/load-translations/index.test.tspackages/i18n/src/load-translations/index.tspackages/i18n/src/locale-from/index.test.tspackages/i18n/src/locale-from/index.tspackages/i18n/src/message-format/index.test.tspackages/i18n/src/message-format/index.tspackages/i18n/src/messages-to-json/index.test.tspackages/i18n/src/messages-to-json/index.tspackages/i18n/src/params/index.test.tspackages/i18n/src/params/index.tspackages/i18n/src/react/index.test.tsxpackages/i18n/src/react/index.tspackages/i18n/src/transforms/index.test.tspackages/i18n/src/transforms/index.tspackages/i18n/src/translations-loading/index.test.tspackages/i18n/src/translations-loading/index.tspackages/i18n/src/types.tspackages/i18n/tsconfig.jsonpackages/i18n/tsconfig.test.jsonpackages/i18n/tsdown.config.mtspackages/i18n/vitest.config.mtspackages/i18n/vitest.setup.mts
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/i18n/package.json`:
- Line 4: The `private: true` property on line 4 of packages/i18n/package.json
prevents the package from being published to npm, which conflicts with the PR
objective of making `@clerk/i18n` a publishable package. Remove the `"private":
true,` line entirely from the package.json file, as having it set to true will
block all publishing attempts regardless of the publishConfig.access setting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: b2b0f728-e690-4131-bc7f-3a3cac8b511d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
packages/i18n/package.jsonpackages/i18n/src/messages-to-json/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/i18n/src/messages-to-json/index.ts
…space merge, prototype pollution - Defer get(locale) to a microtask (Promise.resolve().then) so synchronous throws are caught by .catch() and inFlight/loading are always cleaned up - Deep-merge NamespaceData chunks per namespace instead of Object.assign spread, so multiple chunks sharing a namespace key don't clobber each other - Harden defineLocalization against prototype pollution: switch for...in to Object.keys, use Object.create(null) for output maps, and reject __proto__/ constructor/prototype namespace and message keys - Add regression tests for prototype-polluting inputs in defineLocalization and messagesToJSON
…e constant before JSDoc
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
packages/i18n— a small, reactive i18n engine that will replace the currentpackages/localizationseager-merge approach. This PR establishes the foundations (engine only, no data, no migration). Integration with Clerk's UI components lives on the companioncarp/i18n-mosaicbranch.What's in the package
createI18n($locale, { get })— a namespace factory. Eachi18n(namespace, base)call returns a nanostorescomputedstore; the resolvedMessages<B>type is inferred frombaseso call sites are fully typed with no codegen.params(template),count(forms),params(count(...)),messageFormat(template)— covering parameterised strings, plurals, combined plural+params, and rich-text markup with real React elements (nodangerouslySetInnerHTML).override ?? locale ?? base. Locale data is fetched lazily; any missing key falls back tobaseindividually, not the whole namespace.defineLocalization+ aReadableStore<ResolvedOverrides>passed tocreateI18n. Accepts nested ({ signIn: { title: '…' } }) or flat dot-notation ({ 'signIn.title': '…' }) forms, both fully typed against thebaseregistry.useStore(a stableuseSyncExternalStorewrapper),<Message>,useMessage,formatToParts— in a separate@clerk/i18n/reactentry point so the core is framework-free.allTasks()(nanostores built-in) flushes all in-flight locale loads globally;translationsLoading(i18n)/loadTranslations(store)scope the wait to one instance for per-request RSC rendering.messagesToJSON(...stores)serialises each namespace'sbaseback to raw JSON for upload to a translation service or diff-based change detection.Why nanostores
The reactive core (
computed,atom,task) is the only runtime dependency, and it was the right fit on every axis:createI18nentry point budgets ≤ 2.2 kB; the React bindings ≤ 500 B excluding React. A full i18n engine at that weight isn't possible with heavier primitives.computedeliminates manual invalidation. Each namespace store is acomputed([$locale, $resolved, $overrides], …). When the locale changes or overrides update, all dependent stores re-derive automatically. The alternative — subscribing manually and callingset()— is error-prone and leaks subscriptions in SSR.task()/allTasks()is built-in async coordination. Every locale fetch is registered as a nanostores task.allTasks()(already the SSR idiom for nanostores apps) resolves when all in-flight loads settle — zero plumbing on our side.@nanostores/i18nproved the pattern at production scale. We diverged purposefully (marker-based type inference, built-in override layer,messageFormatfor rich text) but the reactive foundation is validated.Architecture decisions
get(locale)is the seam between CDN-fetch and bundler chunkmessageFormat+walkASTdangerouslySetInnerHTML; React and string renderers share one recursive visitor/react)What this is not
@clerk/localizationslocale files (Phase 1–3, tracked separately).@clerk/uior@clerk/clerk-js(oncarp/i18n-mosaic).Test plan
turbo test --filter @clerk/i18n— all unit + type tests greenturbo build --filter @clerk/i18n— ESM + CJS +.d.tsoutputs cleanpnpm size --filter @clerk/i18n—createI18n≤ 2.2 kB,useStore + Message≤ 500 BmessageFormatrich text renders real elements in the React binding testSummary by CodeRabbit
@clerk/i18nreactive internationalization package with locale resolution, per-namespace message stores, async translation loading, and runtime localization overrides.messageFormat, pluralization/count,params), plus serialization viamessagesToJSON.useStore,<Message />,useMessage) with SSR-friendly translation loading support.@clerk/i18npackage README.