✨Updated theme i18n to use i18next#24454
Merged
cathysarisky merged 30 commits intomainfrom Dec 9, 2025
Merged
Conversation
Contributor
|
""" WalkthroughA new beta feature called "themeTranslation" was introduced across the codebase. In the React component responsible for displaying beta feature toggles, an entry for "Updated theme translation (beta)" was added, allowing users to enable or disable this feature. The description specifies that it switches theme translation to use i18next. Correspondingly, the backend list of public beta features was updated to include "themeTranslation" in the array of available feature flags. The translation helper function was modified to conditionally use the new i18next-based translation service when the feature flag is enabled. Additionally, a new module exporting an instance of the i18next-based ThemeI18n class was added, and this instance was exposed through the handlebars services module. The i18n initialization function was extended to support a new "theme" namespace with file-based locale loading from theme paths. The theme engine's active theme service was updated to initialize the new translation service conditionally based on the feature flag. An event listener was added to reinitialize theme translation when the lab setting changes. A new unit test suite was added to cover the ThemeI18n class. No other logic or exported entities were changed. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/admin-x-settings/src/components/settings/advanced/labs/BetaFeatures.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "/apps/admin-x-settings".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react-hooks" was referenced from the config file in "apps/admin-x-settings/.eslintrc.cjs". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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: 0
🧹 Nitpick comments (1)
apps/admin-x-settings/src/components/settings/advanced/labs/BetaFeatures.tsx (1)
62-65: Consider adding a docs link for consistencyMost other beta-feature blurbs end with a “Learn more ↗︎” link, giving users a quick jump-off to the relevant help article. Adding one here would keep the UI consistent and help users understand the new translation flow.
- detail={<>Enable theme translation using i18next instead of the old translation package.</>} + detail={<> + Enable theme translation using i18next instead of the old translation package. + <a className='text-green' + href="https://ghost.org/help/theme-translation/" + rel="noopener noreferrer" + target="_blank"> + Learn more → + </a> + </>}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
apps/admin-x-settings/src/components/settings/advanced/labs/BetaFeatures.tsx(1 hunks)ghost/core/core/shared/labs.js(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-20T21:08:21.026Z
Learning: In the Ghost project, translation files (ghost/i18n/locales/*/*.json) commonly have blank values for translations, and this is normal behavior that should not be flagged in reviews. These empty translations will be filled in separate PRs.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23070
File: ghost/i18n/locales/hu/comments.json:45-45
Timestamp: 2025-04-30T16:27:03.542Z
Learning: In the Ghost project, new translation string keys are intentionally added with empty values initially and will be translated in a separate PR. Do not report missing translations for entirely new string additions.
Learnt from: cathysarisky
PR: TryGhost/Ghost#22175
File: ghost/i18n/locales/pt-BR/portal.json:18-18
Timestamp: 2025-02-12T20:09:12.731Z
Learning: In the Ghost project, new translation keys are first added with empty string values, and the actual translations are added later in separate PRs to maintain better organization of translation work.
Learnt from: kevinansfield
PR: TryGhost/Ghost#23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-06T00:47:48.488Z
Learning: The theme i18n initialization in Ghost only happens during server boot, theme changes, and language changes - not on every page load or `t` helper call. The initialization check in the `t` helper is a safeguard that doesn't execute during normal page rendering once the theme is loaded.
apps/admin-x-settings/src/components/settings/advanced/labs/BetaFeatures.tsx (2)
Learnt from: kevinansfield
PR: TryGhost/Ghost#23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
Learnt from: sagzy
PR: TryGhost/Ghost#24346
File: apps/admin-x-settings/src/components/settings/growth/Network.tsx:8-12
Timestamp: 2025-07-14T12:20:35.268Z
Learning: The Network component toggle in `apps/admin-x-settings/src/components/settings/growth/Network.tsx` is intentionally implemented as static UI with a no-op onChange handler, as part of a UI-first development approach before connecting actual ActivityPub functionality.
ghost/core/core/shared/labs.js (3)
Learnt from: kevinansfield
PR: TryGhost/Ghost#23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23070
File: ghost/i18n/locales/hu/comments.json:45-45
Timestamp: 2025-04-30T16:27:03.542Z
Learning: In the Ghost project, new translation string keys are intentionally added with empty values initially and will be translated in a separate PR. Do not report missing translations for entirely new string additions.
Learnt from: cathysarisky
PR: TryGhost/Ghost#22175
File: ghost/i18n/locales/pt-BR/portal.json:18-18
Timestamp: 2025-02-12T20:09:12.731Z
Learning: In the Ghost project, new translation keys are first added with empty string values, and the actual translations are added later in separate PRs to maintain better organization of translation work.
🔇 Additional comments (1)
ghost/core/core/shared/labs.js (1)
32-39:themeTranslationflag correctly registeredThe flag is added to
PUBLIC_BETA_FEATURES, so it will automatically be:
• exposed via the Labs API
• included inWRITABLE_KEYS_ALLOWLIST
• returned bygetAllFlags()No functional issues spotted.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
ghost/core/core/frontend/helpers/t.js(2 hunks)ghost/core/core/frontend/services/handlebars.js(1 hunks)ghost/core/core/frontend/services/theme-engine/i18next/index.js(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- ghost/core/core/frontend/services/theme-engine/i18next/index.js
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-20T21:08:21.026Z
Learning: In the Ghost project, translation files (ghost/i18n/locales/*/*.json) commonly have blank values for translations, and this is normal behavior that should not be flagged in reviews. These empty translations will be filled in separate PRs.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23070
File: ghost/i18n/locales/hu/comments.json:45-45
Timestamp: 2025-04-30T16:27:03.542Z
Learning: In the Ghost project, new translation string keys are intentionally added with empty values initially and will be translated in a separate PR. Do not report missing translations for entirely new string additions.
Learnt from: cathysarisky
PR: TryGhost/Ghost#22175
File: ghost/i18n/locales/pt-BR/portal.json:18-18
Timestamp: 2025-02-12T20:09:12.731Z
Learning: In the Ghost project, new translation keys are first added with empty string values, and the actual translations are added later in separate PRs to maintain better organization of translation work.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-06T00:47:48.488Z
Learning: The theme i18n initialization in Ghost only happens during server boot, theme changes, and language changes - not on every page load or `t` helper call. The initialization check in the `t` helper is a safeguard that doesn't execute during normal page rendering once the theme is loaded.
Learnt from: kevinansfield
PR: TryGhost/Ghost#23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-01-29T15:27:29.391Z
Learning: When reviewing translation files in ghost/i18n/locales, translators should only modify the translated values and must not change the keys or placeholder styles (e.g., %%{status}%%, {site}, etc.) as they serve specific purposes like Mailgun string replacement.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23161
File: ghost/i18n/lib/i18n.js:107-144
Timestamp: 2025-05-14T14:39:09.136Z
Learning: In Ghost's i18n system, translation keys are the actual English phrases, so when a translation is missing, displaying the key itself serves as the English fallback, eliminating the need for explicit fallback resource loading.
ghost/core/core/frontend/services/handlebars.js (2)
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-06T00:47:48.488Z
Learning: The theme i18n initialization in Ghost only happens during server boot, theme changes, and language changes - not on every page load or `t` helper call. The initialization check in the `t` helper is a safeguard that doesn't execute during normal page rendering once the theme is loaded.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-01-29T15:27:29.391Z
Learning: When reviewing translation files in ghost/i18n/locales, translators should only modify the translated values and must not change the keys or placeholder styles (e.g., %%{status}%%, {site}, etc.) as they serve specific purposes like Mailgun string replacement.
ghost/core/core/frontend/helpers/t.js (8)
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-06T00:47:48.488Z
Learning: The theme i18n initialization in Ghost only happens during server boot, theme changes, and language changes - not on every page load or `t` helper call. The initialization check in the `t` helper is a safeguard that doesn't execute during normal page rendering once the theme is loaded.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-01-29T15:27:29.391Z
Learning: When reviewing translation files in ghost/i18n/locales, translators should only modify the translated values and must not change the keys or placeholder styles (e.g., %%{status}%%, {site}, etc.) as they serve specific purposes like Mailgun string replacement.
Learnt from: kevinansfield
PR: TryGhost/Ghost#23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-20T21:08:21.026Z
Learning: In the Ghost project, translation files (ghost/i18n/locales/*/*.json) commonly have blank values for translations, and this is normal behavior that should not be flagged in reviews. These empty translations will be filled in separate PRs.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23070
File: ghost/i18n/locales/hu/comments.json:45-45
Timestamp: 2025-04-30T16:27:03.542Z
Learning: In the Ghost project, new translation string keys are intentionally added with empty values initially and will be translated in a separate PR. Do not report missing translations for entirely new string additions.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23161
File: ghost/i18n/lib/i18n.js:107-144
Timestamp: 2025-05-14T14:39:09.136Z
Learning: In Ghost's i18n system, translation keys are the actual English phrases, so when a translation is missing, displaying the key itself serves as the English fallback, eliminating the need for explicit fallback resource loading.
Learnt from: cathysarisky
PR: TryGhost/Ghost#22175
File: ghost/i18n/locales/lt/portal.json:16-16
Timestamp: 2025-02-12T20:23:47.777Z
Learning: When reviewing localization changes, verify the actual history of translations before flagging potential deletions. Empty translation strings may be intentional placeholders rather than deleted content.
Learnt from: cathysarisky
PR: TryGhost/Ghost#22175
File: ghost/i18n/locales/pt-BR/portal.json:18-18
Timestamp: 2025-02-12T20:09:12.731Z
Learning: In the Ghost project, new translation keys are first added with empty string values, and the actual translations are added later in separate PRs to maintain better organization of translation work.
🧬 Code Graph Analysis (1)
ghost/core/core/frontend/services/handlebars.js (1)
ghost/core/core/frontend/helpers/t.js (2)
require(13-13)require(14-14)
🔇 Additional comments (2)
ghost/core/core/frontend/services/handlebars.js (1)
22-22: LGTM! Clean addition of the new i18next service.The new
themeI18nextexport follows the same pattern as the existingthemeI18n, allowing both translation systems to coexist during the feature flag transition.ghost/core/core/frontend/helpers/t.js (1)
14-15: LGTM! Proper imports for the new translation system.The imports for both the new
themeI18nextservice andlabsmodule are correctly added to support the feature flag implementation.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ghost/core/core/frontend/services/theme-engine/i18next/ThemeI18n.js(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-20T21:08:21.026Z
Learning: In the Ghost project, translation files (ghost/i18n/locales/*/*.json) commonly have blank values for translations, and this is normal behavior that should not be flagged in reviews. These empty translations will be filled in separate PRs.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23070
File: ghost/i18n/locales/hu/comments.json:45-45
Timestamp: 2025-04-30T16:27:03.542Z
Learning: In the Ghost project, new translation string keys are intentionally added with empty values initially and will be translated in a separate PR. Do not report missing translations for entirely new string additions.
Learnt from: cathysarisky
PR: TryGhost/Ghost#22175
File: ghost/i18n/locales/pt-BR/portal.json:18-18
Timestamp: 2025-02-12T20:09:12.731Z
Learning: In the Ghost project, new translation keys are first added with empty string values, and the actual translations are added later in separate PRs to maintain better organization of translation work.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-06T00:47:48.488Z
Learning: The theme i18n initialization in Ghost only happens during server boot, theme changes, and language changes - not on every page load or `t` helper call. The initialization check in the `t` helper is a safeguard that doesn't execute during normal page rendering once the theme is loaded.
Learnt from: kevinansfield
PR: TryGhost/Ghost#23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-01-29T15:27:29.391Z
Learning: When reviewing translation files in ghost/i18n/locales, translators should only modify the translated values and must not change the keys or placeholder styles (e.g., %%{status}%%, {site}, etc.) as they serve specific purposes like Mailgun string replacement.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23161
File: ghost/i18n/lib/i18n.js:107-144
Timestamp: 2025-05-14T14:39:09.136Z
Learning: In Ghost's i18n system, translation keys are the actual English phrases, so when a translation is missing, displaying the key itself serves as the English fallback, eliminating the need for explicit fallback resource loading.
ghost/core/core/frontend/services/theme-engine/i18next/ThemeI18n.js (8)
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-06T00:47:48.488Z
Learning: The theme i18n initialization in Ghost only happens during server boot, theme changes, and language changes - not on every page load or `t` helper call. The initialization check in the `t` helper is a safeguard that doesn't execute during normal page rendering once the theme is loaded.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23161
File: ghost/i18n/lib/i18n.js:107-144
Timestamp: 2025-05-14T14:39:09.136Z
Learning: In Ghost's i18n system, translation keys are the actual English phrases, so when a translation is missing, displaying the key itself serves as the English fallback, eliminating the need for explicit fallback resource loading.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-01-29T15:27:29.391Z
Learning: When reviewing translation files in ghost/i18n/locales, translators should only modify the translated values and must not change the keys or placeholder styles (e.g., %%{status}%%, {site}, etc.) as they serve specific purposes like Mailgun string replacement.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23070
File: ghost/i18n/locales/hu/comments.json:45-45
Timestamp: 2025-04-30T16:27:03.542Z
Learning: In the Ghost project, new translation string keys are intentionally added with empty values initially and will be translated in a separate PR. Do not report missing translations for entirely new string additions.
Learnt from: cathysarisky
PR: TryGhost/Ghost#22175
File: ghost/i18n/locales/pt-BR/portal.json:18-18
Timestamp: 2025-02-12T20:09:12.731Z
Learning: In the Ghost project, new translation keys are first added with empty string values, and the actual translations are added later in separate PRs to maintain better organization of translation work.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-20T21:08:21.026Z
Learning: In the Ghost project, translation files (ghost/i18n/locales/*/*.json) commonly have blank values for translations, and this is normal behavior that should not be flagged in reviews. These empty translations will be filled in separate PRs.
Learnt from: aileen
PR: TryGhost/Ghost#24073
File: apps/admin-x-settings/src/components/settings/site/DesignAndThemeModal.tsx:68-85
Timestamp: 2025-06-26T14:53:58.238Z
Learning: In Ghost's theme limit checking functionality (useCheckThemeLimitError hook), the `checkThemeLimitError` function is designed to always resolve and return either an error message string if the theme is disallowed or null if allowed. It does not throw/reject promises - any internal errors are caught within the hook and converted to return values, so `.catch()` blocks are not needed when calling this function.
Learnt from: cathysarisky
PR: TryGhost/Ghost#21651
File: ghost/i18n/locales/ar/comments.json:1-1
Timestamp: 2025-01-29T15:35:26.447Z
Learning: All JSON files containing translation strings in ghost/i18n/locales must include both opening and closing braces to maintain valid JSON structure.
🔇 Additional comments (5)
ghost/core/core/frontend/services/theme-engine/i18next/ThemeI18n.js (5)
1-4: LGTM! Dependencies are appropriate for the functionality.The imports correctly include necessary modules for error handling, i18n functionality, and file system operations.
6-20: LGTM! Constructor follows good practices.The parameter validation, default values, and error handling are well implemented. The use of private properties and consistent error messaging align with Ghost's conventions.
22-31: LGTM! Getter/setter implementation is clean and serves testing needs.The simple getter/setter for basePath is appropriately documented and follows standard conventions for test accessibility.
87-100: LGTM! Translation method is well-implemented.The validation, error handling, and type safety measures are appropriate. The error message includes the key for better debugging, which is helpful.
103-103: LGTM! Standard module export.Clean and standard Node.js module export pattern.
ghost/core/core/frontend/services/theme-engine/i18next/ThemeI18n.js
Outdated
Show resolved
Hide resolved
Member
Author
|
@coderabbitai please no more reviews |
Contributor
|
Tip For best results, initiate chat on the files or code changes. @cathysarisky Understood! I'll stop providing reviews on this PR. Thanks for letting me know. 🧠 Learnings used |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
ghost/i18n/lib/i18n.js (3)
2-3: Remove unused imports or uncomment the theme loading logic.The
fs-extraandpathimports are currently unused because the theme file loading logic is commented out. Consider either removing these imports temporarily or implementing the commented logic if this is ready for testing.-const fs = require('fs-extra'); -const path = require('path');
93-93: Address unused parameter warning.The
optionsparameter is declared but not used due to the commented theme loading logic. Consider either removing it temporarily or implementing the logic that uses it.If keeping the parameter for future implementation:
-module.exports = (lng = 'en', ns = 'portal', options = {}) => { +module.exports = (lng = 'en', ns = 'portal', options = {}) => { // eslint-disable-line no-unused-vars
107-107: Fix indentation in commented code.The commented code has inconsistent indentation that should be corrected if/when it's uncommented.
- /* const themeLocalesPath = options.themePath; + /* const themeLocalesPath = options.themePath;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ghost/i18n/lib/i18n.js(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-20T21:08:21.026Z
Learning: In the Ghost project, translation files (ghost/i18n/locales/*/*.json) commonly have blank values for translations, and this is normal behavior that should not be flagged in reviews. These empty translations will be filled in separate PRs.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23070
File: ghost/i18n/locales/hu/comments.json:45-45
Timestamp: 2025-04-30T16:27:03.542Z
Learning: In the Ghost project, new translation string keys are intentionally added with empty values initially and will be translated in a separate PR. Do not report missing translations for entirely new string additions.
Learnt from: cathysarisky
PR: TryGhost/Ghost#22175
File: ghost/i18n/locales/pt-BR/portal.json:18-18
Timestamp: 2025-02-12T20:09:12.731Z
Learning: In the Ghost project, new translation keys are first added with empty string values, and the actual translations are added later in separate PRs to maintain better organization of translation work.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-01-29T15:27:29.391Z
Learning: When reviewing translation files in ghost/i18n/locales, translators should only modify the translated values and must not change the keys or placeholder styles (e.g., %%{status}%%, {site}, etc.) as they serve specific purposes like Mailgun string replacement.
Learnt from: cathysarisky
PR: TryGhost/Ghost#22175
File: ghost/i18n/locales/lt/portal.json:16-16
Timestamp: 2025-02-12T20:23:47.777Z
Learning: When reviewing localization changes, verify the actual history of translations before flagging potential deletions. Empty translation strings may be intentional placeholders rather than deleted content.
Learnt from: ErisDS
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-29T08:34:28.248Z
Learning: When a user explicitly requests no further changes after making their own corrective commit, respect that request and focus on learning from their solution rather than attempting additional modifications.
Learnt from: SebastianSchroeder
PR: TryGhost/Ghost#23602
File: ghost/i18n/locales/vi/ghost.json:3-4
Timestamp: 2025-05-30T11:41:47.336Z
Learning: SebastianSchroeder prefers to handle missing translations in separate PRs rather than including them in the same PR that adds the i18n infrastructure, to keep PRs focused on specific functionality.
Learnt from: cathysarisky
PR: TryGhost/Ghost#21982
File: ghost/i18n/locales/hu v2/portal.json:70-70
Timestamp: 2025-01-29T15:23:58.658Z
Learning: Typos and spelling errors within localization files (ghost/i18n/locales/*.json) must be treated as important problems, not nitpicks, as they directly affect the user experience for speakers of that language. These issues should be flagged with high priority during code review.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-06T00:47:48.488Z
Learning: The theme i18n initialization in Ghost only happens during server boot, theme changes, and language changes - not on every page load or `t` helper call. The initialization check in the `t` helper is a safeguard that doesn't execute during normal page rendering once the theme is loaded.
Learnt from: kevinansfield
PR: TryGhost/Ghost#23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23161
File: ghost/i18n/lib/i18n.js:107-144
Timestamp: 2025-05-14T14:39:09.136Z
Learning: In Ghost's i18n system, translation keys are the actual English phrases, so when a translation is missing, displaying the key itself serves as the English fallback, eliminating the need for explicit fallback resource loading.
ghost/i18n/lib/i18n.js (6)
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-06T00:47:48.488Z
Learning: The theme i18n initialization in Ghost only happens during server boot, theme changes, and language changes - not on every page load or `t` helper call. The initialization check in the `t` helper is a safeguard that doesn't execute during normal page rendering once the theme is loaded.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23161
File: ghost/i18n/lib/i18n.js:107-144
Timestamp: 2025-05-14T14:39:09.136Z
Learning: In Ghost's i18n system, translation keys are the actual English phrases, so when a translation is missing, displaying the key itself serves as the English fallback, eliminating the need for explicit fallback resource loading.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-01-29T15:27:29.391Z
Learning: When reviewing translation files in ghost/i18n/locales, translators should only modify the translated values and must not change the keys or placeholder styles (e.g., %%{status}%%, {site}, etc.) as they serve specific purposes like Mailgun string replacement.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-20T21:08:21.026Z
Learning: In the Ghost project, translation files (ghost/i18n/locales/*/*.json) commonly have blank values for translations, and this is normal behavior that should not be flagged in reviews. These empty translations will be filled in separate PRs.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23070
File: ghost/i18n/locales/hu/comments.json:45-45
Timestamp: 2025-04-30T16:27:03.542Z
Learning: In the Ghost project, new translation string keys are intentionally added with empty values initially and will be translated in a separate PR. Do not report missing translations for entirely new string additions.
Learnt from: cathysarisky
PR: TryGhost/Ghost#21651
File: ghost/i18n/locales/ar/comments.json:1-1
Timestamp: 2025-01-29T15:35:26.447Z
Learning: All JSON files containing translation strings in ghost/i18n/locales must include both opening and closing braces to maintain valid JSON structure.
🧬 Code Graph Analysis (1)
ghost/i18n/lib/i18n.js (2)
ghost/core/core/frontend/services/theme-engine/i18next/ThemeI18n.js (2)
fs(4-4)path(3-3)ghost/i18n/test/i18n.test.js (2)
fs(2-2)path(3-3)
🪛 ESLint
ghost/i18n/lib/i18n.js
[error] 2-2: 'fs' is assigned a value but never used.
(no-unused-vars)
[error] 3-3: 'path' is assigned a value but never used.
(no-unused-vars)
[error] 93-93: 'options' is assigned a value but never used.
(no-unused-vars)
[error] 107-107: Expected indentation of 8 spaces but found 5.
(indent)
🔇 Additional comments (2)
ghost/i18n/lib/i18n.js (2)
99-101: Good security consideration for theme translations.Setting
escapeValue: falsefor the theme namespace is appropriate since themes may need to include HTML content in translations, unlike other namespaces that should escape user input for security.
103-138: Empty theme translations – please verify usageI performed a search for any calls to the theme namespace in i18n helpers (e.g.
t('theme:…')) and found no matches. This suggests that the commented-out theme loading logic is only needed for the browser tests in this PR and isn’t used elsewhere in production code.• File requiring attention: ghost/i18n/lib/i18n.js (lines 103–138)
Before merging, please manually confirm that no production code paths rely on theme translations to avoid missing fallbacks.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
ghost/i18n/lib/i18n.js (2)
2-3: Remove unused import or uncomment fs import for consistency.The
pathimport is currently unused since the theme file loading logic is commented out. Either remove thepathimport or uncomment thefsimport to maintain consistency with the intended functionality.-//const fs = require('fs-extra'); -const path = require('path'); +//const fs = require('fs-extra'); +//const path = require('path');Or alternatively, if you plan to enable the theme loading soon:
-//const fs = require('fs-extra'); +const fs = require('fs-extra'); const path = require('path');
102-138: Theme namespace currently returns empty resources - appears intentional for testing.The theme translation loading logic is completely commented out, which means theme translations will not work in the current state. This appears intentional for debugging/testing purposes based on the PR title mentioning "trying to suss out theme i18n browser test problems."
The commented fallback logic looks well-designed (requested locale → English → empty object), but there's a minor indentation issue.
Fix the indentation in the commented code:
- /* const themeLocalesPath = options.themePath; + /* const themeLocalesPath = options.themePath;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ghost/core/core/frontend/services/theme-engine/i18next/ThemeI18n.js(1 hunks)ghost/i18n/lib/i18n.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- ghost/core/core/frontend/services/theme-engine/i18next/ThemeI18n.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-20T21:08:21.026Z
Learning: In the Ghost project, translation files (ghost/i18n/locales/*/*.json) commonly have blank values for translations, and this is normal behavior that should not be flagged in reviews. These empty translations will be filled in separate PRs.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23070
File: ghost/i18n/locales/hu/comments.json:45-45
Timestamp: 2025-04-30T16:27:03.542Z
Learning: In the Ghost project, new translation string keys are intentionally added with empty values initially and will be translated in a separate PR. Do not report missing translations for entirely new string additions.
Learnt from: cathysarisky
PR: TryGhost/Ghost#22175
File: ghost/i18n/locales/pt-BR/portal.json:18-18
Timestamp: 2025-02-12T20:09:12.731Z
Learning: In the Ghost project, new translation keys are first added with empty string values, and the actual translations are added later in separate PRs to maintain better organization of translation work.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-01-29T15:27:29.391Z
Learning: When reviewing translation files in ghost/i18n/locales, translators should only modify the translated values and must not change the keys or placeholder styles (e.g., %%{status}%%, {site}, etc.) as they serve specific purposes like Mailgun string replacement.
Learnt from: cathysarisky
PR: TryGhost/Ghost#22175
File: ghost/i18n/locales/lt/portal.json:16-16
Timestamp: 2025-02-12T20:23:47.777Z
Learning: When reviewing localization changes, verify the actual history of translations before flagging potential deletions. Empty translation strings may be intentional placeholders rather than deleted content.
Learnt from: ErisDS
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-29T08:34:28.248Z
Learning: When a user explicitly requests no further changes after making their own corrective commit, respect that request and focus on learning from their solution rather than attempting additional modifications.
Learnt from: SebastianSchroeder
PR: TryGhost/Ghost#23602
File: ghost/i18n/locales/vi/ghost.json:3-4
Timestamp: 2025-05-30T11:41:47.336Z
Learning: SebastianSchroeder prefers to handle missing translations in separate PRs rather than including them in the same PR that adds the i18n infrastructure, to keep PRs focused on specific functionality.
Learnt from: cathysarisky
PR: TryGhost/Ghost#21982
File: ghost/i18n/locales/hu v2/portal.json:70-70
Timestamp: 2025-01-29T15:23:58.658Z
Learning: Typos and spelling errors within localization files (ghost/i18n/locales/*.json) must be treated as important problems, not nitpicks, as they directly affect the user experience for speakers of that language. These issues should be flagged with high priority during code review.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-06T00:47:48.488Z
Learning: The theme i18n initialization in Ghost only happens during server boot, theme changes, and language changes - not on every page load or `t` helper call. The initialization check in the `t` helper is a safeguard that doesn't execute during normal page rendering once the theme is loaded.
Learnt from: kevinansfield
PR: TryGhost/Ghost#23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23161
File: ghost/i18n/lib/i18n.js:107-144
Timestamp: 2025-05-14T14:39:09.136Z
Learning: In Ghost's i18n system, translation keys are the actual English phrases, so when a translation is missing, displaying the key itself serves as the English fallback, eliminating the need for explicit fallback resource loading.
ghost/i18n/lib/i18n.js (7)
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-06T00:47:48.488Z
Learning: The theme i18n initialization in Ghost only happens during server boot, theme changes, and language changes - not on every page load or `t` helper call. The initialization check in the `t` helper is a safeguard that doesn't execute during normal page rendering once the theme is loaded.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23161
File: ghost/i18n/lib/i18n.js:107-144
Timestamp: 2025-05-14T14:39:09.136Z
Learning: In Ghost's i18n system, translation keys are the actual English phrases, so when a translation is missing, displaying the key itself serves as the English fallback, eliminating the need for explicit fallback resource loading.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-01-29T15:27:29.391Z
Learning: When reviewing translation files in ghost/i18n/locales, translators should only modify the translated values and must not change the keys or placeholder styles (e.g., %%{status}%%, {site}, etc.) as they serve specific purposes like Mailgun string replacement.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-20T21:08:21.026Z
Learning: In the Ghost project, translation files (ghost/i18n/locales/*/*.json) commonly have blank values for translations, and this is normal behavior that should not be flagged in reviews. These empty translations will be filled in separate PRs.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23070
File: ghost/i18n/locales/hu/comments.json:45-45
Timestamp: 2025-04-30T16:27:03.542Z
Learning: In the Ghost project, new translation string keys are intentionally added with empty values initially and will be translated in a separate PR. Do not report missing translations for entirely new string additions.
Learnt from: cathysarisky
PR: TryGhost/Ghost#21651
File: ghost/i18n/locales/ar/comments.json:1-1
Timestamp: 2025-01-29T15:35:26.447Z
Learning: All JSON files containing translation strings in ghost/i18n/locales must include both opening and closing braces to maintain valid JSON structure.
Learnt from: cathysarisky
PR: TryGhost/Ghost#22175
File: ghost/i18n/locales/pt-BR/portal.json:18-18
Timestamp: 2025-02-12T20:09:12.731Z
Learning: In the Ghost project, new translation keys are first added with empty string values, and the actual translations are added later in separate PRs to maintain better organization of translation work.
🧬 Code Graph Analysis (1)
ghost/i18n/lib/i18n.js (2)
ghost/core/core/frontend/services/theme-engine/i18next/ThemeI18n.js (1)
path(3-3)ghost/i18n/test/i18n.test.js (2)
path(3-3)resources(136-136)
🪛 ESLint
ghost/i18n/lib/i18n.js
[error] 3-3: 'path' is assigned a value but never used.
(no-unused-vars)
[error] 93-93: 'options' is assigned a value but never used.
(no-unused-vars)
[error] 107-107: Expected indentation of 8 spaces but found 5.
(indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Setup
🔇 Additional comments (3)
ghost/i18n/lib/i18n.js (3)
91-91: Good documentation update.The JSDoc correctly reflects the addition of the 'theme' namespace option.
93-93: Options parameter is currently unused but appears intentional.The
optionsparameter is flagged as unused by ESLint since the theme loading logic is commented out. This appears intentional for the current experimental state of the theme translation feature.
99-101: Theme interpolation configuration looks correct.Setting
escapeValue: falsefor theme translations is appropriate, as theme content may legitimately contain HTML markup. This configuration properly differentiates theme translations from other namespaces.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ghost/i18n/lib/i18n.js (1)
119-121: Monitor performance impact of require cache deletion.The require cache deletion ensures fresh reads of theme translation files, which is appropriate for the theme translation system that can change during runtime. However, monitor for potential memory or performance implications if this function is called frequently.
Consider implementing a more targeted cache invalidation strategy if performance becomes an issue:
// Alternative approach - only delete if file has actually changed const stats = fs.statSync(localePath); const cacheKey = require.resolve(localePath); if (!fileTimestamps[cacheKey] || fileTimestamps[cacheKey] !== stats.mtime.getTime()) { delete require.cache[cacheKey]; fileTimestamps[cacheKey] = stats.mtime.getTime(); }Also applies to: 131-133
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ghost/i18n/lib/i18n.js(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-20T21:08:21.026Z
Learning: In the Ghost project, translation files (ghost/i18n/locales/*/*.json) commonly have blank values for translations, and this is normal behavior that should not be flagged in reviews. These empty translations will be filled in separate PRs.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23070
File: ghost/i18n/locales/hu/comments.json:45-45
Timestamp: 2025-04-30T16:27:03.542Z
Learning: In the Ghost project, new translation string keys are intentionally added with empty values initially and will be translated in a separate PR. Do not report missing translations for entirely new string additions.
Learnt from: cathysarisky
PR: TryGhost/Ghost#22175
File: ghost/i18n/locales/pt-BR/portal.json:18-18
Timestamp: 2025-02-12T20:09:12.731Z
Learning: In the Ghost project, new translation keys are first added with empty string values, and the actual translations are added later in separate PRs to maintain better organization of translation work.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-01-29T15:27:29.391Z
Learning: When reviewing translation files in ghost/i18n/locales, translators should only modify the translated values and must not change the keys or placeholder styles (e.g., %%{status}%%, {site}, etc.) as they serve specific purposes like Mailgun string replacement.
Learnt from: cathysarisky
PR: TryGhost/Ghost#22175
File: ghost/i18n/locales/lt/portal.json:16-16
Timestamp: 2025-02-12T20:23:47.777Z
Learning: When reviewing localization changes, verify the actual history of translations before flagging potential deletions. Empty translation strings may be intentional placeholders rather than deleted content.
Learnt from: ErisDS
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-29T08:34:28.248Z
Learning: When a user explicitly requests no further changes after making their own corrective commit, respect that request and focus on learning from their solution rather than attempting additional modifications.
Learnt from: SebastianSchroeder
PR: TryGhost/Ghost#23602
File: ghost/i18n/locales/vi/ghost.json:3-4
Timestamp: 2025-05-30T11:41:47.336Z
Learning: SebastianSchroeder prefers to handle missing translations in separate PRs rather than including them in the same PR that adds the i18n infrastructure, to keep PRs focused on specific functionality.
Learnt from: cathysarisky
PR: TryGhost/Ghost#21982
File: ghost/i18n/locales/hu v2/portal.json:70-70
Timestamp: 2025-01-29T15:23:58.658Z
Learning: Typos and spelling errors within localization files (ghost/i18n/locales/*.json) must be treated as important problems, not nitpicks, as they directly affect the user experience for speakers of that language. These issues should be flagged with high priority during code review.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-06T00:47:48.488Z
Learning: The theme i18n initialization in Ghost only happens during server boot, theme changes, and language changes - not on every page load or `t` helper call. The initialization check in the `t` helper is a safeguard that doesn't execute during normal page rendering once the theme is loaded.
Learnt from: kevinansfield
PR: TryGhost/Ghost#23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23161
File: ghost/i18n/lib/i18n.js:107-144
Timestamp: 2025-05-14T14:39:09.136Z
Learning: In Ghost's i18n system, translation keys are the actual English phrases, so when a translation is missing, displaying the key itself serves as the English fallback, eliminating the need for explicit fallback resource loading.
ghost/i18n/lib/i18n.js (6)
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-06T00:47:48.488Z
Learning: The theme i18n initialization in Ghost only happens during server boot, theme changes, and language changes - not on every page load or `t` helper call. The initialization check in the `t` helper is a safeguard that doesn't execute during normal page rendering once the theme is loaded.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23161
File: ghost/i18n/lib/i18n.js:107-144
Timestamp: 2025-05-14T14:39:09.136Z
Learning: In Ghost's i18n system, translation keys are the actual English phrases, so when a translation is missing, displaying the key itself serves as the English fallback, eliminating the need for explicit fallback resource loading.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-20T21:08:21.026Z
Learning: In the Ghost project, translation files (ghost/i18n/locales/*/*.json) commonly have blank values for translations, and this is normal behavior that should not be flagged in reviews. These empty translations will be filled in separate PRs.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-01-29T15:27:29.391Z
Learning: When reviewing translation files in ghost/i18n/locales, translators should only modify the translated values and must not change the keys or placeholder styles (e.g., %%{status}%%, {site}, etc.) as they serve specific purposes like Mailgun string replacement.
Learnt from: cathysarisky
PR: TryGhost/Ghost#21651
File: ghost/i18n/locales/ar/comments.json:1-1
Timestamp: 2025-01-29T15:35:26.447Z
Learning: All JSON files containing translation strings in ghost/i18n/locales must include both opening and closing braces to maintain valid JSON structure.
Learnt from: cathysarisky
PR: TryGhost/Ghost#21982
File: ghost/i18n/locales/hu v2/portal.json:70-70
Timestamp: 2025-01-29T15:23:58.658Z
Learning: Typos and spelling errors within localization files (ghost/i18n/locales/*.json) must be treated as important problems, not nitpicks, as they directly affect the user experience for speakers of that language. These issues should be flagged with high priority during code review.
🪛 ESLint
ghost/i18n/lib/i18n.js
[error] 123-123: Direct calls to new Error() are not allowed. Please use @tryghost/errors.
(ghost/ghost-custom/no-native-error)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Setup
🔇 Additional comments (3)
ghost/i18n/lib/i18n.js (3)
2-3: Dependencies appropriately added for theme resource loading.The additions of
pathandfs-extraare necessary for the new theme translation file loading functionality and follow Ghost's patterns for file system operations.
157-171: Function signature and namespace handling properly implemented.The backwards-compatible signature change and conditional namespace handling are well-structured. The separation between existing and theme resource generation maintains code clarity while enabling the new functionality.
163-165: Appropriate interpolation configuration for theme translations.Disabling
escapeValuefor the theme namespace is correct as themes often need to include markup in translations. The conditional application maintains security for other namespaces while providing necessary flexibility for theme developers.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
ghost/core/core/bridge.js(1 hunks)ghost/core/core/frontend/helpers/t.js(2 hunks)ghost/core/core/frontend/services/theme-engine/active.js(2 hunks)ghost/core/test/unit/frontend/services/theme-engine/i18next.test.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- ghost/core/core/frontend/helpers/t.js
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-20T21:08:21.026Z
Learning: In the Ghost project, translation files (ghost/i18n/locales/*/*.json) commonly have blank values for translations, and this is normal behavior that should not be flagged in reviews. These empty translations will be filled in separate PRs.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23070
File: ghost/i18n/locales/hu/comments.json:45-45
Timestamp: 2025-04-30T16:27:03.542Z
Learning: In the Ghost project, new translation string keys are intentionally added with empty values initially and will be translated in a separate PR. Do not report missing translations for entirely new string additions.
Learnt from: cathysarisky
PR: TryGhost/Ghost#22175
File: ghost/i18n/locales/pt-BR/portal.json:18-18
Timestamp: 2025-02-12T20:09:12.731Z
Learning: In the Ghost project, new translation keys are first added with empty string values, and the actual translations are added later in separate PRs to maintain better organization of translation work.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-01-29T15:27:29.391Z
Learning: When reviewing translation files in ghost/i18n/locales, translators should only modify the translated values and must not change the keys or placeholder styles (e.g., %%{status}%%, {site}, etc.) as they serve specific purposes like Mailgun string replacement.
Learnt from: cathysarisky
PR: TryGhost/Ghost#22175
File: ghost/i18n/locales/lt/portal.json:16-16
Timestamp: 2025-02-12T20:23:47.777Z
Learning: When reviewing localization changes, verify the actual history of translations before flagging potential deletions. Empty translation strings may be intentional placeholders rather than deleted content.
Learnt from: ErisDS
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-29T08:34:28.248Z
Learning: When a user explicitly requests no further changes after making their own corrective commit, respect that request and focus on learning from their solution rather than attempting additional modifications.
Learnt from: SebastianSchroeder
PR: TryGhost/Ghost#23602
File: ghost/i18n/locales/vi/ghost.json:3-4
Timestamp: 2025-05-30T11:41:47.336Z
Learning: SebastianSchroeder prefers to handle missing translations in separate PRs rather than including them in the same PR that adds the i18n infrastructure, to keep PRs focused on specific functionality.
Learnt from: cathysarisky
PR: TryGhost/Ghost#21982
File: ghost/i18n/locales/hu v2/portal.json:70-70
Timestamp: 2025-01-29T15:23:58.658Z
Learning: Typos and spelling errors within localization files (ghost/i18n/locales/*.json) must be treated as important problems, not nitpicks, as they directly affect the user experience for speakers of that language. These issues should be flagged with high priority during code review.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-06T00:47:48.488Z
Learning: The theme i18n initialization in Ghost only happens during server boot, theme changes, and language changes - not on every page load or `t` helper call. The initialization check in the `t` helper is a safeguard that doesn't execute during normal page rendering once the theme is loaded.
Learnt from: kevinansfield
PR: TryGhost/Ghost#23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
ghost/core/core/bridge.js (1)
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-06T00:47:48.488Z
Learning: The theme i18n initialization in Ghost only happens during server boot, theme changes, and language changes - not on every page load or `t` helper call. The initialization check in the `t` helper is a safeguard that doesn't execute during normal page rendering once the theme is loaded.
ghost/core/core/frontend/services/theme-engine/active.js (4)
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-06T00:47:48.488Z
Learning: The theme i18n initialization in Ghost only happens during server boot, theme changes, and language changes - not on every page load or `t` helper call. The initialization check in the `t` helper is a safeguard that doesn't execute during normal page rendering once the theme is loaded.
Learnt from: kevinansfield
PR: TryGhost/Ghost#23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23070
File: ghost/i18n/locales/hu/comments.json:45-45
Timestamp: 2025-04-30T16:27:03.542Z
Learning: In the Ghost project, new translation string keys are intentionally added with empty values initially and will be translated in a separate PR. Do not report missing translations for entirely new string additions.
Learnt from: cathysarisky
PR: TryGhost/Ghost#22175
File: ghost/i18n/locales/lt/portal.json:16-16
Timestamp: 2025-02-12T20:23:47.777Z
Learning: When reviewing localization changes, verify the actual history of translations before flagging potential deletions. Empty translation strings may be intentional placeholders rather than deleted content.
ghost/core/test/unit/frontend/services/theme-engine/i18next.test.js (8)
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-06T00:47:48.488Z
Learning: The theme i18n initialization in Ghost only happens during server boot, theme changes, and language changes - not on every page load or `t` helper call. The initialization check in the `t` helper is a safeguard that doesn't execute during normal page rendering once the theme is loaded.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-05-20T21:08:21.026Z
Learning: In the Ghost project, translation files (ghost/i18n/locales/*/*.json) commonly have blank values for translations, and this is normal behavior that should not be flagged in reviews. These empty translations will be filled in separate PRs.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23070
File: ghost/i18n/locales/hu/comments.json:45-45
Timestamp: 2025-04-30T16:27:03.542Z
Learning: In the Ghost project, new translation string keys are intentionally added with empty values initially and will be translated in a separate PR. Do not report missing translations for entirely new string additions.
Learnt from: mike182uk
PR: TryGhost/Ghost#22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Learnt from: cathysarisky
PR: TryGhost/Ghost#23161
File: ghost/i18n/lib/i18n.js:107-144
Timestamp: 2025-05-14T14:39:09.136Z
Learning: In Ghost's i18n system, translation keys are the actual English phrases, so when a translation is missing, displaying the key itself serves as the English fallback, eliminating the need for explicit fallback resource loading.
Learnt from: cathysarisky
PR: TryGhost/Ghost#0
File: :0-0
Timestamp: 2025-01-29T15:27:29.391Z
Learning: When reviewing translation files in ghost/i18n/locales, translators should only modify the translated values and must not change the keys or placeholder styles (e.g., %%{status}%%, {site}, etc.) as they serve specific purposes like Mailgun string replacement.
Learnt from: cathysarisky
PR: TryGhost/Ghost#22175
File: ghost/i18n/locales/pt-BR/portal.json:18-18
Timestamp: 2025-02-12T20:09:12.731Z
Learning: In the Ghost project, new translation keys are first added with empty string values, and the actual translations are added later in separate PRs to maintain better organization of translation work.
Learnt from: mike182uk
PR: TryGhost/Ghost#22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.
🧬 Code Graph Analysis (1)
ghost/core/core/frontend/services/theme-engine/active.js (2)
ghost/core/core/frontend/helpers/t.js (3)
require(13-13)require(14-14)labs(15-15)ghost/core/core/shared/labs.js (2)
require(129-129)labs(61-61)
🪛 ESLint
ghost/core/core/bridge.js
[error] 49-51: Block must not be padded by blank lines.
(padded-blocks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Setup
🔇 Additional comments (6)
ghost/core/core/frontend/services/theme-engine/active.js (2)
21-22: Clean integration of new dependencies.The imports are properly added to support the conditional translation service initialization.
105-112: Well-implemented feature flag conditional logic.The conditional initialization cleanly switches between the legacy and new translation services based on the feature flag. This follows Ghost's established patterns for feature flag implementation and maintains backward compatibility.
ghost/core/test/unit/frontend/services/theme-engine/i18next.test.js (4)
6-16: Well-structured test setup.The test suite follows proper testing patterns with clear setup and teardown. The use of fixture themes for testing is appropriate.
18-31: Good coverage of basic initialization scenarios.These tests properly verify default locale behavior and custom locale initialization, ensuring the new service works as expected.
33-43: Excellent fallback behavior testing.These tests verify the critical fallback chain: locale-specific → English → key fallback. This aligns with Ghost's established translation patterns where keys serve as English fallbacks.
45-62: Comprehensive edge case and error handling coverage.The tests properly cover edge cases (empty string keys) and error conditions (uninitialized usage), ensuring robust behavior of the new translation service.
cathysarisky
changed the title
ErisDS
added
core team
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on November 3
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Contributor
|
@JohnONolan Hi John! Let me try my chance starting from this PR. Are we able to get some attention to this? A very valuable effort is waiting here since a long time. |
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
ghost/core/core/shared/labs.js
Outdated
| 'editorExcerpt', | ||
| 'additionalPaymentMethods' | ||
| 'additionalPaymentMethods', | ||
| 'themeTranslation' |
Member
There was a problem hiding this comment.
We don't tend to put features directly into public beta, there's a surprising number of people who will check everything available without knowing why or what they should be checking.
Would rolling this out as a private beta (hidden but can still be enabled on a per-site basis) first be problematic? It would let us test and gain confidence in the feature design + code before making it more widely available.
Member
Author
There was a problem hiding this comment.
No objections. Self-hosters can enable it, right? Would someone on Pro who wanted to test it just email support?
ghost/core/core/frontend/services/theme-engine/i18next/ThemeI18n.js
Outdated
Show resolved
Hide resolved
apps/admin-x-settings/src/components/settings/advanced/labs/BetaFeatures.tsx
Outdated
Show resolved
Hide resolved
Contributor
E2E Tests FailedTo view the Playwright test report locally, run: REPORT_DIR=$(mktemp -d) && gh run download 19828738672 -n playwright-report -D "$REPORT_DIR" && npx playwright show-report "$REPORT_DIR" |
cathysarisky
changed the title
kevinansfield
approved these changes
Dec 9, 2025
Member
kevinansfield
left a comment
kevinansfield
left a comment
There was a problem hiding this comment.
Nice work! 🙌 I took the liberty of pushing some e2e-frontend tests for theme translation as I was using those as an extra verification step.
There's one comment about a potential edge-case but I didn't make any changes as I wondered if you knew any reasons for/against the proposed fix - if you think its fixable then an e2e test to poke the problem and prove the fix would be good. This isn't a blocker however, it's very much an edge case.
Otherwise approved ✅ Thanks for working on this and addressing the previous feedback ❤️
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| } else { | ||
| // Initialize the legacy translation service | ||
| themeI18n.init(options); | ||
| } |
There was a problem hiding this comment.
Bug: Stale i18n data when switching flag after theme change
The initI18n method only initializes one i18n service based on the current flag state, leaving the other service with potentially stale data. When the theme changes while flag is ON, only themeI18next is updated. If the flag is later toggled OFF, there's a brief race window before the settings.labs.edited event handler reinitializes themeI18n. During this window, t.js could use stale translations from the old theme. The bridge handler does eventually fix this, but a request arriving between the settings update and event propagation could receive incorrect translations.
Additional Locations (1)
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
4 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.
Note
Introduces i18next-powered theme translations gated by a new
labsflag, with runtime reinitialization and comprehensive tests.themeTranslationand expose toggle inapps/admin-x-settings.themeTranslation.core/frontend/helpers/t: switch between legacythemeI18nand newthemeI18nextbased onlabs.isSet('themeTranslation'), with lazy init.theme-engine/active:initI18ninitializes eitheri18nor newi18nextservice per flag.handlebarsservice: exportthemeI18nextalongside existingthemeI18n.core/bridge: trackthemeTranslationstate; onsettings.labs.editedor locale change, re-init active theme i18n.theme-engine/i18next/ThemeI18nand index: loads theme locale JSON fromcontent/themes/<theme>/locales, falls back toenor key, supports interpolation, and exposest.@tryghost/i18nto supportns: 'theme', load theme resources from a directory, disable HTML escaping for theme interpolation, and retain existing namespaces.thelper path andThemeI18nbehavior, plus theme resource handling in@tryghost/i18n.Written by Cursor Bugbot for commit edd29e6. This will update automatically on new commits. Configure here.