Skip to content

Moved the comments domain from the posts package into apps/admin - #29209

Merged
9larsons merged 1 commit into
mainfrom
slars/pla-221-move-comments
Jul 9, 2026
Merged

Moved the comments domain from the posts package into apps/admin#29209
9larsons merged 1 commit into
mainfrom
slars/pla-221-move-comments

Conversation

@9larsons

@9larsons 9larsons commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

ref https://linear.app/ghost/issue/PLA-221/posts-admin-move-tags-comments-automations

Third domain to leave the posts package as it dissolves into apps/admin (PLA-221), after tags and automations.

Comments was the cleanest large move because the shared filter engine was already extracted to admin-x-framework in an earlier PR — comments already imports @tryghost/admin-x-framework/filters / /filter-sources / utils/get-site-timezone. So its only remaining posts-internal ties were MainLayout and the comments-only use-comments-pinning-enabled hook.

What changed

  • apps/posts/src/views/comments/apps/admin/src/comments/ (17 files); use-comments-pinning-enabled (its only consumers are comments) moves into src/comments/hooks/.
  • The comments route folds into the shell's own route table; removed from posts' route table.
  • MainLayout → shade Box + Container, matching the tags/automations moves; rendered layout unchanged.
  • Moved tests colocate beside their sources.

Admin strictness

The stricter admin config surfaced the usual set, all fixed at source (no disables):

  • verbatimModuleSyntax: type-only imports for Comment, Filter, FilterFieldConfig, ValueSource
  • no-misused-promises: react-query fetchNextPage voided at the two call sites
  • react-refresh/only-export-components: the non-component buildThreadLink helper extracted to thread-link.ts
  • a test's expect.any(Function) cast; carried over the @tryghost/nql-lang module type declaration and added the dep (used by the comment-fields filter test)

Verification

  • tsc -b, eslint . (admin) — exit 0
  • vitest run (admin) — 479 tests pass (incl. the 5 moved comments suites, 30 tests)
  • @tryghost/posts build / lint / test — exit 0 (comments removed cleanly; 279 tests, the 30 comments tests now live in admin)
  • nx build @tryghost/admin — exit 0; comments code-splits into its own lazy chunk

Comments has solid e2e coverage (comment-moderation, disable-commenting, thread-sidebar), so the domain move is well netted.

ref https://linear.app/ghost/issue/PLA-221/posts-admin-move-tags-comments-automations

- third domain to leave the posts package as it dissolves into apps/admin; comments already read the filter engine from admin-x-framework (extracted earlier), so its only posts-internal ties were MainLayout and the comments-only pinning hook, both handled here
- the comments route folds into the shell's own route table; MainLayout is replaced with shade Box + Container, unchanged rendering
- use-comments-pinning-enabled moves into the comments domain (its only consumers)
- brings the migrated code up to Admin's stricter lint/tsconfig: verbatimModuleSyntax type-only imports, voided react-query fetchNextPage, a non-component thread-link helper extracted for react-refresh, and the nql-lang test type declaration carried over
- the moved tests colocate beside their sources; adds @tryghost/nql-lang to admin for the comment-fields filter test
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR migrates the comments feature from apps/posts into apps/admin. It adds a new /comments route in apps/admin's router and removes the corresponding route from apps/posts. Import paths across comment components, hooks, and tests are updated from @src/views/comments or relative paths to @/comments aliases. Several imports (Comment, Filter, FilterFieldConfig, ValueSource) are converted to type-only imports. A buildThreadLink helper is extracted into a new thread-link.ts module, replacing an inline implementation. The comments page layout is changed from MainLayout to Box/Container, and fetchNextPage props are wrapped in callback functions. A new @tryghost/nql-lang dependency and its type declaration file are added.

Possibly related PRs

  • TryGhost/Ghost#29188: Adds similar new lazy-loaded routes under ForceUpgradeGuard, converts a page layout from MainLayout to Box/Container, and wires fetchNextPage with the same wrapper callback pattern.

Suggested labels: migration

Suggested reviewers: EvanHahn, acburdine

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: moving the comments domain from posts into apps/admin.
Description check ✅ Passed The description is directly related and matches the migration, routing, lint, and test updates in the changeset.
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 slars/pla-221-move-comments

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

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

@nx-cloud

nx-cloud Bot commented Jul 9, 2026

Copy link
Copy Markdown

🤖 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 c5ea859

Command Status Duration Result
nx run @tryghost/admin:build ✅ Succeeded 5s View ↗
nx run ghost:build:assets ✅ Succeeded 2s View ↗
nx run ghost:build:tsc ✅ Succeeded 7s View ↗

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


☁️ Nx Cloud last updated this comment at 2026-07-09 17:48:40 UTC

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@apps/admin/package.json`:
- Line 22: Move `@tryghost/nql-lang` out of the production dependencies and into
devDependencies in the admin package manifest, since it is only used by the
comment-fields test and should not be included in the shipped runtime bundle.
Update the package.json entry for `@tryghost/nql-lang` accordingly and keep the
rest of the dependency list unchanged.
🪄 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: a815ef3f-ac9a-4abb-8897-03cfb0fe52b0

📥 Commits

Reviewing files that changed from the base of the PR and between 83a5b36 and c5ea859.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (28)
  • apps/admin/package.json
  • apps/admin/src/comments/comment-fields.test.ts
  • apps/admin/src/comments/comment-fields.ts
  • apps/admin/src/comments/comment-filter-query.test.ts
  • apps/admin/src/comments/comment-filter-query.ts
  • apps/admin/src/comments/comments.tsx
  • apps/admin/src/comments/components/comment-content.tsx
  • apps/admin/src/comments/components/comment-header.test.tsx
  • apps/admin/src/comments/components/comment-header.tsx
  • apps/admin/src/comments/components/comment-likes-modal.tsx
  • apps/admin/src/comments/components/comment-menu.tsx
  • apps/admin/src/comments/components/comment-metrics.tsx
  • apps/admin/src/comments/components/comment-reports-modal.tsx
  • apps/admin/src/comments/components/comment-thread-list.tsx
  • apps/admin/src/comments/components/comment-thread-sidebar.tsx
  • apps/admin/src/comments/components/comments-filters.tsx
  • apps/admin/src/comments/components/comments-list.tsx
  • apps/admin/src/comments/components/disable-commenting-dialog.tsx
  • apps/admin/src/comments/components/thread-link.ts
  • apps/admin/src/comments/hooks/use-comments-pinning-enabled.ts
  • apps/admin/src/comments/hooks/use-filter-state.test.tsx
  • apps/admin/src/comments/hooks/use-filter-state.ts
  • apps/admin/src/comments/legacy-comment-filter-query.ts
  • apps/admin/src/comments/use-comment-filter-fields.test.tsx
  • apps/admin/src/comments/use-comment-filter-fields.ts
  • apps/admin/src/nql-lang.d.ts
  • apps/admin/src/routes.tsx
  • apps/posts/src/routes.tsx
💤 Files with no reviewable changes (1)
  • apps/posts/src/routes.tsx

Comment thread apps/admin/package.json
"@tryghost/admin-x-settings": "workspace:*",
"@tryghost/kg-unsplash-selector": "workspace:*",
"@tryghost/koenig-lexical": "workspace:*",
"@tryghost/nql-lang": "catalog:",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check whether `@tryghost/nql-lang` is imported in non-test source files within apps/admin.

# Search for imports of `@tryghost/nql-lang` in non-test files
rg -nP --type=ts --type=tsx -g '!**/*.test.*' -g '!**/*.spec.*' "`@tryghost/nql-lang`" apps/admin/src/ 2>/dev/null || echo "No non-test imports found"

# Also check for any require() calls
rg -nP "require\(['\"]`@tryghost/nql-lang`" apps/admin/src/ 2>/dev/null || echo "No require() calls found"

Repository: TryGhost/Ghost

Length of output: 203


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- apps/admin/package.json ---'
nl -ba apps/admin/package.json | sed -n '1,120p'

echo
echo '--- repo-wide references to `@tryghost/nql-lang` ---'
rg -n "`@tryghost/nql-lang`" . --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' || true

echo
echo '--- test-file references under apps/admin ---'
rg -n "`@tryghost/nql-lang`" apps/admin --glob '**/*.test.*' --glob '**/*.spec.*' --glob '!**/node_modules/**' || true

Repository: TryGhost/Ghost

Length of output: 224


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- apps/admin/package.json (top-level deps) ---'
sed -n '1,140p' apps/admin/package.json

echo
echo '--- repo-wide references to `@tryghost/nql-lang` ---'
rg -n "`@tryghost/nql-lang`" . --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' || true

echo
echo '--- test-file references under apps/admin ---'
rg -n "`@tryghost/nql-lang`" apps/admin --glob '**/*.test.*' --glob '**/*.spec.*' --glob '!**/node_modules/**' || true

Repository: TryGhost/Ghost

Length of output: 6573


Move @tryghost/nql-lang to devDependencies. It’s only imported by apps/admin/src/comments/comment-fields.test.ts, so it doesn’t need to ship as a production dependency.

🤖 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/admin/package.json` at line 22, Move `@tryghost/nql-lang` out of the
production dependencies and into devDependencies in the admin package manifest,
since it is only used by the comment-fields test and should not be included in
the shipped runtime bundle. Update the package.json entry for `@tryghost/nql-lang`
accordingly and keep the rest of the dependency list unchanged.

Source: Coding guidelines

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.07%. Comparing base (83a5b36) to head (c5ea859).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #29209      +/-   ##
==========================================
+ Coverage   74.04%   74.07%   +0.02%     
==========================================
  Files        1570     1570              
  Lines      136624   136624              
  Branches    16524    16528       +4     
==========================================
+ Hits       101165   101199      +34     
+ Misses      34423    34389      -34     
  Partials     1036     1036              
Flag Coverage Δ
admin-tests 55.19% <ø> (ø)
e2e-tests 76.18% <ø> (+0.02%) ⬆️

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.

@9larsons
9larsons enabled auto-merge (squash) July 9, 2026 17:50
@9larsons
9larsons merged commit c235d6c into main Jul 9, 2026
84 of 86 checks passed
@9larsons
9larsons deleted the slars/pla-221-move-comments branch July 9, 2026 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant