Moved ghost/core boundary enforcement to dependency-cruiser#29134
Moved ghost/core boundary enforcement to dependency-cruiser#29134ErisDS wants to merge 1 commit into
Conversation
no ref The frontend/server/shared boundaries were enforced by the custom `ghost/node/no-restricted-require` rule in ghost/core/eslint.config.mjs. That rule is specific to eslint-plugin-ghost and has no equivalent in oxlint, so it blocks the planned ESLint-to-oxlint migration. It also only inspects require() calls, missing ESM imports. The four boundary rules now run on dependency-cruiser, which enforces them on the resolved module graph (require and import) from the repo root. This drops the custom rule and lets the boundaries outlive ESLint. The original burn-down intent — "adding files to this list is an anti-pattern / work down until the list is empty" — and the per-file allowlist categorisation are carried over verbatim from the pre-flat- config .eslintrc.js, having been lost in the flat-config migration. Enforced in three places: a cached nx target on the ghost-monorepo root project (scoped inputs so the whole-repo projectRoot glob doesn't defeat caching), a CI step, and a lint-staged pre-commit signal. The cross-package apps/library model is deliberately out of scope and tabled as a follow-up.
|
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. |
WalkthroughThis PR replaces ESLint-based architectural boundary enforcement with dependency-cruiser. A new Sequence Diagram(s)flowchart TD
Developer --> GitCommit
GitCommit --> LintStagedBoundaryCheck
CI --> NxLintBoundariesTarget
LintStagedBoundaryCheck --> DepcruiseCLI
NxLintBoundariesTarget --> DepcruiseCLI
DepcruiseCLI --> DependencyCruiserConfig
DependencyCruiserConfig --> ForbiddenRuleEvaluation
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
package.json (1)
63-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider
err-longoutput for clearer boundary-violation diagnostics.Default depcruise output is terse;
--output-type err-longincludes the rule'scomment/namein the failure output, which would help contributors working through the documented allowlist ("Goal: Work down until only proxy.js remains").♻️ Optional diff
- "lint:boundaries": "depcruise ghost/core/core --config .dependency-cruiser.cjs", + "lint:boundaries": "depcruise ghost/core/core --config .dependency-cruiser.cjs --output-type err-long",Also applies to: 122-124
🤖 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 `@package.json` at line 63, The lint:boundaries depcruise script currently uses terse default output, so update the lint:boundaries command to use err-long for clearer boundary-violation diagnostics. Adjust the package.json script that runs depcruise so contributors get the rule comment/name in failures, making it easier to work through the allowlist guidance and locate violations in ghost/core/core.
🤖 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 `@package.json`:
- Line 63: The lint:boundaries depcruise script currently uses terse default
output, so update the lint:boundaries command to use err-long for clearer
boundary-violation diagnostics. Adjust the package.json script that runs
depcruise so contributors get the rule comment/name in failures, making it
easier to work through the allowlist guidance and locate violations in
ghost/core/core.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 17b3d2f3-731d-441a-9bc9-a04fc284a5f9
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
.dependency-cruiser.cjs.github/workflows/ci.yml.lintstagedrc.cjsghost/core/eslint.config.mjspackage.jsonpnpm-workspace.yaml
💤 Files with no reviewable changes (1)
- ghost/core/eslint.config.mjs
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | <1s | View ↗ |
nx run @tryghost/admin-x-settings:test:acceptance |
✅ Succeeded | 10m 24s | View ↗ |
nx run-many -t test:unit -p @tryghost/admin-x-d... |
✅ Succeeded | 8m 58s | View ↗ |
nx run ghost:test:integration |
✅ Succeeded | 2m 55s | View ↗ |
nx run ghost:test:legacy |
✅ Succeeded | 2m 57s | View ↗ |
nx run ghost:test:ci:integration |
✅ Succeeded | 2m 33s | View ↗ |
nx run ghost:test:e2e |
✅ Succeeded | 2m 26s | View ↗ |
nx run ghost-monorepo:lint:boundaries |
✅ Succeeded | <1s | View ↗ |
Additional runs (8) |
✅ Succeeded | ... | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-07-06 15:45:16 UTC
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #29134 +/- ##
==========================================
- Coverage 74.27% 74.26% -0.01%
==========================================
Files 1565 1565
Lines 135716 135728 +12
Branches 16457 16453 -4
==========================================
- Hits 100802 100801 -1
- Misses 33888 33927 +39
+ Partials 1026 1000 -26
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
Why
ghost/core's frontend/server/shared layer boundaries are enforced by the customghost/node/no-restricted-requirerule inghost/core/eslint.config.mjs. That rule is specific toeslint-plugin-ghostand has no equivalent in oxlint, so it stands in the way of the planned ESLint → oxlint migration. It also only inspectsrequire()calls, so it misses ESMimports.What
Moves the four boundary rules onto dependency-cruiser, which enforces them on the resolved module graph (both
requireandimport) from the repo root:core/sharedmust not depend oncore/serverorcore/frontendcore/frontendmust not reachcore/server/modelsdirectlycore/frontendmay cross tocore/serveronly via theservices/proxyseam (allowlisted)core/servermust not depend oncore/frontend(allowlisted)The custom
no-restricted-requireblocks (and their now-unusedpath/__dirnameimports) are removed from the ESLint config. The original burn-down intent — "adding files to this list is an anti-pattern / work down until the list is empty" — and the per-file allowlist categorisation are carried over verbatim from the pre-flat-config.eslintrc.js(they'd been dropped in the flat-config migration).Enforced in three places:
ghost-monoreporoot project (lint:boundaries), with scopedinputsso the root project's whole-repoprojectRootglob doesn't defeat cachingghost/core/corefilesBehaviour is unchanged: parity verified (0 violations on the same graph the ESLint rules covered), and each rule still fires on a negative test.
Scope
The cross-package
apps/*boundary model (foundation-libraries vs apps) surfaced during this work but is deliberately out of scope here and tabled as a follow-up.Checklist