Removed unused dependencies surfaced by knip#27720
Conversation
WalkthroughMultiple package.json files were modified across the repository. Several packages had version bumps (activitypub, announcement-bar, comments-ui, portal). Numerous dependencies and devDependencies were removed or added in various packages (examples: lodash, moment, sinon, ts-node removed; new testing, linting, and tooling deps added). apps/shade received a large dependency and devDependency overhaul including Storybook and modern React tooling. ghost/core had several runtime and devDependency changes (added 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/comments-ui/package.json (1)
25-25:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
PLAYWRIGHT_REPORTER=listto thetest:e2escript.The
test:e2ecommand is missing thePLAYWRIGHT_REPORTER=listenvironment variable required for Playwright e2e tests.🔧 Proposed fix
- "test:e2e": "NODE_OPTIONS='--experimental-specifier-resolution=node --no-warnings' VITE_TEST=true playwright test", + "test:e2e": "NODE_OPTIONS='--experimental-specifier-resolution=node --no-warnings' VITE_TEST=true PLAYWRIGHT_REPORTER=list playwright test",As per coding guidelines: "Set PLAYWRIGHT_REPORTER=list environment variable when running Playwright e2e tests as an AI agent for better parsing."
🤖 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/comments-ui/package.json` at line 25, The test:e2e npm script in package.json is missing the PLAYWRIGHT_REPORTER=list environment variable; update the "test:e2e" script to prepend or include PLAYWRIGHT_REPORTER=list alongside NODE_OPTIONS and VITE_TEST so Playwright runs with the list reporter (i.e., modify the "test:e2e" script entry to set PLAYWRIGHT_REPORTER=list before running `playwright test`).
🤖 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.
Outside diff comments:
In `@apps/comments-ui/package.json`:
- Line 25: The test:e2e npm script in package.json is missing the
PLAYWRIGHT_REPORTER=list environment variable; update the "test:e2e" script to
prepend or include PLAYWRIGHT_REPORTER=list alongside NODE_OPTIONS and VITE_TEST
so Playwright runs with the list reporter (i.e., modify the "test:e2e" script
entry to set PLAYWRIGHT_REPORTER=list before running `playwright test`).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7131ba23-5b49-4062-81e7-f990a8bc7dd4
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
apps/activitypub/package.jsonapps/admin-x-design-system/package.jsonapps/admin-x-framework/package.jsonapps/admin/package.jsonapps/announcement-bar/package.jsonapps/comments-ui/package.jsonapps/portal/package.jsonapps/posts/package.jsonapps/shade/package.jsone2e/package.jsonghost/admin/package.jsonghost/core/package.jsonghost/parse-email-address/package.jsonpackage.json
💤 Files with no reviewable changes (10)
- apps/posts/package.json
- ghost/parse-email-address/package.json
- apps/admin/package.json
- apps/admin-x-design-system/package.json
- e2e/package.json
- ghost/admin/package.json
- apps/admin-x-framework/package.json
- package.json
- apps/shade/package.json
- ghost/core/package.json
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #27720 +/- ##
=======================================
Coverage 73.18% 73.18%
=======================================
Files 1561 1561
Lines 127214 127214
Branches 15393 15395 +2
=======================================
+ Hits 93100 93101 +1
- Misses 33134 33154 +20
+ Partials 980 959 -21
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/comments-ui/package.json (1)
25-27:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
PLAYWRIGHT_REPORTER=listto the base e2e script.
test:e2eis the root Playwright command, so setting the reporter there will also covertest:slowmoandtest:e2e:full.Suggested patch
- "test:e2e": "NODE_OPTIONS='--experimental-specifier-resolution=node --no-warnings' VITE_TEST=true playwright test", + "test:e2e": "NODE_OPTIONS='--experimental-specifier-resolution=node --no-warnings' VITE_TEST=true PLAYWRIGHT_REPORTER=list playwright test",As per coding guidelines, "Set PLAYWRIGHT_REPORTER=list environment variable when running Playwright e2e tests as an AI agent for better parsing".
🤖 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/comments-ui/package.json` around lines 25 - 27, Update the "test:e2e" npm script to include the PLAYWRIGHT_REPORTER=list environment variable so all downstream scripts (test:slowmo and test:e2e:full) inherit the reporter setting; modify the script named "test:e2e" in package.json to prepend or include PLAYWRIGHT_REPORTER=list alongside NODE_OPTIONS and VITE_TEST so Playwright runs with the list reporter by default.
🤖 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.
Outside diff comments:
In `@apps/comments-ui/package.json`:
- Around line 25-27: Update the "test:e2e" npm script to include the
PLAYWRIGHT_REPORTER=list environment variable so all downstream scripts
(test:slowmo and test:e2e:full) inherit the reporter setting; modify the script
named "test:e2e" in package.json to prepend or include PLAYWRIGHT_REPORTER=list
alongside NODE_OPTIONS and VITE_TEST so Playwright runs with the list reporter
by default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 468bf92e-9a05-4e42-afec-488e3095ec0c
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
apps/activitypub/package.jsonapps/admin-x-design-system/package.jsonapps/admin-x-framework/package.jsonapps/admin/package.jsonapps/announcement-bar/package.jsonapps/comments-ui/package.jsonapps/portal/package.jsonapps/posts/package.jsonapps/shade/package.jsone2e/package.jsonghost/admin/package.jsonghost/core/package.jsonghost/parse-email-address/package.jsonpackage.json
💤 Files with no reviewable changes (10)
- apps/posts/package.json
- apps/admin-x-design-system/package.json
- e2e/package.json
- package.json
- ghost/admin/package.json
- apps/admin/package.json
- apps/shade/package.json
- ghost/parse-email-address/package.json
- ghost/core/package.json
- apps/admin-x-framework/package.json
no ref Two related dep-contract hygiene fixes the post-knip-baseline audit surfaced. Both came out of running `pnpm knip --debug` against `main` after #27720 landed and verifying each entry by hand.
1 participant
Summary
First cleanup pass after wiring up knip in #27716. Walked through each unused-dep candidate in the baseline report, grep-verified each one, and removed the genuinely vestigial entries.
Net effect: 15 package.json files touched, ~390 lines off the lockfile.
What was removed
ghost/core dependencies —
@slack/webhook,@tryghost/http-cache-utils,@tryghost/kg-default-transforms,cluster-key-slot,superagent,superagent-throttle(no source references;@tryghost/requestreplaced superagent;cluster-key-slotstill present transitively under ioredis where it's actually used).ghost/core devDependencies —
detect-indent,detect-newline,find-root,parse-prometheus-text-format,toml.ghost/admin dependencies —
i18n-iso-countries,lru-cache(only mentioned inpackage.json).Root devDependencies —
@actions/core,chalk,chokidar,inquirer(no references in.github/scripts/orscripts/).apps/admin —
lodash.apps/announcement-bar —
@tryghost/content-api.apps/comments-ui —
@testing-library/user-event.apps/portal —
@babel/eslint-parser(eslint config uses@typescript-eslint/parser+ default espree).apps/posts —
moment(onlymoment-timezoneis imported).apps/shade —
@dnd-kit/core,@dnd-kit/sortable,@ebay/nice-modal-react,@radix-ui/react-form,@radix-ui/react-radio-group,@sentry/react,@types/color(color@5 ships its own types),@uiw/react-codemirror,next-themes,react-colorful,react-hot-toast,react-select,@codemirror/lang-html,@radix-ui/react-tooltip(devDep duplicate of the deps entry),@testing-library/react-hooks,@types/lodash-es,chai,lodash-es,sinon.apps/activitypub —
@types/dompurify(dompurify ships its own types),ts-jest(project uses vitest).apps/admin-x-design-system —
@radix-ui/react-tooltip(devDep duplicate of the deps entry).apps/admin-x-framework —
sinon.e2e —
ts-node(project uses tsx).ghost/parse-email-address —
sinon,ts-node.What was kept (verified false positives)
Held off on
tailwindcss/@tailwindcss/vite(PostCSS pipeline), vite plugins (@vitejs/plugin-react,vite-plugin-svgr,vite-plugin-css-injected-by-js), test runners loaded via config (@vitest/coverage-v8,c8,msw,tsx,mocha-slow-test-reporter,cssnano),mysql2(knex client peer), eslint plugins,prop-types, andpapaparse/bookshelf-relations/form-datawhich all have real source references.A first-pass also tried to remove
@tryghost/debugfrom portal/comments-ui/signup-form/sodo-search — those vite configs dodedupe: ['@tryghost/debug']and@tryghost/i18nimports it transitively — but the UMD build externalizes it as a runtimerequire()when it's not a direct dep, which would crash in the browser. Reverted that removal; left a follow-up note in the cleanup plan.Workspace version bumps
Per the workspace versioning rule, bumped non-private workspaces whose
package.jsonwas modified:1.1.18→1.1.191.4.10→1.4.112.68.29→2.68.303.1.14→3.1.16(the pre-commit hook auto-patched once on top of the manual bump)ghost/core's version field is release-managed so it isn't bumped here.
Test plan
pnpm install— clean; lockfile sheds the removed deps' transitive subtreescd ghost/core && pnpm test:unit— 6295 passing, 3 pendingrequire()externalization in the UMD bundles:@tryghost/portal,@tryghost/comments-ui,@tryghost/signup-form,@tryghost/sodo-search,@tryghost/announcement-bar@tryghost/shade,@tryghost/admin-x-framework,@tryghost/admin-x-design-system,@tryghost/admin,@tryghost/posts,@tryghost/activitypub,@tryghost/statspnpm --filter @tryghost/portal lint— clean (verifies @babel/eslint-parser removal)pnpm --filter ghost-admin lint— clean (verifies i18n-iso-countries / lru-cache removal)pnpm --filter @tryghost/admin-x-framework test— passingpnpm --filter @tryghost/parse-email-address test— passing