Fix JSON catalogue declarations, loading, and empty-locale fallback - #42
Conversation
🦋 Changeset detectedLatest commit: c413729 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
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.
|
github-actions
Bot
added
dependencies
|
Warning Review limit reached
Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughSayKit updates catalogue declaration naming, JSON loading, fallback preservation, example applications, documentation, and the React package default build entry. ChangesCatalogue declarations and fallback handling
JSON catalogue loading
Example catalogue migrations
React package entry output
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/config/src/features/catalogue/path.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider using
jointo preserve path relativity.Using
resolveconverts relative paths to absolute ones based on the current working directory. Usingjoinensures that the returned path retains the same relative or absolute nature as the inputcataloguePath, which is often the expected behaviour for a sibling-path utility.♻️ Proposed refactor
Update the import:
-import { parse, resolve } from 'node:path'; +import { join, parse, resolve } from 'node:path';Use
joinfor the path construction:export function declarationPathFor(cataloguePath: string) { const { dir, name, ext } = parse(cataloguePath); - return resolve(dir, `${name}.d${ext}.ts`); + return join(dir, `${name}.d${ext}.ts`); }Also applies to: 14-26
🤖 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/config/src/features/catalogue/path.ts` at line 1, Replace resolve with join in the path utility and update the node:path import accordingly. Ensure the sibling-path construction preserves whether cataloguePath is relative or absolute while retaining the existing parsing and path-segment behavior.
🤖 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/config/src/features/catalogue/record.ts`:
- Around line 46-51: The fallback selection in the record-building logic must
preserve an existing less-specific translation when a PO entry has an empty
translation. Update the value assignment around message.translation and
message.message to use message.message only when record[key] is not already set,
while retaining the current behavior for genuinely missing record entries and
skipping empty values.
---
Nitpick comments:
In `@packages/config/src/features/catalogue/path.ts`:
- Line 1: Replace resolve with join in the path utility and update the node:path
import accordingly. Ensure the sibling-path construction preserves whether
cataloguePath is relative or absolute while retaining the existing parsing and
path-segment behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4481958c-db03-4c85-965f-fda4363af625
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (46)
.changeset/declaration-file-naming.md.changeset/empty-translation-fallback.md.changeset/json-catalogue-load.md.changeset/react-default-entry-filename.mdexamples/README.mdexamples/carbon/package.jsonexamples/carbon/saykit.config.tsexamples/carbon/src/i18n.tsexamples/carbon/src/locales/de.d.json.tsexamples/carbon/src/locales/de.jsonexamples/carbon/src/locales/en.d.json.tsexamples/carbon/src/locales/en.jsonexamples/carbon/src/locales/en.poexamples/carbon/src/locales/fr.d.json.tsexamples/carbon/src/locales/fr.jsonexamples/carbon/src/locales/fr.poexamples/nextjs/src/locales/en.d.po.tsexamples/nextjs/src/locales/fr.d.po.tsexamples/tanstack-start/package.jsonexamples/tanstack-start/saykit.config.tsexamples/tanstack-start/src/i18n.tsexamples/tanstack-start/src/locales/en.d.json.tsexamples/tanstack-start/src/locales/en.jsonexamples/tanstack-start/src/locales/en.poexamples/tanstack-start/src/locales/en.po.d.tsexamples/tanstack-start/src/locales/fr.d.json.tsexamples/tanstack-start/src/locales/fr.jsonexamples/tanstack-start/src/locales/fr.poexamples/tanstack-start/src/locales/fr.po.d.tsexamples/tanstack-start/src/routes/{-$locale}/index.tsxpackages/config/src/features/catalogue/path.tspackages/config/src/features/catalogue/record.test.tspackages/config/src/features/catalogue/record.tspackages/config/src/features/catalogue/storage.test.tspackages/config/src/features/catalogue/storage.tspackages/config/src/features/workers/extract-worker.test.tspackages/integration-react/package.jsonpackages/integration-react/tsdown.config.tspackages/plugin-unplugin/src/index.test.tspackages/plugin-unplugin/src/index.tstsconfig.base.jsonwebsite/content/core-concepts/extraction.mdxwebsite/content/getting-started/quickstart.mdxwebsite/content/guides/custom-formatter.mdxwebsite/content/guides/typed-messages.mdxwebsite/content/reference/cli.mdx
💤 Files with no reviewable changes (6)
- examples/carbon/src/locales/fr.po
- examples/carbon/src/locales/en.po
- examples/tanstack-start/src/locales/fr.po
- examples/tanstack-start/src/locales/en.po
- examples/tanstack-start/src/locales/en.po.d.ts
- examples/tanstack-start/src/locales/fr.po.d.ts
1 participant
Started as a fix for JSON catalogue declarations being silently ignored by TypeScript, and grew to cover the three other things that were broken along the same path.
Declaration file naming (
@saykit/config, minor)TypeScript resolves
./locales/en.jsonby stripping the extension it recognises and looking foren.d.json.ts. Theen.json.d.tsform is only consulted for extensions the resolver doesn't know, so the old naming worked for.pobut was ignored for.json— JSON catalogues fell through to the real file and got the literal object type inferred from their current contents instead of the declaredRecord<string, string>.Catalogue declarations are now emitted as
{locale}.d.{extension}.ts. Docs updated across five pages, including a new section explaining the resolution rule and theallowArbitraryExtensionsrequirement for non-JSON extensions.Breaking for existing projects: leftover
{locale}.{extension}.d.tsfiles are not migrated automatically and should be deleted by hand.JSON catalogue loading (
unplugin-saykit, patch)A
.jsonid is interpreted as JSON by whatever runs after the plugin — Rollup's json plugin parses the module code, webpack/rspack assign thejsonmodule type, esbuild picks its loader from the extension. Emittingexport default {...}for those ids was a build-time syntax error. Non-JSON catalogues still get the ESM wrapper, which previously had no test coverage and now does.Empty-locale fallback (
@saykit/config, patch)Formats with a single value slot per key (JSON, unlike PO's separate
msgid/msgstr) report an untranslated key as an empty message and an empty translation. Record assembly wrote that empty value over the string already resolved from a less specific locale, so every untranslated JSON locale shipped blank strings instead of falling back to the source. Empty values are now skipped.React default entry filename (
@saykit/react, patch)Frameworks that guard their server environment against client-only modules commonly deny the
**/*.client.*pattern. TanStack Start does, and since its SSR environment doesn't set thereact-servercondition,@saykit/reactresolved todist/index.client.mjsand was rejected on its filename alone — even though a"use client"module is meant to be server-rendered. The default entry is nowdist/index.mjs; thereact-servercondition still resolves todist/index.server.mjs.Examples
Reworked so each covers a distinct path, with a new
examples/README.mddocumenting the matrix:json()plainunplugin-saykit/rolldownen,fr,de(empty)po()babel-plugin-saykiten,frjson({ dialect: 'arb' })unplugin-saykit/viteen,frExisting French translations were carried across formats rather than retyped. Carbon's untranslated
delocale is the end-to-end check for the fallback fix: its built chunk should contain English strings, not empty ones.Verification
checkpasses for all three examples and@saykit/reactbuildpasses for all three examples — tanstack-start's build was failing before this branch and now succeedsKnown gap, not addressed
@saykit/transform-jsxpasses empty arrays for comments and source references where@saykit/transform-jsextracts both, so no JSX example can carry atranslators:comment in its catalogue. Noted inexamples/README.md.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation