feat(i18n): introduce i18next-translation-dedupe#2482
feat(i18n): introduce i18next-translation-dedupe#2482luhc228 merged 1 commit intolynx-family:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds a new package Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes 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 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 |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
packages/i18n/i18next-custom-sections/src/plugin.ts (1)
117-142: Silent no-op when Lynx template plugin isn't exposed.If
api.useExposed(Symbol.for('LynxTemplatePlugin'))returns undefined (e.g., the user forgotpluginReactLynx/misordered plugins),pluginLynxI18nCustomSectionsreturns silently and extraction is never wired. The extractor's default rendered asset is then not skipped either, so users get default behavior with no diagnostic. Consider logging a warning viaapi.loggerso the misconfiguration is discoverable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/i18n/i18next-custom-sections/src/plugin.ts` around lines 117 - 142, When api.useExposed(Symbol.for('LynxTemplatePlugin')) returns undefined the setup function currently returns silently; update the setup in plugin.ts to emit a warning via api.logger (e.g., api.logger.warn) before returning so misconfiguration is discoverable. Specifically, in the setup method where templatePluginExposure and { LynxTemplatePlugin } are derived, add a clear warning message that references LynxTemplatePlugin / pluginLynxI18nCustomSections (or the plugin id 'lynx:i18n-custom-sections') explaining that the Lynx template plugin is not exposed and extraction wiring will be skipped, then return as before; do not change the early-return behavior, only add the diagnostic log.packages/i18n/i18next-custom-sections/tests/i18n-custom-sections.test.ts (2)
39-41: Temp dist directories leak across test runs.
mkdtempcreates a new directory underos.tmpdir()on every run and nothing ever removes it. Over time on CI/dev machines this accumulates non-trivial build output. Consider tracking the path and removing it inafterEach(e.g.,fs.rm(distRoot, { recursive: true, force: true })), ideally in atry/finallyso cleanup still runs on assertion failure.Also applies to: 78-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/i18n/i18next-custom-sections/tests/i18n-custom-sections.test.ts` around lines 39 - 41, Tests create temporary directories via mkdtemp(tmpdir(), ...) and never remove them; update the test to track the created distRoot(s) and clean them up in an afterEach hook by calling fs.rm(distRoot, { recursive: true, force: true }) (or fs.rmdir on older Node) inside a try/finally so cleanup always runs even on assertion failure; apply the same cleanup for the other temporary dir created around lines 78-79 and reference the distRoot variable and the afterEach hook when implementing.
103-112: Substring assertions on bundle contents are fragile.
mainThreadContent.includes('Hello world')will also match unrelated occurrences (e.g., a comment, a source-map path, or another string containing those words), and won't tell you why it matched if it fails. The Chinese literal is safer, but for'Hello world'consider a more targeted check, for example asserting the extracted translations object / JSON blob is not present, or scanning for a quoted occurrence ('"Hello world"'). Not blocking — current fixture is small enough that collisions are unlikely — just flagging for future-proofing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/i18n/i18next-custom-sections/tests/i18n-custom-sections.test.ts` around lines 103 - 112, The current substring checks using mainThreadContent.includes('Hello world') and backgroundContent.includes('Hello world') are fragile; change the assertions to look for a more targeted occurrence (e.g., a quoted string like '"Hello world"' or use a regex that matches a JSON/string literal) or parse the bundle to extract the translations object and assert the specific key/value is absent; update the tests that read files via readFileSync (mainThreadContent, backgroundContent) to use the quoted match or parsed-JSON assertion instead of plain includes to avoid false positives.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/feat-i18next-custom-sections.md:
- Around line 9-24: The example changeset uses locale keys "en-US" and "zh-CN"
but the plugin/test output and fixture use short keys like "en" and "zh"; update
the example under customSections -> i18next-translations -> content to use the
actual emitted locale keys ("en" and "zh") so the shape matches what the plugin
emits and what the integration test asserts (referencing customSections,
i18next-translations, and content in the diff).
In `@packages/i18n/i18next-custom-sections/package.json`:
- Around line 36-47: Move i18next from devDependencies into peerDependencies in
package.json so the exported types used by toI18nextResources and
loadTranslationsFromCustomSections resolve for consumers: remove the "i18next":
"26.0.6" entry from devDependencies and add the same version string under
peerDependencies (you may optionally keep it also in devDependencies for local
development/test builds, but ensure it's declared as a peerDependency so the
compiled .d.ts references are satisfied).
In `@packages/i18n/i18next-custom-sections/README.md`:
- Around line 29-48: Update the README text to use the actual custom section key
string used in code: replace the two occurrences of "i18n-translations" with
"i18next-translations" so the docs match the exported constant
I18N_TRANSLATIONS_SECTION_KEY and the behavior of
pluginLynxI18nCustomSections(); ensure the README examples (including the Custom
Section Contract list and any sample lynx.getCustomSectionSync(...) usage) use
"i18next-translations".
In `@packages/i18n/i18next-custom-sections/src/plugin.ts`:
- Around line 86-107: The beforeEncode hook currently only reads the first entry
name (entryNames[0]) so translations for other entries are skipped; update the
hooks.beforeEncode handler (BeforeEncodeArgs) to iterate over all
args.entryNames, collect/merge translations from this.store for each entryName
(using the same structure stored in afterExtract), and set
args.encodeData.customSections[I18N_TRANSLATIONS_SECTION_KEY] to the combined
translations object (or attach per-entry translations as appropriate) before
returning args; ensure you handle missing store entries (skip or merge safely)
and preserve existing encodeData.customSections.
In `@packages/i18n/i18next-custom-sections/src/runtime.ts`:
- Around line 15-17: The custom-lynx type and cast (LynxWithCustomSections /
getCustomSectionSync) unsafely assume the plugin returns a TranslationsByLocale
object; change the declared return type to unknown (or keep Cloneable) and add a
runtime guard before calling toI18nextResources: fetch the value via
lynx.getCustomSectionSync, verify it's an object (typeof x === 'object' && x !==
null) and that Object.entries can be used, and if not treat it as {} (or bail to
an empty TranslationsByLocale) so toI18nextResources always receives a plain
object; update the code paths around toI18nextResources and the cast to remove
the unsafe assertion.
In
`@packages/i18n/i18next-custom-sections/tests/fixtures/i18next-rspeedy-project/src/i18n.ts`:
- Around line 10-15: The i18n.init call is being invoked with its promise
discarded (void i18n.init(...)), which can let i18n.t run before resources are
ready; update the call to either await the promise or assign it to a variable
and handle errors (e.g., const initPromise = i18n.init(...).catch(...)) so
initialization completion is guaranteed before usage in App.tsx; locate the call
to i18n.init in this file and the usage in App.tsx and ensure
loadTranslationsFromCustomSections() remains the resources source while awaiting
or storing the init promise.
In `@packages/i18n/i18next-custom-sections/tsconfig.build.json`:
- Around line 9-10: The "exclude" pattern "__tests__/**" in tsconfig.build.json
does not match the actual test folder and is effectively dead because "include":
["src"] already restricts sources; update the file by either removing the
redundant "exclude" entry or replacing it with a correct pattern that matches
your layout (for example adjust to exclude "tests/**" or "src/**/*.test.ts") so
the "exclude" accurately reflects where tests live; look for the entries
"include": ["src"] and "exclude": ["__tests__/**"] to make the change.
---
Nitpick comments:
In `@packages/i18n/i18next-custom-sections/src/plugin.ts`:
- Around line 117-142: When api.useExposed(Symbol.for('LynxTemplatePlugin'))
returns undefined the setup function currently returns silently; update the
setup in plugin.ts to emit a warning via api.logger (e.g., api.logger.warn)
before returning so misconfiguration is discoverable. Specifically, in the setup
method where templatePluginExposure and { LynxTemplatePlugin } are derived, add
a clear warning message that references LynxTemplatePlugin /
pluginLynxI18nCustomSections (or the plugin id 'lynx:i18n-custom-sections')
explaining that the Lynx template plugin is not exposed and extraction wiring
will be skipped, then return as before; do not change the early-return behavior,
only add the diagnostic log.
In `@packages/i18n/i18next-custom-sections/tests/i18n-custom-sections.test.ts`:
- Around line 39-41: Tests create temporary directories via mkdtemp(tmpdir(),
...) and never remove them; update the test to track the created distRoot(s) and
clean them up in an afterEach hook by calling fs.rm(distRoot, { recursive: true,
force: true }) (or fs.rmdir on older Node) inside a try/finally so cleanup
always runs even on assertion failure; apply the same cleanup for the other
temporary dir created around lines 78-79 and reference the distRoot variable and
the afterEach hook when implementing.
- Around line 103-112: The current substring checks using
mainThreadContent.includes('Hello world') and backgroundContent.includes('Hello
world') are fragile; change the assertions to look for a more targeted
occurrence (e.g., a quoted string like '"Hello world"' or use a regex that
matches a JSON/string literal) or parse the bundle to extract the translations
object and assert the specific key/value is absent; update the tests that read
files via readFileSync (mainThreadContent, backgroundContent) to use the quoted
match or parsed-JSON assertion instead of plain includes to avoid false
positives.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9d2b0455-e586-4a42-b9ee-99b7624a5f59
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
.changeset/feat-i18next-custom-sections.md.github/i18n.instructions.mdeslint.config.jspackages/i18n/i18next-custom-sections/README.mdpackages/i18n/i18next-custom-sections/package.jsonpackages/i18n/i18next-custom-sections/src/constants.tspackages/i18n/i18next-custom-sections/src/index.tspackages/i18n/i18next-custom-sections/src/plugin.tspackages/i18n/i18next-custom-sections/src/runtime.tspackages/i18n/i18next-custom-sections/tests/fixtures/i18next-rspeedy-project/src/App.tsxpackages/i18n/i18next-custom-sections/tests/fixtures/i18next-rspeedy-project/src/i18n.tspackages/i18n/i18next-custom-sections/tests/fixtures/i18next-rspeedy-project/src/index.tsxpackages/i18n/i18next-custom-sections/tests/fixtures/i18next-rspeedy-project/src/locales/en.jsonpackages/i18n/i18next-custom-sections/tests/fixtures/i18next-rspeedy-project/src/locales/zh.jsonpackages/i18n/i18next-custom-sections/tests/fixtures/i18next-rspeedy-project/src/rspeedy-env.d.tspackages/i18n/i18next-custom-sections/tests/fixtures/i18next-rspeedy-project/src/tsconfig.jsonpackages/i18n/i18next-custom-sections/tests/fixtures/i18next-rspeedy-project/tsconfig.jsonpackages/i18n/i18next-custom-sections/tests/fixtures/i18next-rspeedy-project/tsconfig.node.jsonpackages/i18n/i18next-custom-sections/tests/i18n-custom-sections.test.tspackages/i18n/i18next-custom-sections/tsconfig.build.jsonpackages/i18n/i18next-custom-sections/tsconfig.jsonpackages/i18n/i18next-custom-sections/vitest.config.tspackages/i18n/tsconfig.jsonpnpm-workspace.yamltsconfig.jsonvitest.config.ts
There was a problem hiding this comment.
Pull request overview
Adds a new i18n helper package to integrate rsbuild-plugin-i18next-extractor with Lynx bundles by storing extracted translations in Lynx customSections, plus an integration test fixture and workspace wiring so it runs in the monorepo test/build setup.
Changes:
- Introduce
@lynx-js/i18next-custom-sections(plugin + runtime helpers) to inject extracted translations into LynxcustomSectionsand skip emitting the extractor’s default translations asset. - Add an integration test that builds a fixture Rspeedy project and asserts translations are moved to
tasm.jsoncustom sections and removed from JS bundles. - Wire the new
packages/i18n/*workspace into pnpm, TypeScript project references, Vitest projects, and ESLint ignores for fixtures.
Reviewed changes
Copilot reviewed 24 out of 27 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Includes i18n package Vitest projects in the root workspace runner. |
| tsconfig.json | Adds packages/i18n to TS project references. |
| pnpm-workspace.yaml | Adds packages/i18n/* to the pnpm workspace. |
| pnpm-lock.yaml | Locks dependencies for the new package and updated dependency graph. |
| packages/i18n/tsconfig.json | Creates a composite TS “umbrella” project referencing i18n subpackages. |
| packages/i18n/i18next-custom-sections/vitest.config.ts | Defines Vitest project config for the new package. |
| packages/i18n/i18next-custom-sections/tsconfig.json | Package-local TS config for typechecking sources/tests. |
| packages/i18n/i18next-custom-sections/tsconfig.build.json | Package build TS config emitting to lib/. |
| packages/i18n/i18next-custom-sections/tests/i18n-custom-sections.test.ts | Integration test validating custom section injection and removal of inline translations. |
| packages/i18n/i18next-custom-sections/tests/fixtures/i18next-rspeedy-project/tsconfig.node.json | Fixture TS config for node-side config/build files. |
| packages/i18n/i18next-custom-sections/tests/fixtures/i18next-rspeedy-project/tsconfig.json | Fixture root TS config. |
| packages/i18n/i18next-custom-sections/tests/fixtures/i18next-rspeedy-project/src/tsconfig.json | Fixture src TS config (ReactLynx JSX + bundler resolution). |
| packages/i18n/i18next-custom-sections/tests/fixtures/i18next-rspeedy-project/src/rspeedy-env.d.ts | Fixture env typing for Rspeedy client types. |
| packages/i18n/i18next-custom-sections/tests/fixtures/i18next-rspeedy-project/src/locales/zh.json | Fixture translation resource (zh). |
| packages/i18n/i18next-custom-sections/tests/fixtures/i18next-rspeedy-project/src/locales/en.json | Fixture translation resource (en). |
| packages/i18n/i18next-custom-sections/tests/fixtures/i18next-rspeedy-project/src/index.tsx | Fixture app entry. |
| packages/i18n/i18next-custom-sections/tests/fixtures/i18next-rspeedy-project/src/i18n.ts | Fixture i18next init loading resources from custom sections. |
| packages/i18n/i18next-custom-sections/tests/fixtures/i18next-rspeedy-project/src/App.tsx | Fixture component calling i18n.t. |
| packages/i18n/i18next-custom-sections/src/runtime.ts | Runtime helper to read translations from custom sections and convert to i18next Resource. |
| packages/i18n/i18next-custom-sections/src/plugin.ts | Rspeedy/Rspack plugin to capture extractor output and write to customSections. |
| packages/i18n/i18next-custom-sections/src/index.ts | Public entry re-exporting constants, runtime helpers, and plugin. |
| packages/i18n/i18next-custom-sections/src/constants.ts | Defines the custom section key constant. |
| packages/i18n/i18next-custom-sections/package.json | New package manifest, exports, scripts, and deps. |
| packages/i18n/i18next-custom-sections/README.md | Usage docs for the new package. |
| eslint.config.js | Excludes i18n fixture mini-apps from root ESLint coverage. |
| .github/i18n.instructions.md | Adds repo guidance for treating i18n fixtures as build inputs rather than production source. |
| .changeset/feat-i18next-custom-sections.md | Changeset announcing the new package. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9e12322 to
7ce26de
Compare
🦋 Changeset detectedLatest commit: 413f52d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
7ce26de to
b34e7a3
Compare
41f4386 to
72b66b6
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/i18n/i18next-translation-dedupe/src/plugin.ts (1)
86-107:⚠️ Potential issue | 🟠 MajorMerge translations for all encoded entries.
Line 89 only reads
args.entryNames[0], so templates encoded with multiple entry names can miss translations for entries 1..N even though the store is populated per entry. Iterate all names and merge their locale maps before writing the custom section.Suggested fix
- const entryName = args.entryNames[0]; - - if (!entryName) { - return args; - } - - const translationsByLocale = this.store.get(entryName); - - if (!translationsByLocale) { + const translationsByLocale: AfterExtractPayload[ + 'extractedTranslationsByLocale' + ] = {}; + + for (const entryName of args.entryNames) { + const entryTranslationsByLocale = this.store.get(entryName); + + if (!entryTranslationsByLocale) { + continue; + } + + for (const [locale, translations] of Object.entries( + entryTranslationsByLocale, + )) { + translationsByLocale[locale] = { + ...(translationsByLocale[locale] ?? {}), + ...translations, + }; + } + } + + if (Object.keys(translationsByLocale).length === 0) { return args; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/i18n/i18next-translation-dedupe/src/plugin.ts` around lines 86 - 107, The current beforeEncode handler only uses the first entry name (args.entryNames[0]) so translations for additional encoded entries are skipped; update the hooks.beforeEncode.tap callback (BeforeEncodeArgs) to iterate over args.entryNames, retrieve each entry's locale map from this.store (using this.store.get(entryName)), and merge all found translations into a single object before assigning args.encodeData.customSections[I18N_TRANSLATIONS_SECTION_KEY] = { content: mergedTranslations }; keep existing early returns for missing store entries but ensure you accumulate translations from every entry name rather than taking only the first.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/i18n/i18next-translation-dedupe/src/plugin.ts`:
- Around line 86-107: The current beforeEncode handler only uses the first entry
name (args.entryNames[0]) so translations for additional encoded entries are
skipped; update the hooks.beforeEncode.tap callback (BeforeEncodeArgs) to
iterate over args.entryNames, retrieve each entry's locale map from this.store
(using this.store.get(entryName)), and merge all found translations into a
single object before assigning
args.encodeData.customSections[I18N_TRANSLATIONS_SECTION_KEY] = { content:
mergedTranslations }; keep existing early returns for missing store entries but
ensure you accumulate translations from every entry name rather than taking only
the first.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c7aa05c5-071b-4ec4-b6da-fd3e44c77658
📒 Files selected for processing (24)
.changeset/feat-i18next-custom-sections.md.github/i18n.instructions.mdCODEOWNERSeslint.config.jspackages/i18n/i18next-translation-dedupe/README.mdpackages/i18n/i18next-translation-dedupe/package.jsonpackages/i18n/i18next-translation-dedupe/src/constants.tspackages/i18n/i18next-translation-dedupe/src/index.tspackages/i18n/i18next-translation-dedupe/src/plugin.tspackages/i18n/i18next-translation-dedupe/src/runtime.tspackages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/App.tsxpackages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/i18n.tspackages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/index.tsxpackages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/locales/en.jsonpackages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/locales/zh.jsonpackages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/rspeedy-env.d.tspackages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/tsconfig.jsonpackages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/tsconfig.jsonpackages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/tsconfig.node.jsonpackages/i18n/i18next-translation-dedupe/tests/i18next-translation-dedupe.test.tspackages/i18n/i18next-translation-dedupe/tsconfig.build.jsonpackages/i18n/i18next-translation-dedupe/tsconfig.jsonpackages/i18n/i18next-translation-dedupe/vitest.config.tspackages/i18n/tsconfig.json
✅ Files skipped from review due to trivial changes (17)
- packages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/locales/zh.json
- packages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/index.tsx
- packages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/locales/en.json
- CODEOWNERS
- packages/i18n/i18next-translation-dedupe/src/constants.ts
- packages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/tsconfig.json
- packages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/tsconfig.node.json
- .github/i18n.instructions.md
- packages/i18n/i18next-translation-dedupe/tsconfig.json
- packages/i18n/tsconfig.json
- eslint.config.js
- packages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/rspeedy-env.d.ts
- packages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/i18n.ts
- packages/i18n/i18next-translation-dedupe/vitest.config.ts
- packages/i18n/i18next-translation-dedupe/tsconfig.build.json
- packages/i18n/i18next-translation-dedupe/src/index.ts
- packages/i18n/i18next-translation-dedupe/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- .changeset/feat-i18next-custom-sections.md
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Merging this PR will improve performance by 16.89%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | transform 1000 view elements |
46.8 ms | 40 ms | +16.89% |
Comparing luhc228:i18n-custom-section-plugin (413f52d) with main (25b09e9)
Footnotes
-
26 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
React External#610 Bundle Size — 583.28KiB (0%).413f52d(current) vs 25b09e9 main#604(baseline) Bundle metrics
|
| Current #610 |
Baseline #604 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
17 |
17 |
|
5 |
5 |
|
8.59% |
8.59% |
|
0 |
0 |
|
0 |
0 |
Bundle analysis report Branch luhc228:i18n-custom-section-plug... Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#625 Bundle Size — 195.57KiB (0%).413f52d(current) vs 25b09e9 main#619(baseline) Bundle metrics
|
| Current #625 |
Baseline #619 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
173 |
173 |
|
66 |
66 |
|
44% |
44% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #625 |
Baseline #619 |
|
|---|---|---|
111.23KiB |
111.23KiB |
|
84.34KiB |
84.34KiB |
Bundle analysis report Branch luhc228:i18n-custom-section-plug... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#7492 Bundle Size — 224.41KiB (0%).413f52d(current) vs 25b09e9 main#7486(baseline) Bundle metrics
|
| Current #7492 |
Baseline #7486 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
179 |
179 |
|
69 |
69 |
|
44.51% |
44.51% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #7492 |
Baseline #7486 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
78.65KiB |
78.65KiB |
Bundle analysis report Branch luhc228:i18n-custom-section-plug... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9066 Bundle Size — 898.7KiB (0%).413f52d(current) vs 25b09e9 main#9060(baseline) Bundle metrics
Bundle size by type
|
| Current #9066 |
Baseline #9060 |
|
|---|---|---|
494.47KiB |
494.47KiB |
|
402.02KiB |
402.02KiB |
|
2.22KiB |
2.22KiB |
Bundle analysis report Branch luhc228:i18n-custom-section-plug... Project dashboard
Generated by RelativeCI Documentation Report issue
72b66b6 to
413f52d
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/i18n/i18next-translation-dedupe/src/plugin.ts (1)
86-107:⚠️ Potential issue | 🟠 MajorTranslations still only injected for the first entry.
Same concern as the prior review:
args.entryNames[0]is used, so multi-entry templates (e.g.,chunks: ['a', 'a:main-thread']) only get translations for the first entry. Thestoreis populated per-entry inafterExtract, but entries 1..N are silently dropped here. Iterate overargs.entryNamesand merge/select translations (or pick the non-:main-threadchunk) instead of hard-coding index 0.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/i18n/i18next-translation-dedupe/src/plugin.ts` around lines 86 - 107, The current beforeEncode hook uses args.entryNames[0], so only the first entry's translations are injected; update hooks.beforeEncode.tap (BeforeEncodeArgs handler) to iterate over args.entryNames, for each name call this.store.get(entryName) and merge the resulting translationsByLocale into a single object (or prefer/select the non-":main-thread" entry when duplicates exist) before assigning args.encodeData.customSections[I18N_TRANSLATIONS_SECTION_KEY] = { content: mergedTranslations }; ensure this mirrors how afterExtract populates the store so all entries' translations are included rather than dropping entries 1..N.
🧹 Nitpick comments (2)
packages/i18n/i18next-translation-dedupe/src/plugin.ts (1)
1-3: Nit: copyright year.This is a new file added in 2026; consider updating the header year to match repo convention for new files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/i18n/i18next-translation-dedupe/src/plugin.ts` around lines 1 - 3, Update the file header in packages/i18n/i18next-translation-dedupe/src/plugin.ts to reflect the current year (2026) instead of 2024 by changing the copyright year in the top comment block; ensure the header still matches the repository's license line and formatting.packages/i18n/i18next-translation-dedupe/README.md (1)
44-61: Document locale key format constraint.The example uses
en-US/zh-CN, but the locale keys are whateverrsbuild-plugin-i18next-extractoremits per the user'si18next-cliconfig (oftenen/zh). The sibling test fixture even assertsen/zh. Consider clarifying that locale keys are pass-through from the extractor so users aren't misled into thinking the plugin normalizes to BCP-47 tags.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/i18n/i18next-translation-dedupe/README.md` around lines 44 - 61, Clarify that the custom section key "i18next-translations" does not normalize locale keys and that the map keys are passed through from the extractor (rsbuild-plugin-i18next-extractor) according to the user's i18next-cli configuration (e.g., may be "en"/"zh" rather than BCP-47 "en-US"/"zh-CN"); update the README example and wording to state explicitly that locale keys are emitter-provided/passthrough and reference the extractor and i18next-cli so readers understand the source of the key format (the test fixture also asserts "en"/"zh").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/i18n/i18next-translation-dedupe/src/plugin.ts`:
- Around line 86-107: The current beforeEncode hook uses args.entryNames[0], so
only the first entry's translations are injected; update hooks.beforeEncode.tap
(BeforeEncodeArgs handler) to iterate over args.entryNames, for each name call
this.store.get(entryName) and merge the resulting translationsByLocale into a
single object (or prefer/select the non-":main-thread" entry when duplicates
exist) before assigning
args.encodeData.customSections[I18N_TRANSLATIONS_SECTION_KEY] = { content:
mergedTranslations }; ensure this mirrors how afterExtract populates the store
so all entries' translations are included rather than dropping entries 1..N.
---
Nitpick comments:
In `@packages/i18n/i18next-translation-dedupe/README.md`:
- Around line 44-61: Clarify that the custom section key "i18next-translations"
does not normalize locale keys and that the map keys are passed through from the
extractor (rsbuild-plugin-i18next-extractor) according to the user's i18next-cli
configuration (e.g., may be "en"/"zh" rather than BCP-47 "en-US"/"zh-CN");
update the README example and wording to state explicitly that locale keys are
emitter-provided/passthrough and reference the extractor and i18next-cli so
readers understand the source of the key format (the test fixture also asserts
"en"/"zh").
In `@packages/i18n/i18next-translation-dedupe/src/plugin.ts`:
- Around line 1-3: Update the file header in
packages/i18n/i18next-translation-dedupe/src/plugin.ts to reflect the current
year (2026) instead of 2024 by changing the copyright year in the top comment
block; ensure the header still matches the repository's license line and
formatting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7c21f2a7-120d-4f72-addb-356a93c0608d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
.changeset/feat-i18next-custom-sections.md.github/i18n.instructions.mdCODEOWNERSeslint.config.jspackages/i18n/i18next-translation-dedupe/README.mdpackages/i18n/i18next-translation-dedupe/package.jsonpackages/i18n/i18next-translation-dedupe/src/constants.tspackages/i18n/i18next-translation-dedupe/src/index.tspackages/i18n/i18next-translation-dedupe/src/plugin.tspackages/i18n/i18next-translation-dedupe/src/runtime.tspackages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/App.tsxpackages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/i18n.tspackages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/index.tsxpackages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/locales/en.jsonpackages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/locales/zh.jsonpackages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/rspeedy-env.d.tspackages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/tsconfig.jsonpackages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/tsconfig.jsonpackages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/tsconfig.node.jsonpackages/i18n/i18next-translation-dedupe/tests/i18next-translation-dedupe.test.tspackages/i18n/i18next-translation-dedupe/tsconfig.build.jsonpackages/i18n/i18next-translation-dedupe/tsconfig.jsonpackages/i18n/i18next-translation-dedupe/vitest.config.tspackages/i18n/tsconfig.jsonpnpm-workspace.yamltsconfig.jsonvitest.config.ts
✅ Files skipped from review due to trivial changes (21)
- packages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/locales/zh.json
- eslint.config.js
- packages/i18n/i18next-translation-dedupe/src/constants.ts
- tsconfig.json
- packages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/index.tsx
- pnpm-workspace.yaml
- packages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/tsconfig.node.json
- packages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/locales/en.json
- .github/i18n.instructions.md
- packages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/tsconfig.json
- packages/i18n/i18next-translation-dedupe/tsconfig.json
- packages/i18n/tsconfig.json
- packages/i18n/i18next-translation-dedupe/vitest.config.ts
- packages/i18n/i18next-translation-dedupe/src/index.ts
- packages/i18n/i18next-translation-dedupe/tsconfig.build.json
- packages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/rspeedy-env.d.ts
- packages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/i18n.ts
- packages/i18n/i18next-translation-dedupe/tests/i18next-translation-dedupe.test.ts
- CODEOWNERS
- vitest.config.ts
- packages/i18n/i18next-translation-dedupe/package.json
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/App.tsx
- .changeset/feat-i18next-custom-sections.md
- packages/i18n/i18next-translation-dedupe/tests/fixtures/i18next-rspeedy-project/src/tsconfig.json
- packages/i18n/i18next-translation-dedupe/src/runtime.ts
Summary by CodeRabbit
Checklist