Updated @tryghost/i18n to support ESM#29144
Merged
Merged
Conversation
Option A prototype: require->CJS Node entry (unchanged fs theme loading), import/browser->static ESM locale registry (no dynamicRequireTargets). - lib/i18n-core.js: inject generateResources via createGenerateResources(loader) - lib/require-loader.js: CJS-only dynamic require (isolated from core) - lib/locale-registry.generated.mjs: 310 static locale imports (codegen) - scripts/generate-locale-registry.js: codegen - index.esm.mjs: ESM entry wiring static registry - package.json: dual exports map (require/import/browser conditions) Core impact: zero. All ghost/core theme-i18n + i18n tests green.
Ref PLA-209. CJS require path unchanged (fs-backed theme loading); ESM/browser import path uses static per-namespace locale registries so apps tree-shake to a single namespace and no bundler needs the dynamicRequireTargets shim. - lib/i18n-core.js: injectable generateResources loader (no dynamic require) - lib/require-loader.js: CJS-only dynamic require, isolated from ESM path - lib/registry/<ns>.generated.mjs: per-namespace static locale registries (codegen) - lib/registry/<ns>.mjs + ./registry/* export: per-namespace public entries - index.esm.mjs: default ESM entry (all namespaces, classic signature) - scripts/: registry codegen, freshness CI gate, types gen, bundle measurement - build/*.d.ts: real type declarations - apps: import @tryghost/i18n/registry/<ns>; drop dynamicRequireTargets shim from apps/_shared/vite-public-app.mjs and per-app i18nNamespace wiring Per-app bundles verified to contain only their namespace's 62 locales.
ref PLA-209. Portal errors.test vi.mock target + test-utils and comments pagination test now import @tryghost/i18n/registry/<ns> to match source.
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run @tryghost/admin-x-settings:test:acceptance |
✅ Succeeded | 10m 4s | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-07-07 14:26:37 UTC
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR migrates Ghost's i18n system to static ESM per-namespace registries. Shared Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
E2E Tests FailedTo view the Playwright test report locally, run: REPORT_DIR=$(mktemp -d) && gh run download 28824390653 -n playwright-report -D "$REPORT_DIR" && npx playwright show-report "$REPORT_DIR" |
![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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/signup-form/src/i18n.d.ts (1)
1-3: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the shorthand
declare module '@tryghost/i18n/registry/signup-form'This ambient declaration resolves that import to
anyand masks the generated@tryghost/i18nsubpath typings, so the signup-form app loses type checking on@tryghost/i18n/registry/signup-form.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/signup-form/src/i18n.d.ts` around lines 1 - 3, Remove the ambient shorthand declaration for `@tryghost/i18n/registry/signup-form` from the signup-form i18n typings so it no longer forces that import to any. Update the i18n.d.ts file to keep only the broader `@tryghost/i18n` declaration, and let the generated subpath typings provide the registry/signup-form types for proper type checking.
🧹 Nitpick comments (2)
ghost/i18n/lib/require-loader.js (1)
5-11: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winBroad catch masks real JSON errors as "missing locale".
The
catchhere treats every error the same way — a missing file (MODULE_NOT_FOUND) and a malformed/invalid JSON file (SyntaxError) both silently fall back to English. A corrupted locale file would ship broken translations without any signal. Consider narrowing the catch to only fall back for missing-module errors and rethrowing (or logging) anything else.♻️ Proposed refactor
function requireLoader(locale, ns) { try { return require(`../locales/${locale}/${ns}.json`); } catch (err) { + if (err.code !== 'MODULE_NOT_FOUND') { + throw err; + } return require(`../locales/en/${ns}.json`); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ghost/i18n/lib/require-loader.js` around lines 5 - 11, The requireLoader locale fallback is too broad and hides real parsing failures. Update requireLoader so the fallback to `../locales/en/${ns}.json` only happens for missing-module cases from the `require()` of the locale JSON, and do not treat `SyntaxError` or other non-MODULE_NOT_FOUND errors as a missing translation. Use the existing `requireLoader(locale, ns)` function and its `catch (err)` path to either rethrow or log unexpected errors while preserving the English fallback only for absent locale files.ghost/i18n/index.esm.mjs (1)
14-29: 🚀 Performance & Scalability | 🔵 TrivialKeep browser consumers on the subpath entries.
This root entry still statically imports every namespace registry, so any browser consumer that remains on
@tryghost/i18nwill keep all locale data in the bundle. Please confirm the app migrations and exports leave no browser path on this entry.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ghost/i18n/index.esm.mjs` around lines 14 - 29, The root `@tryghost/i18n` entry in index.esm.mjs still statically imports all generated registries, which keeps every locale bundle reachable for browser consumers. Update the package exports and app migration points so browser usage resolves to the specific subpath entries instead of this root entry, and keep the registry aggregation confined to non-browser/internal paths via the existing createI18n and REGISTRIES setup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ghost/i18n/lib/i18n-core.js`:
- Around line 12-33: `createGenerateResources` can still crash when both the
requested locale and the English fallback are missing because
`mergeDefaultExport` is called with `undefined`. Update the `generateResources`
logic to defensively handle a double miss in the `loadResource(locale, ns)` /
`loadResource('en', ns)` path, and ensure `mergeDefaultExport` is only called
with a defined resource (or given a safe empty fallback) so missing namespace
entries degrade gracefully instead of throwing.
In `@ghost/i18n/scripts/generate-types.js`:
- Line 23: The Namespace union in generate-types.js still includes theme even
though the locale registry generator does not emit a backing module for it, so
remove theme from the Namespace type and keep the existing locale factory
signature unchanged. Update the type definition near Namespace so it matches the
namespaces actually produced by generate-locale-registry.js, and ensure any
references to `@tryghost/i18n/registry/theme` are no longer allowed through the
generated types.
---
Outside diff comments:
In `@apps/signup-form/src/i18n.d.ts`:
- Around line 1-3: Remove the ambient shorthand declaration for
`@tryghost/i18n/registry/signup-form` from the signup-form i18n typings so it no
longer forces that import to any. Update the i18n.d.ts file to keep only the
broader `@tryghost/i18n` declaration, and let the generated subpath typings
provide the registry/signup-form types for proper type checking.
---
Nitpick comments:
In `@ghost/i18n/index.esm.mjs`:
- Around line 14-29: The root `@tryghost/i18n` entry in index.esm.mjs still
statically imports all generated registries, which keeps every locale bundle
reachable for browser consumers. Update the package exports and app migration
points so browser usage resolves to the specific subpath entries instead of this
root entry, and keep the registry aggregation confined to non-browser/internal
paths via the existing createI18n and REGISTRIES setup.
In `@ghost/i18n/lib/require-loader.js`:
- Around line 5-11: The requireLoader locale fallback is too broad and hides
real parsing failures. Update requireLoader so the fallback to
`../locales/en/${ns}.json` only happens for missing-module cases from the
`require()` of the locale JSON, and do not treat `SyntaxError` or other
non-MODULE_NOT_FOUND errors as a missing translation. Use the existing
`requireLoader(locale, ns)` function and its `catch (err)` path to either
rethrow or log unexpected errors while preserving the English fallback only for
absent locale files.
🪄 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: ebdcd6e4-fb6d-4b1b-a9ed-f3ce41cf8fd7
📒 Files selected for processing (38)
apps/_shared/vite-public-app.mjsapps/comments-ui/src/app.tsxapps/comments-ui/test/unit/components/content/pagination.test.jsxapps/comments-ui/vite.config.mtsapps/portal/src/utils/i18n.jsapps/portal/test/errors.test.jsapps/portal/test/utils/test-utils.jsxapps/portal/vite.config.mjsapps/signup-form/.storybook/preview.tsxapps/signup-form/src/app.tsxapps/signup-form/src/i18n.d.tsapps/signup-form/src/preview.stories.tsxapps/signup-form/vite.config.mtsapps/sodo-search/src/app.jsxapps/sodo-search/vite.config.mjsghost/i18n/eslint.config.mjsghost/i18n/index.esm.mjsghost/i18n/lib/esm-factory.mjsghost/i18n/lib/i18n-core.jsghost/i18n/lib/i18n.browser.jsghost/i18n/lib/i18n.jsghost/i18n/lib/registry/comments.generated.mjsghost/i18n/lib/registry/comments.mjsghost/i18n/lib/registry/ghost.generated.mjsghost/i18n/lib/registry/ghost.mjsghost/i18n/lib/registry/index.generated.mjsghost/i18n/lib/registry/portal.generated.mjsghost/i18n/lib/registry/portal.mjsghost/i18n/lib/registry/search.generated.mjsghost/i18n/lib/registry/search.mjsghost/i18n/lib/registry/signup-form.generated.mjsghost/i18n/lib/registry/signup-form.mjsghost/i18n/lib/require-loader.jsghost/i18n/package.jsonghost/i18n/scripts/check-registry-fresh.jsghost/i18n/scripts/generate-locale-registry.jsghost/i18n/scripts/generate-types.jsghost/i18n/scripts/measure-bundle-locales.js
💤 Files with no reviewable changes (4)
- apps/portal/vite.config.mjs
- apps/signup-form/vite.config.mts
- apps/sodo-search/vite.config.mjs
- apps/comments-ui/vite.config.mts
ref PLA-209. Non-blocking cleanups from adversarial review:
- MINOR-1: nx build target outputs include {projectRoot}/build so generated
.d.ts types are cached/restored on Nx cache hits (via package.json nx.targets)
- MINOR-2: check-registry-fresh.js regenerates into a temp dir and diffs,
never mutating tracked lib/registry; fails hard on drift
- MINOR-3: dropped the root '.' ESM entry (all-namespaces footgun) — removed
import/browser conditions + deleted index.esm.mjs; apps must use /registry/<ns>
- MINOR-4: removed unreachable index.browser.js + lib/i18n.browser.js and their
test block; retargeted ESM behaviour tests onto per-namespace registry entries
- NIT-2: English floor — mergeDefaultExport(res || {}) so a missing en namespace
never throws; covered by direct createGenerateResources unit tests
- NIT-3: lint:code globs *.mjs and eslint config lints lib/esm-factory.mjs as ESM
i18n suite 41 tests, 100% coverage. Core i18n (theme-i18n e2e 9/9, theme-engine
units, email tests) green. All 4 public apps build with per-namespace tree-shaking.
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Contributor
E2E Tests FailedTo view the Playwright test report locally, run: REPORT_DIR=$(mktemp -d) && gh run download 28826200367 -n playwright-report -D "$REPORT_DIR" && npx playwright show-report "$REPORT_DIR" |
ref PLA-209
- comments-ui and signup-form acceptance tests were failing: both apps never
rendered because the built UMD bundle threw "require is not defined" at load
- the per-namespace registry path (registry/<ns> -> esm-factory.mjs) imported the
CJS lib/i18n-core.js, whose require('i18next') / require('./locale-data.json')
leaked verbatim into the browser bundle. This PR had removed the vite
commonjsOptions that previously transformed ghost/i18n, so nothing rewrote them
- fix keeps the browser graph pure ESM: added lib/i18n-core.mjs (static i18next +
JSON imports) as the twin of the CJS lib/i18n-core.js, and pointed esm-factory
at it. No package.json "browser" mapping reintroduced
- the two cores are deliberate twins (Node needs sync CJS, browser needs pure
ESM); guarded by a CJS/ESM parity test. CJS core logic is byte-identical to
before, so ghost core's Node path is unchanged
- verified: all 4 public apps build with 0 leaked require/module.exports; i18n
suite 45 tests / 100% coverage; lint + registry freshness green
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
ref PLA-209 - ship the two .d.ts as committed static files (index.d.ts, registry.d.ts) and drop scripts/generate-types.js + the build:types step. The "generator" only ever wrote two constant strings, so it was a build step producing fixed output — the files describe the surface directly now (typechecked clean for consumers) - build/ is no longer produced; removed it from files + nx build outputs. build now just runs build:registry - removed scripts/measure-bundle-locales.js — an ad-hoc bundle-analysis helper that didn't belong in the shipped package - trimmed the vite-public-app.mjs i18n comment to describe the current registry import, without the historical dynamicRequireTargets narrative
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ghost/i18n/registry.d.ts (1)
1-13: 🎯 Functional Correctness | 🔵 TrivialGeneric type doesn't narrow to the module's own namespace.
This declaration file appears to be shared across all
@tryghost/i18n/registry/<namespace>subpaths. Sincei18n: I18nFactoryinherits the call signature(locale?, ns?: Namespace | string, options?), a consumer importing from e.g.@tryghost/i18n/registry/portalcan still type-check a call likei18n('en', 'search')even though that namespace's registry module presumably only bundles its own namespace's locale data. Same applies to the standalonegenerateResources(locales, ns)export — nothing constrainsnsto the namespace this specific module was generated for.Consider whether a lightweight per-namespace parameterization (e.g. templating this
.d.tswith the concrete namespace literal during generation) would give callers real type safety here, since the current typing can silently accept mismatched namespace arguments.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ghost/i18n/registry.d.ts` around lines 1 - 13, The registry declaration is too generic and does not restrict calls to the module’s own namespace. Update the generated per-namespace typings in i18n/registry.d.ts so the exported i18n and generateResources signatures are parameterized with the concrete namespace literal for each subpath (for example, the namespace used by that registry module), rather than allowing any string. Use the existing i18n/I18nFactory and generateResources symbols as the insertion points, and ensure the namespace argument type is narrowed during generation so mismatched namespaces fail type-checking.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@ghost/i18n/registry.d.ts`:
- Around line 1-13: The registry declaration is too generic and does not
restrict calls to the module’s own namespace. Update the generated per-namespace
typings in i18n/registry.d.ts so the exported i18n and generateResources
signatures are parameterized with the concrete namespace literal for each
subpath (for example, the namespace used by that registry module), rather than
allowing any string. Use the existing i18n/I18nFactory and generateResources
symbols as the insertion points, and ensure the namespace argument type is
narrowed during generation so mismatched namespaces fail type-checking.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f869c793-936a-421c-81dc-febf225cd056
📒 Files selected for processing (4)
apps/_shared/vite-public-app.mjsghost/i18n/index.d.tsghost/i18n/package.jsonghost/i18n/registry.d.ts
✅ Files skipped from review due to trivial changes (1)
- ghost/i18n/index.d.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/_shared/vite-public-app.mjs
- ghost/i18n/package.json
ref PLA-209
- the per-namespace registry entries now collect locales via Vite's
`import.meta.glob('../../locales/*/<ns>.json', {eager, import: 'default'})`
instead of a committed, generated list of 62 static imports per namespace
- deletes the generator (generate-locale-registry.js), the freshness guard
(check-registry-fresh.js), all 5 *.generated.mjs + index.generated.mjs, and the
build:registry / prebuild:check / build scripts + nx build target. i18n is now
source-only (no build step)
- Vite-coupled by design: every consumer of this browser path builds with Vite
(rolldown-vite). This is the bundler we're standardising on, so coupling the
locale collection to it is fine — and import.meta.glob's lazy mode is the
natural basis for per-locale lazy loading later (PLA-210)
- registry entries must now be Vitest-transformed (not externalized) for the glob
to resolve; dropped the stale externalization (its CJS i18n-core dedup reason no
longer applies now the ESM path uses the i18n-core.mjs twin)
- verified: byte-identical output to the generated approach (portal 2363 vs
2361 KB), per-namespace scoping intact (presence matrix clean), all 4 apps build
with no require/module/glob leaks, i18n suite 45 tests / 100% coverage
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
ghost/i18n/lib/registry/signup-form.mjs (1)
6-15: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winSame duplicated registry-building logic as
ghost.mjs.See the extraction suggestion in
ghost.mjs— applies identically here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ghost/i18n/lib/registry/signup-form.mjs` around lines 6 - 15, The registry-building logic in signup-form.mjs duplicates the same locale registration flow already found in ghost.mjs. Extract the shared `import.meta.glob` plus `REGISTRY` construction into a common helper and have `createNamespacedI18n` callers reuse it, so `signup-form.mjs` no longer manually loops over `modules` and parses locale keys itself.ghost/i18n/lib/registry/portal.mjs (1)
6-15: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winSame duplicated registry-building logic as
ghost.mjs.See the extraction suggestion in
ghost.mjs— applies identically here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ghost/i18n/lib/registry/portal.mjs` around lines 6 - 15, The registry-building logic in portal.mjs is duplicated from ghost.mjs, so extract the shared locale registry creation into the same reusable helper used there and have this module reuse it instead of rebuilding REGISTRY inline. Update the portal registry setup around createNamespacedI18n, import.meta.glob, and the REGISTRY population loop to delegate to the shared extraction so both modules stay consistent.ghost/i18n/lib/registry/search.mjs (1)
6-15: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winSame duplicated registry-building logic as
ghost.mjs.See the extraction suggestion in
ghost.mjs— applies identically here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ghost/i18n/lib/registry/search.mjs` around lines 6 - 15, The registry-building logic in the search namespace is duplicated from ghost.mjs; extract the shared module registry construction into a reusable helper and have both createNamespacedI18n callers use it. Refactor the REGISTRY initialization loop that iterates over import.meta.glob results so the locale-to-resource mapping is centralized, keeping the search.mjs setup aligned with the same shared implementation used elsewhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ghost/i18n/lib/registry/ghost.mjs`:
- Around line 6-15: The REGISTRY-building loop in ghost.mjs is duplicated across
multiple namespace modules and should be centralized in esm-factory.mjs. Add a
shared helper there (next to createNamespacedI18n) that takes the glob modules
object, parses the locale from each filePath, and returns the registry, then
update ghost.mjs to call it instead of constructing REGISTRY inline. Apply the
same helper from portal.mjs, search.mjs, and signup-form.mjs so the locale
parsing and registry creation live in one place.
---
Duplicate comments:
In `@ghost/i18n/lib/registry/portal.mjs`:
- Around line 6-15: The registry-building logic in portal.mjs is duplicated from
ghost.mjs, so extract the shared locale registry creation into the same reusable
helper used there and have this module reuse it instead of rebuilding REGISTRY
inline. Update the portal registry setup around createNamespacedI18n,
import.meta.glob, and the REGISTRY population loop to delegate to the shared
extraction so both modules stay consistent.
In `@ghost/i18n/lib/registry/search.mjs`:
- Around line 6-15: The registry-building logic in the search namespace is
duplicated from ghost.mjs; extract the shared module registry construction into
a reusable helper and have both createNamespacedI18n callers use it. Refactor
the REGISTRY initialization loop that iterates over import.meta.glob results so
the locale-to-resource mapping is centralized, keeping the search.mjs setup
aligned with the same shared implementation used elsewhere.
In `@ghost/i18n/lib/registry/signup-form.mjs`:
- Around line 6-15: The registry-building logic in signup-form.mjs duplicates
the same locale registration flow already found in ghost.mjs. Extract the shared
`import.meta.glob` plus `REGISTRY` construction into a common helper and have
`createNamespacedI18n` callers reuse it, so `signup-form.mjs` no longer manually
loops over `modules` and parses locale keys itself.
🪄 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: 0c91a0ac-a023-40ae-88a8-eb3bd08a810f
📒 Files selected for processing (7)
ghost/i18n/lib/registry/comments.mjsghost/i18n/lib/registry/ghost.mjsghost/i18n/lib/registry/portal.mjsghost/i18n/lib/registry/search.mjsghost/i18n/lib/registry/signup-form.mjsghost/i18n/package.jsonghost/i18n/vitest.config.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- ghost/i18n/lib/registry/comments.mjs
ref PLA-209 - the per-namespace browser entries were copy-pasting the glob->registry loop; moved it into esm-factory as i18nFromGlob(globModules, namespace) - each entry is now just the literal glob pattern + namespace name (the only bits Vite requires to be per-file, since import.meta.glob patterns must be literals) plus the standard re-exports — everything else is shared
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
9larsons
changed the title
9larsons
added a commit
that referenced
this pull request
Jul 7, 2026
ref https://linear.app/ghost/issue/PLA-96 - adds a `rolldown` named catalog (vite: npm:rolldown-vite@7.3.1) and points the 8 admin-surface apps (posts, stats, admin, admin-x-framework, admin-x-settings, admin-x-design-system, shade, activitypub) at catalog:rolldown - public UMD apps stay on stock vite until their own Rolldown config lands (#29141) - the known Rolldown interop breakers are already fixed on main: shade Flag ESM swap (#29139), @svg-maps/world shim (#29143), @tryghost/i18n dual-format (#29144) - verified: all 8 apps + the admin shell build clean under Rolldown; react/react-dom stay external (no dual-instance regression); only benign bundler-agnostic warnings
9larsons
added a commit
that referenced
this pull request
Jul 7, 2026
ref https://linear.app/ghost/issue/PLA-96 - points the 5 public apps (portal, comments-ui, sodo-search, signup-form, announcement-bar) at catalog:rolldown, putting the whole repo on Rolldown — the prerequisite for moving to Vite 8 (which ships Rolldown as default) - removes portal's rollupOptions.output.manualChunks:false — Rolldown rejects it and it was redundant (UMD lib mode is single-file anyway) - no i18n locale-bundling plugin needed: the merged @tryghost/i18n dual-format work (#29144) already handles per-namespace locale bundling via import.meta.glob, superseding this branch's original readdir-based plugin - verified: all 5 apps build clean under Rolldown, 0 require/module leaks, per-namespace locale scoping intact, UMD sizes match the Rollup builds
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
1 participant
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.
Ref PLA-209.
What & why
@tryghost/i18nwas CJS-only with a dynamicrequire(\../locales/${locale}/${ns}.json`)locale loader. That's thedynamicRequireTargetspattern Rolldown never implemented, so browser bundles built with rolldown-vite **silently drop all non-English locales**. Today's public apps paper over it with a per-appcommonjsOptions.dynamicRequireTargetsshim inapps/_shared/vite-public-app.mjs`.This makes
@tryghost/i18na dual-format package:require→ unchanged CJS Node entry (index.js). Keeps the synchronous, fs-backed theme locale loading thatghost/core's Handlebars{{t}}helper depends on. Zero changes to core.import/browser→ static ESM per-namespace registries. Each public app imports@tryghost/i18n/registry/<namespace>and statically bundles ONLY its namespace's 62 locale files — no dynamic require, no shim, works under any bundler.The dynamic-require loader is isolated in
lib/require-loader.jsso no ESM/shared path ever imports it.Per-namespace bundle evidence (critical)
Each public app now bundles only its own namespace's locales. Verified via sourcemap (which
registry/*.generated.mjsand how many of each namespace's*.jsonare included) and by string-presence of non-English translations:registry/portal.generated.mjsonlyregistry/comments.generated.mjsonlyregistry/signup-form.generated.mjsonlyregistry/search.generated.mjsonlyRaw per-namespace locale payloads: portal 1.55 MB, ghost 0.50, comments 0.24, signup-form 0.04, search 0.02 (all-5 = 2.35 MB). Portal carries only its 1.55 MB — not the 3.2/2.35 MB of all namespaces. Portal bundle vs
main: 2.42 MB → 2.37 MB (equivalent; main's old shim was also single-namespace, but broke under Rolldown — see below).Both bundlers verified
Isolated harness importing
@tryghost/i18n/registry/portal:This is the whole point: under rolldown-vite the old approach ships a bundle with essentially no translations; the static registry ships all 62.
Core untouched — test proof
require('@tryghost/i18n')still resolves toindex.js;nl→"Back"verified.theme-i18n: 9/9 (boots Ghost, changes locale via Admin API, renders{{t}}against real on-disk theme locale files — the fs-backed path).theme-i18n1/1,i18n10/10,i18next8/8.email-renderer186/186,email-helpers15/15,comments-service-emails-renderer3/3,member-welcome-emails-snapshot(integration) 4/4.@tryghost/i18nown suite: 37/37.with {type:'json'}import attribute resolves).