[WIP] Theme i18n

[cathysarisky]

Don't look at this one. I replaced it with a better one!
#23161

[github-actions]

Thanks for contributing to Ghost's i18n :)

This PR has been automatically marked as stale because there has not been any activity here in 3 weeks.
I18n PRs tend to get out of date quickly, so we're closing them to keep the PR list clean.

If you're still interested in working on this PR, please let us know. Otherwise this PR will be closed shortly, but can always be reopened later. Thank you for understanding 🙂

[coderabbitai]

Walkthrough

This set of changes refactors the internationalization (i18n) system for theme translations in the codebase. The previous I18n class, which handled loading, managing, and formatting localized strings, has been removed. The ThemeI18n class is re-implemented as a standalone class that no longer inherits from I18n, and now relies directly on the @tryghost/i18n library for translation logic. Its initialization is now asynchronous and requires a theme path, with improved fallback mechanisms for missing or invalid locale files. The translation helper (t) is updated to ensure it always returns a plain string, with explanatory comments added. Test suites for both the helper and the theme i18n service are updated to accommodate asynchronous initialization and to test the new fallback and translation behaviors. The i18n core module is extended to support a new 'theme' namespace, with resource loading from theme directories and namespace-specific interpolation and escaping settings. Comprehensive new tests are added for the i18n module, especially for theme resource loading and configuration.

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e95074d and 45666b8.

📒 Files selected for processing (1)

• ghost/core/core/frontend/helpers/t.js (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• ghost/core/core/frontend/helpers/t.js

⏰ Context from checks skipped due to timeout of 90000ms (10)

• GitHub Check: Ghost-CLI tests
• GitHub Check: Unit tests (Node 22.13.1)
• GitHub Check: Regression tests (Node 20.11.1, sqlite3)
• GitHub Check: Unit tests (Node 20.11.1)
• GitHub Check: Database tests (Node 20.11.1, sqlite3)
• GitHub Check: Regression tests (Node 20.11.1, mysql8)
• GitHub Check: Database tests (Node 22.13.1, mysql8)
• GitHub Check: Database tests (Node 20.11.1, mysql8)
• GitHub Check: i18n
• GitHub Check: Lint

✨ Finishing Touches

• 📝 Generate Docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (5)

ghost/core/core/frontend/services/theme-engine/i18n/ThemeI18n.js (3)

52-69: Duplicate FS checks – shave one I/O round-trip

fs.pathExists + fs.access hit the disk twice. A single fs.access (or fs.stat) is enough:

-const themePathExists = await fs.pathExists(themeLocalesPath);
-if (!themePathExists) {
+try {
+    await fs.access(themeLocalesPath);
+} catch {
     this._i18n = {t: key => key};
     return;
-}
+}

This reduces latency and potential TOCTOU windows.

---

71-79: this._locale is retained when falling back to English – clarify intent

Inside the fallback branch you still call
i18nLib(this._locale, 'theme', …), even though you verified only en.json exists. That’s perfectly valid because the library falls back, but a future maintainer might assume the language switched to en. Consider adding a short comment or explicitly passing 'en' for clarity.

---

94-100: Guard against non-primitive returns from i18next

You coerce result with String(result), which handles objects like {} but silently converts undefined → 'undefined'. That may leak into rendered templates. Consider returning '' (an empty string) when the key is missing instead:

-const result = this._i18n.t(key, bindings);
-return typeof result === 'string' ? result : String(result);
+const result = this._i18n.t(key, bindings);
+if (typeof result === 'string') {
+    return result;
+}
+// If i18next returns `undefined`, an object, etc.
+return '';

ghost/i18n/test/i18n.test.js (2)

167-178: Use fs.mkdtemp for isolated temp dirs

All tests share the hard-coded directory temp-theme-locales. If tests run in parallel (e.g. with mocha --parallel) they’ll stomp on each other. Prefer an OS-generated unique directory:

-beforeEach(async function () {
-    themeLocalesPath = path.join(__dirname, 'temp-theme-locales');
-    await fsExtra.ensureDir(themeLocalesPath);
+beforeEach(async function () {
+    themeLocalesPath = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'ghost-theme-i18n-'));

Remember to require('os') at the top.

---

218-226: Invalid-JSON test may produce unhandled rejection noise

fsExtra.writeFile(..., 'invalid json') causes i18nLib to log a parse error to stderr in some environments, which can clutter CI output. Consider writing an incomplete but valid JSON instead (e.g. '{') or stub the logger to keep test output clean.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dc0c405 and e95074d.

📒 Files selected for processing (8)

• ghost/core/core/frontend/helpers/t.js (1 hunks)
• ghost/core/core/frontend/services/theme-engine/i18n/I18n.js (0 hunks)
• ghost/core/core/frontend/services/theme-engine/i18n/ThemeI18n.js (1 hunks)
• ghost/core/test/unit/frontend/helpers/t.test.js (5 hunks)
• ghost/core/test/unit/frontend/services/theme-engine/i18n.test.js (1 hunks)
• ghost/core/test/unit/frontend/services/theme-engine/theme-i18n.test.js (0 hunks)
• ghost/i18n/lib/i18n.js (3 hunks)
• ghost/i18n/test/i18n.test.js (2 hunks)

💤 Files with no reviewable changes (2)

• ghost/core/test/unit/frontend/services/theme-engine/theme-i18n.test.js
• ghost/core/core/frontend/services/theme-engine/i18n/I18n.js

🧰 Additional context used

🧬 Code Graph Analysis (1)

ghost/i18n/lib/i18n.js (1)

ghost/i18n/test/i18n.test.js (4)

• fs (2-2)
• path (3-3)
• resources (157-157)
• resources (243-243)

🔇 Additional comments (13)

ghost/i18n/lib/i18n.js (5)

2-3: Good addition of file system dependencies.

These imports are necessary for the new file operations to load theme locale files. Using fs-extra instead of the native fs module is appropriate as it provides additional utilities that simplify file operations.

---

90-93: Well-documented API changes.

The JSDoc has been properly updated to include the new 'theme' namespace and the options parameter with themePath. This is good practice for maintaining clear API documentation.

---

94-94: Function signature updated correctly.

The function signature now includes an options parameter with a sensible default empty object, which is consistent with the updated JSDoc.

---

96-112: Well-structured interpolation and escaping configuration.

The code elegantly configures interpolation delimiters and HTML escaping based on the namespace. For 'theme' and 'newsletter', it uses single curly braces '{' and '}', and specifically for 'theme', it disables HTML escaping. The configuration is clear and includes helpful comments explaining these choices.

---

161-166: Clear documentation for fallback behavior.

These comments provide excellent explanations of the fallback behavior, clearly documenting how empty strings are handled and which languages are used as fallbacks. This level of documentation is important for future maintenance.

ghost/core/core/frontend/helpers/t.js (1)

29-33: Improved code clarity with explicit result handling.

The code now explicitly stores the result in a variable before returning it, with clear comments explaining that the helper returns a plain string rather than a SafeString. This makes the behavior more explicit and easier to understand.

ghost/core/test/unit/frontend/helpers/t.test.js (2)

17-20: Good test isolation practice.

Adding a beforeEach hook to reset the internal _i18n instance before each test ensures proper test isolation and prevents test state from bleeding between tests.

---

22-30: Properly updated tests for async initialization.

The tests have been correctly converted to async functions to properly await the asynchronous themeI18n.init calls. This change is necessary to support the new asynchronous initialization pattern and was applied consistently across all tests.

Also applies to: 32-40, 42-50, 52-60, 78-84

ghost/core/test/unit/frontend/services/theme-engine/i18n.test.js (5)

3-4: Correctly updated imports for new implementation.

The imports have been updated to reflect the change from the old I18n class to the new ThemeI18n class, and the addition of the path module for path operations is appropriate for the implementation.

---

6-16: Well-structured test setup.

The test suite has been updated to properly test the ThemeI18n class with a good setup that creates a clean instance before each test and uses appropriate fixtures. The use of sinon.restore() in afterEach ensures proper cleanup.

---

18-26: Comprehensive locale handling tests.

These tests effectively verify that the default locale is "en" and that different locales can be set during initialization. The second test is properly marked as async to handle the asynchronous init call.

---

27-31: Good initialization test with real translations.

This test verifies that the class can be initialized with a theme path and that translations work correctly. Using real theme fixtures with actual translation files makes this test more robust.

---

33-61: Comprehensive testing of translation behaviors.

These tests thoroughly verify various aspects of translation behavior, including:

• Fallback to English when translations aren't found
• Fallback to the key itself when no translation files exist
• Empty key handling
• Error handling for use before initialization
• Key fallback for unknown strings

This level of test coverage is excellent for ensuring the reliability of the refactored implementation.

[coderabbitai]

🛠️ Refactor suggestion

Consider using asynchronous file operations for theme resource loading.

While the resource loading logic for the 'theme' namespace is well-structured and includes clear fallback mechanisms, it uses synchronous file reading (fs.readFileSync), which could block the event loop in a production environment.

- const content = fs.readFileSync(localePath, 'utf8');
+ // Use fs-extra's readFile which returns a promise
+ const content = await fs.readFile(localePath, 'utf8');

Additionally, consider simplifying the nested error handling structure to make the code more maintainable.

Committable suggestion skipped: line range outside the PR's diff.