Changed routing events to carry domain data instead of Express routers

[vershwal]

ref https://linear.app/ghost/issue/HKG-1899

The last piece of code in the DDD Architecture Cleanup milestone (design step 3.8).

The problem

RouterManager.routerCreated() broadcast a live Express-backed router instance:

routingEvents.emit('router.created', router);

The sitemap — the only listener — then reached into that object for three things:

if (router.name !== 'StaticRoutesRouter' && router.name !== 'CollectionRouter') return;
const entry = {
    url: router.getRoute({absolute: true}),
    datum: {id: router.identifier, staticRoute: router.name === 'StaticRoutesRouter'}
};

Two frontend subsystems coupled through an infrastructure handle rather than a message. Change a router's method surface and static/collection routes silently stop appearing in the sitemap, with nothing to catch it.

The change

RouteRegistered { path, type, id }   // path in domain notation, e.g. '/about/'
RoutesReset                          // no payload

RouterManager emits data; the sitemap filters on type and absolutises path itself.

Three decisions worth reviewing

1. path (domain notation), not a pre-rendered absolute URL.
This matches the issue spec and the design doc's Layer-2 table (RouteRegistered { path, type, id }). Our internal plan doc had argued for an emitter-computed absolute url on the grounds that the sitemap would otherwise "have to reach back for urlUtils" — that premise was wrong: site-map-manager.js:2 already imported urlUtils for its /404/ sentinel. So path costs nothing and keeps the event domain-level instead of carrying rendered output.

The conversion is provably lossless. ParentRouter.getRoute() and CollectionRouter.getRoute() (the only two emitting types) are both exactly urlUtils.createUrl(this.route.value, options.absolute), and the subscriber now calls urlUtils.createUrl(path, true) on that same value. Verified identical by instantiating the real router classes under three configs — root, http://localhost/blog/, https://example.com/blog/ — across /, /about/, /feed/, /podcast/, /hello/world/. Subdirectory installs included.

2. Kept the frontend-internal EventEmitter rather than @tryghost/domain-events.
The issue allows either. Two reasons for local:

• 729891bc00 deliberately created frontend/services/routing/events.js to move these events off the server's shared bus, as part of taking these modules off the dependency-cruiser allowlist. Adopting DomainEvents — a process-wide static singleton — reverses that.
• DomainEvents.subscribe wraps every handler in try/catch and only logs (DomainEvents.js:33-39). The sitemap's handler mutates _routerEntries and invalidates the index; a swallowed throw there is a silently stale sitemap.

3. path is null for routers that own no route.
StaticPagesRouter has no this.route and TaxonomyRouter has no index route (/tag/ doesn't exist). Previously invisible because the subscriber's name filter ran before it ever called getRoute(); the type filter still runs before path is used, so createUrl(null, …) is unreachable for the two live types.

type deliberately carries the router's name — a hand-passed super() string literal, not constructor.name, so it's already decoupled from the class name. Re-vocabularising it to static-route/collection would add exactly the mapping layer the design doc's "Keeping it lean" section warns against, for no behavioural gain.

No backwards-compat shim

The issue offers keeping router.created "if other consumers exist". None do — repo-wide search across .js/.ts/.tsx/.json/.hbs (including apps/, packages/, e2e/, and computed event names) found only the two emitters, the one consumer, and their tests. grep -rn "router.created\|routers.reset" ghost/core/core ghost/core/test now returns nothing.

Second commit — toDomainNotation()

A pure deletion, split into its own commit for blame. It's the milestone's documented last loose end. It never had a production caller, and its docstring's promise of one on the download path is provably obsolete: DynamicRoutingService.download() returns settings.yamlSource verbatim and never re-serialises.

Testing

Behaviour is intended to be byte-identical; the tests were written first.

• router-manager.test.js — new domain events block (5 → 7 tests): payload shape pinned by Object.keys(event).sort(), the null-path case, and the ordering the sitemap depends on (RoutesReset always precedes every registration that refills the entries it emptied).
• manager.test.js — the hand-built fake router becomes a plain payload.
• Full pnpm test:unit: matches pristine main exactly (same 2 pre-existing failures in automations-repository and email-renderer, both unrelated and reproducing on main).
• test/e2e-frontend/ — 18 files / 259 tests green. This is the real safety net: default-routes.test.js and custom-routes.test.js render actual sitemap XML through the real emit→subscribe path, and advanced-url-config.test.js covers the subdirectory install.
• --project legacy — 35 files / 451 tests green.
• pnpm lint:boundaries — 0 violations across 5,137 modules; no new cross-context coupling.

The one tsc error locally (values-service.ts / subFieldsOf) reproduces on pristine main — a stale local workspace-package build, not this branch.

Reviewed before opening by three independent agents (clean code, production safety, scope/design alignment); their actionable findings are already folded in.

[nx-cloud]

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci

---

View your CI Pipeline Execution ↗ for commit 11d3ce1

Command	Status	Duration	Result
nx run ghost:test:ci:integration	✅ Succeeded	3m 1s	View ↗
nx run ghost:test:integration	✅ Succeeded	2m 33s	View ↗
nx run ghost:test:e2e	✅ Succeeded	2m 12s	View ↗
nx run ghost:test:legacy	✅ Succeeded	2m 13s	View ↗
nx run ghost-monorepo:lint:boundaries	✅ Succeeded	21s	View ↗
nx run-many -t test:unit -p ghost	✅ Succeeded	31s	View ↗
nx run-many -t lint -p ghost,ghost-monorepo	✅ Succeeded	20s	View ↗
nx run-many --target=build --projects=tag:publi...	✅ Succeeded	<1s	View ↗
nx run @tryghost/admin:build	✅ Succeeded	8s	View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗

---

☁️ Nx Cloud last updated this comment at 2026-08-06 12:12:00 UTC

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a59758e1-bf81-4d40-8e2e-ce222efa0c10

📥 Commits

Reviewing files that changed from the base of the PR and between 11d3ce1 and 0a26769.

📒 Files selected for processing (2)

• ghost/core/core/frontend/services/routing/router-manager.js
• ghost/core/test/unit/frontend/services/sitemap/manager.test.js

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

• ghost/core/test/unit/frontend/services/sitemap/manager.test.js
• ghost/core/core/frontend/services/routing/router-manager.js

---

Walkthrough

Frontend routing now emits RouteRegistered and RoutesReset events with plain route data. Sitemap management consumes these payloads to build URLs and reset route state. Router-manager tests verify event ordering, payload fields, null paths, and serializable data. Sitemap tests cover registration and reset behavior. Obsolete toDomainNotation tests were removed.

Possibly related PRs

• TryGhost/Ghost#29451: Related frontend routing event changes use the same routing services and tests.
• TryGhost/Ghost#29662: Related changes modify the permalink-adapter.ts notation helper.
• TryGhost/Ghost#29742: Related changes modify the same toDomainNotation helper.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: routing events now carry domain data instead of Express router instances.
Description check	✅ Passed	The description directly explains the routing event changes, payload design, helper removal, compatibility decision, and validation results.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch princi-hkg-1899-introduce-domain-events-for-routing

---

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

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

Refactors Ghost’s frontend-internal routing events so subscribers (notably the sitemap) receive domain data ({path, type, id}) rather than a live Express-backed router instance, reducing infrastructure coupling as part of the DDD architecture cleanup.

Changes:

• RouterManager now emits RouteRegistered {path, type, id} and RoutesReset instead of router.created / routers.reset.
• SiteMapManager now derives absolute URLs from the event’s domain path using urlUtils.createUrl.
• Removes unused toDomainNotation() from the permalink adapter and updates affected unit tests.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
ghost/core/core/frontend/services/routing/router-manager.js	Emit domain payloads (RouteRegistered, RoutesReset) instead of Express router instances.
ghost/core/core/frontend/services/sitemap/site-map-manager.js	Consume new routing domain events and build sitemap entries from {path, type, id}.
ghost/core/core/frontend/services/routing/events.js	Documents the new routing domain event payloads and intent.
ghost/core/core/frontend/services/routing/permalink-adapter.ts	Removes unused toDomainNotation() helper.
ghost/core/test/unit/frontend/services/routing/permalink-adapter.test.js	Updates tests to reflect removal of toDomainNotation().
ghost/core/test/unit/frontend/services/routing/router-manager.test.js	Adds/updates unit tests pinning the new event payload shape and ordering.
ghost/core/test/unit/frontend/services/sitemap/manager.test.js	Updates sitemap unit tests to use RouteRegistered / RoutesReset payloads.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

[codecov]

Codecov Report

❌ Patch coverage is 97.43590% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 75.41%. Comparing base (125050e) to head (0a26769).

Files with missing lines	Patch %	Lines
...e/core/frontend/services/routing/router-manager.js	90.90%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #29801      +/-   ##
==========================================
- Coverage   75.45%   75.41%   -0.04%     
==========================================
  Files        1606     1606              
  Lines      140364   140371       +7     
  Branches    17408    17394      -14     
==========================================
- Hits       105905   105858      -47     
- Misses      33410    33436      +26     
- Partials     1049     1077      +28     

Flag	Coverage Δ
e2e-tests	77.56% <97.43%> (-0.05%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.