Added public api surfaces to the admin domains - #30317
Conversation
no ref The admin shell (routes.tsx), the layout, and several domains deep-imported other domains' internals, so every screen move risked breaking unrelated folders. Each domain (members, tags, comments, automations, posts, onboarding, whats-new, settings) now exposes an api.ts mirroring analytics/api.ts, and the shell, layout, and cross-domain consumers import only those surfaces. Route definitions the shell held inline (members children, the posts analytics subtree) moved into their domains; lazy entries stay dynamic imports inside the domains so route-level code-splitting is unchanged (production build emits an identical chunk inventory). Cross-domain strays were re-homed: buildMembersUrl, the member formatting helpers, and the shared-views parsing now flow through members/api.ts; the whats-new dialog, banner, and hooks through whats-new/api.ts; BrandIcon and its svg set moved from settings to src/shared/brand-icon; the AccessRule types moved to src/route-access.ts and NotFound to src/shared so domains no longer import shell files. Two dependency-cruiser rules keep it that way: domains may import a different domain only via its api.ts, and the shell and layout may import domains only via api.ts. Both rules were proven to fire on deliberate violations in the unresolved @/-specifier and resolved relative shapes, then the violations were reverted.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: QUIET Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (2)
🔇 Additional comments (2)
WalkthroughThe admin application now exposes domain functionality through public Suggested reviewers: Merge Risk: ⚪ Minimal · up to This PR reorganizes admin domain exports and imports while preserving code-splitting and existing production behavior; no actionable merge-blocking risk remains after the reported checks. 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
Full details: Type-Safe BoundariesExplanation PASS — The PR diff adds API re-exports, lazy dynamic imports, route definitions, and import-path changes. It adds no HTTP/SDK, environment, database/filesystem, queue, webhook, or event-payload consumption. The added source contains no Full details: New Files Are TypescriptExplanation PASS. In the full PR diff ( ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run @tryghost/admin:test:acceptance |
✅ Succeeded | 7m 44s | View ↗ |
nx run ghost-monorepo:lint:boundaries |
✅ Succeeded | 19s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 5s | View ↗ |
nx run-many -t test:unit -p @tryghost/admin |
✅ Succeeded | 6s | View ↗ |
nx run-many -t lint -p ghost-monorepo,@tryghost... |
✅ Succeeded | 4s | View ↗ |
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | <1s | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-08-26 13:11:39 UTC
no ref Review follow-up: the shell-into-domains rule policed only top-level src files and layout/; hooks/, providers/, ember-bridge/, utils/ and schemas/ could still deep-import domain internals. The from-clause now covers the non-domain support folders; proven firing from hooks/ with a deliberate violation, then reverted.
E2E Tests FailedTo view the Playwright test report locally, run: REPORT_DIR=$(mktemp -d) && gh run download 32970642110 -n playwright-report -D "$REPORT_DIR" && npx playwright show-report "$REPORT_DIR" |
The shell (
routes.tsx,layout/) deep-imported domain internals, and only analytics had a declared public surface. Every domain now exposes anapi.tsmirroringanalytics/api.ts, and dependency-cruiser keeps it that way.api.ts: members and posts/analytics also gain their ownroutes.tsxfragments (their route definitions move out of the shell table, mirroring analytics); tags/comments/automations/onboarding/whats-new/settings export exactly what the shell and other domains consume. Lazy entries are exported thunks — dynamicimport()s stay inside the domains, so code-splitting is preserved: 358 chunks before and after, name multiset identical (entry +0.4 kB from the small members helpers now in the entry graph). Settings internals untouched beyond import lines (draft Changed settings screens to read the framework data hooks #30285 owns them).buildMembersUrl+ member-format helpers + shared-views parsing → viamembers/api(posts/comments/settings/layout consumers updated); whats-new pieces →whats-new/api;BrandIcon+ its 20 SVGs →src/shared/brand-icon/(git-mv, history preserved);AccessRule/AccessRouteHandle→ newsrc/route-access.tstypes module;NotFound→src/shared/. One upward import deliberately left:latest-post.tsx→useIsEmberOwnedRoutefrom@/routes(moving it drags the routes table; dies with Ember).admin-domains-cross-via-api-only(domain → other domain only viaapi.ts, own-domain exempt via group capture) andadmin-shell-into-domains-via-api-only(src-root + layout → domains viaapi.ts). Both cover the@/specifier and resolved-path shapes; proven firing with three deliberate violations (both rules, both shapes), then reverted — and the clean pass over ~870 same-domain self-imports proves the exemption.67 files, +255/−117.
Verification:
tsc -bclean; eslint 0 errors;test:unit140 files / 1644; acceptance across route-access/settings-routing/home/tags/members/analytics/automations/whats-new 8 files / 41, no flakes;pnpm lint:boundariesclean (5,285 modules); production-build chunk comparison as above; oxfmt clean on touched files.