Skip to content

Moved ghost/core boundary enforcement to dependency-cruiser #29134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ErisDS wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Moved ghost/core boundary enforcement to dependency-cruiser #29134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 94 additions & 0 deletions .dependency-cruiser.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
'use strict';

/*
* Architectural boundary rules for ghost/core, enforced on the resolved module
* graph. These mirror the `ghost/node/no-restricted-require` rules that used to
* live in ghost/core/eslint.config.mjs; moving them here lets us drop the
* custom eslint-plugin-ghost rule when migrating off ESLint, and catches ESM
* imports the require-only rule could not.
*
* Run from the repo root, so paths are anchored on `^ghost/core/core/`.
*
* @type {import('dependency-cruiser').IConfiguration}
*/
module.exports = {
forbidden: [
// ============================================================
// shared/ must not require server/* or frontend/*
// ============================================================
{
name: 'shared-not-server-or-frontend',
comment: 'Invalid require of core/server or core/frontend from core/shared.',
severity: 'error',
from: {path: '^ghost/core/core/shared/'},
to: {path: '^ghost/core/core/(server|frontend)/'}
},
// ============================================================
// Frontend must not require server/models directly
// ============================================================
{
name: 'frontend-not-server-models',
comment: 'Invalid require of core/server/models from core/frontend. Fetch content through the public Content API (api.postsPublic / api.pagesPublic), injected via core/frontend/services/proxy — not the model layer directly. See #28420.',
severity: 'error',
from: {path: '^ghost/core/core/frontend/'},
to: {path: '^ghost/core/core/server/models/'}
},
// ============================================================
// Frontend must cross to server only via proxy (with allowlist)
// ============================================================
{
name: 'frontend-to-server-via-proxy-only',
comment: 'Invalid require of core/server from core/frontend. Cross only via the proxy seam (core/frontend/services/proxy.js).',
severity: 'error',
from: {
path: '^ghost/core/core/frontend/',
// Adding files to this list is an anti-pattern
// Goal: Work down until only proxy.js remains
pathNot: [
// The sanctioned seam.
'^ghost/core/core/frontend/services/proxy\\.js$',

// Composition root wiring (less wrong).
'^ghost/core/core/frontend/web/site\\.js$',
'^ghost/core/core/frontend/web/middleware/frontend-caching\\.js$',
'^ghost/core/core/frontend/web/middleware/handle-image-sizes\\.js$',
'^ghost/core/core/frontend/web/routers/link-redirects\\.js$',
'^ghost/core/core/frontend/web/routers/serve-favicon\\.js$',
'^ghost/core/core/frontend/apps/private-blogging/lib/router\\.js$',

// Leaks that bypass the proxy (fix first).
'^ghost/core/core/frontend/services/routing/controllers/unsubscribe\\.js$', // services/members + settings-helpers
'^ghost/core/core/frontend/services/routing/router-manager\\.js$', // server/lib/common/events bus
'^ghost/core/core/frontend/services/sitemap/site-map-manager\\.js$' // server/lib/common/events bus
]
},
to: {path: '^ghost/core/core/server/'}
},
// ============================================================
// Server must not require frontend (with allowlist)
// ============================================================
{
name: 'server-not-frontend',
comment: 'Invalid require of core/frontend from core/server. The server must not depend on the frontend rendering layer.',
severity: 'error',
from: {
path: '^ghost/core/core/server/',
// Adding files to this list is an anti-pattern
// Goal: Work down until the list is empty
pathNot: [
// Composition root: mounts the frontend Express app onto the server (less wrong).
'^ghost/core/core/server/web/parent/frontend\\.js$',

// Leak: reaches into the frontend routing config for QUERY/TAXONOMIES (fix first — config should be injected, see the in-file TODO).
'^ghost/core/core/server/services/route-settings/validate\\.js$'
]
},
to: {path: '^ghost/core/core/frontend/'}
}
],
options: {
doNotFollow: {path: 'node_modules'},
exclude: {path: '(^|/)(node_modules|coverage|coverage-next|test|built)/'}
}
};

7 changes: 7 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,13 @@ jobs:
NX_BASE: ${{ needs.job_setup.outputs.nx_base }}
NX_HEAD: ${{ env.HEAD_COMMIT }}

# Architectural boundary rules (dependency-cruiser). Runs from the repo
# root against the resolved module graph, so it sits outside the nx
# per-project lint target above — as its own root-project target, cached
# on the config + ghost/core/core sources. No build required.
- name: Lint boundaries
run: pnpm nx run ghost-monorepo:lint:boundaries

- uses: tryghost/actions/actions/slack-build@4d127f5388618fd9dc260e8ef4a64777f8b564cb # main
if: failure() && github.event_name == 'push' && github.ref == 'refs/heads/main'
with:
Expand Down
15 changes: 14 additions & 1 deletion .lintstagedrc.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,17 @@ function buildCommand(workspace, files) {
return `pnpm ${dirArg}exec eslint --cache -- ${relativeFiles}`;
}

// Early signal for architectural layer boundaries. Runs dependency-cruiser
// (root config) on just the staged files; the full graph is cruised in CI via
// `pnpm lint:boundaries`. Paths are root-relative to match the root config.
function buildBoundaryCommand(files) {
const relativeFiles = files
.map(file => normalize(path.relative(ROOT, file)))
.map(shellQuote)
.join(' ');
return `pnpm exec depcruise --config .dependency-cruiser.cjs -- ${relativeFiles}`;
}

module.exports = {
'*.{js,ts,tsx,jsx,cjs}': (files) => {
const groups = new Map();
Expand All @@ -104,5 +115,7 @@ module.exports = {
return [...groups.entries()].map(([workspace, wsFiles]) =>
buildCommand(workspace || null, wsFiles)
);
}
},
'ghost/core/core/{server,shared,frontend}/**/*.{js,ts}': (files) =>
buildBoundaryCommand(files)
};
77 changes: 0 additions & 77 deletions ghost/core/eslint.config.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import path from 'node:path';
import js from '@eslint/js';
import globals from 'globals';
import ghostPlugin from 'eslint-plugin-ghost';
Expand All @@ -13,8 +12,6 @@ import {
strictLinterOptions
} from '../../eslint.shared.mjs';

const __dirname = import.meta.dirname;

// nodeLibRules + jsUnusedVarsRule covers the bulk. Two workspace-specific:
// turn off ghost/filenames/match-regex (we use local-filenames/match-regex in
// scoped blocks below) and no-unused-private-class-members (codebase has
Expand Down Expand Up @@ -188,19 +185,6 @@ export default tseslint.config(
}
},
// ============================================================
// shared/ must not require server/* or frontend/*
// ============================================================
{
files: ['core/shared/**'],
plugins: {ghost: ghostPlugin},
rules: {
'ghost/node/no-restricted-require': ['error', [
{name: path.resolve(__dirname, 'core/server/**'), message: 'Invalid require of core/server from core/shared.'},
{name: path.resolve(__dirname, 'core/frontend/**'), message: 'Invalid require of core/frontend from core/shared.'}
]]
}
},
// ============================================================
// schema.js: no created_by/updated_by columns
// ============================================================
{
Expand Down Expand Up @@ -271,67 +255,6 @@ export default tseslint.config(
}
},
// ============================================================
// Frontend must not require server/models directly
// ============================================================
{
files: ['core/frontend/**'],
plugins: {ghost: ghostPlugin},
rules: {
'ghost/node/no-restricted-require': ['error', [
{
name: [path.resolve(__dirname, 'core/server/models/**')],
message: 'Invalid require of core/server/models from core/frontend. Fetch content through the public Content API (api.postsPublic / api.pagesPublic), injected via core/frontend/services/proxy — not the model layer directly. See #28420.'
}
]]
}
},
// ============================================================
// Frontend must cross to server only via proxy (with allowlist)
// ============================================================
{
files: ['core/frontend/**'],
ignores: [
'core/frontend/services/proxy.js',
'core/frontend/web/site.js',
'core/frontend/web/middleware/frontend-caching.js',
'core/frontend/web/middleware/handle-image-sizes.js',
'core/frontend/web/routers/link-redirects.js',
'core/frontend/web/routers/serve-favicon.js',
'core/frontend/apps/private-blogging/lib/router.js',
'core/frontend/services/routing/controllers/unsubscribe.js',
'core/frontend/services/routing/router-manager.js',
'core/frontend/services/sitemap/site-map-manager.js'
],
plugins: {ghost: ghostPlugin},
rules: {
'ghost/node/no-restricted-require': ['error', [
{
name: [path.resolve(__dirname, 'core/server/**')],
message: 'Invalid require of core/server from core/frontend. Cross only via the proxy seam (core/frontend/services/proxy.js).'
}
]]
}
},
// ============================================================
// Server must not require frontend (with allowlist)
// ============================================================
{
files: ['core/server/**'],
ignores: [
'core/server/web/parent/frontend.js',
'core/server/services/route-settings/validate.js'
],
plugins: {ghost: ghostPlugin},
rules: {
'ghost/node/no-restricted-require': ['error', [
{
name: [path.resolve(__dirname, 'core/frontend/**')],
message: 'Invalid require of core/frontend from core/server. The server must not depend on the frontend rendering layer.'
}
]]
}
},
// ============================================================
// Test files
// ============================================================
{
Expand Down
15 changes: 15 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
"knip": "knip",
"knip:fix": "knip --fix --allow-remove-files=false",
"lint": "pnpm nx run-many -t lint",
"lint:boundaries": "depcruise ghost/core/core --config .dependency-cruiser.cjs",
"test": "pnpm nx run-many -t test --exclude @tryghost/e2e --exclude ghost-admin",
"test:unit": "pnpm nx run-many -t test:unit",
"test:watch": "vitest",
Expand All @@ -84,6 +85,7 @@
"@playwright/test": "catalog:",
"@secretlint/secretlint-rule-pattern": "13.0.2",
"@secretlint/secretlint-rule-preset-recommend": "13.0.2",
"dependency-cruiser": "catalog:",
"eslint": "catalog:",
"eslint-plugin-ghost": "catalog:",
"eslint-plugin-i18next": "6.1.5",
Expand All @@ -108,6 +110,19 @@
"nx": {
"includedScripts": [],
"targets": {
"lint:boundaries": {
"executor": "nx:run-commands",
"cache": true,
"inputs": [
"{workspaceRoot}/.dependency-cruiser.cjs",
"{workspaceRoot}/ghost/core/core/**/*",
"sharedGlobals"
],
"outputs": [],
"options": {
"command": "depcruise ghost/core/core --config .dependency-cruiser.cjs"
}
},
"docker:up": {
"executor": "nx:run-commands",
"options": {
Expand Down
Loading
Loading