Skip to content

Fixed knip and removed unused dependencies it surfaced - #29496

Merged
9larsons merged 1 commit into
mainfrom
slars/knip-implementation-7c4d96
Jul 21, 2026
Merged

Fixed knip and removed unused dependencies it surfaced#29496
9larsons merged 1 commit into
mainfrom
slars/knip-implementation-7c4d96

Conversation

@9larsons

Copy link
Copy Markdown
Contributor

Why

knip (added in #27716) was crashing before it could produce a report at all — pnpm knip died with an uncaught ENOENT from inside packages/i18n/generate-context.js, a codegen script it had no business executing. With the tool actually broken, its first batch of real advisories (unused dependencies) had never been triaged.

What

Fixed the crash. The root vitest.config.mjs's test.projects glob (packages/**, apps/*, used by the real vitest CLI for its unified watcher) was being naively expanded by knip's vitest plugin, which dynamically import()ed every matched file as if each were itself a vitest config — including README.md files and packages/i18n/generate-context.js (whose un-awaited async IIFE threw an unhandled rejection that killed the whole process). Disabled knip's vitest config auto-discovery at the root workspace; every package already has its own correctly-scoped vitest.config.ts that knip reads directly.

Removed 6 genuinely dead dependencies knip flagged, each verified via full-repo grep to have zero references anywhere:

  • @tryghost/string, mingo (admin-x-settings) — also removed a stale @tryghost/string type declaration
  • @tryghost/kg-clean-basic-html, @tryghost/kg-markdown-html-renderer (ghost/core)
  • lodash (kg-converters)
  • @lexical/code (kg-lexical-html-renderer)

Pinned 6 more in ignoreDependencies since they're used through means knip's static analysis can't trace, and would otherwise get flagged as false positives on every future run:

  • bookshelf-relations — resolved by string name inside bookshelf's .plugin() API, not a static import
  • @tryghost/kg-unsplash-selector (admin) — only referenced via a Tailwind @source CSS directive
  • @tryghost/debug, @vitejs/plugin-react, vite-plugin-svgr (5 public apps) — dynamically imported inside the shared apps/_shared/vite-public-app.mjs, whose vite.config knip previously couldn't even load

Verification

  • pnpm knip now runs to completion and produces a full report (previously crashed)
  • Confirmed each removed dependency has no references via full-repo grep before deleting
  • pnpm build — all 32 projects build successfully
  • Full test suites pass for every touched package: ghost/core (7486/7487 — the one failure is a pre-existing unrelated flaky timeout), admin-x-settings (205/205 + lint), kg-converters, kg-lexical-html-renderer (67/67)

Checklist

  • I've explained my change
  • Existing test suites verify the change (no new dependency-removal-specific tests needed — coverage is in the packages' existing suites)

no ref

- knip.json: disabled vitest config auto-discovery at the root workspace.
  Knip's vitest plugin was naively dynamically importing every file
  matched by the root vitest.config.mjs's `test.projects` glob
  (packages/**, apps/*, used by the real vitest CLI for its unified
  watcher) as if each were itself a vitest config — including README.md
  files and packages/i18n/generate-context.js, whose un-awaited async IIFE
  threw an unhandled rejection that crashed the whole `knip` process
  before any report was produced. Each package already has its own
  correctly-scoped vitest.config.ts that knip reads directly, so the root
  config's broad glob was never needed for knip's analysis.
- Removed 6 dependencies knip flagged as unused after verifying via
  full-repo grep that they have zero references anywhere: @tryghost/string
  and mingo (admin-x-settings, plus a stale @tryghost/string type
  declaration), @tryghost/kg-clean-basic-html and
  @tryghost/kg-markdown-html-renderer (ghost/core), lodash
  (kg-converters), @lexical/code (kg-lexical-html-renderer).
- Pinned 6 more in ignoreDependencies since they're used through means
  knip's static analysis can't trace: bookshelf-relations is resolved by
  string name inside bookshelf's plugin API, kg-unsplash-selector is only
  referenced via a Tailwind @source CSS directive, and
  @tryghost/debug/@vitejs/plugin-react/vite-plugin-svgr are dynamically
  imported inside the shared apps/_shared/vite-public-app.mjs, whose
  vite.config knip previously couldn't even load.
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 74acd136-11fa-4b87-b5dd-b0b639c366de

📥 Commits

Reviewing files that changed from the base of the PR and between 524a7b5 and bf3a92e.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (6)
  • apps/admin-x-settings/package.json
  • apps/admin-x-settings/src/typings.d.ts
  • ghost/core/package.json
  • knip.json
  • koenig/kg-converters/package.json
  • koenig/kg-lexical-html-renderer/package.json
💤 Files with no reviewable changes (4)
  • koenig/kg-lexical-html-renderer/package.json
  • apps/admin-x-settings/package.json
  • ghost/core/package.json
  • apps/admin-x-settings/src/typings.d.ts

Walkthrough

Updated dependency manifests for admin settings, Ghost core, and Koenig packages. Removed the @tryghost/string declaration, removed unused package dependencies, replaced a Lexical dependency, and expanded Knip workspace ignoreDependencies configuration for the root, core, and several applications.

Suggested reviewers: kevinansfield

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: fixing Knip and removing unused dependencies it identified.
Description check ✅ Passed The description matches the changeset by explaining the Knip crash fix, dependency removals, and ignored dependencies.
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/knip-implementation-7c4d96

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 21, 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 bf3a92e

Command Status Duration Result
nx run ghost:test:ci:integration ✅ Succeeded 3m 5s View ↗
nx run-many -t test:unit -p @tryghost/admin-x-s... ✅ Succeeded 28s View ↗
nx run ghost:test:integration ✅ Succeeded 3m 10s View ↗
nx run @tryghost/admin:test:acceptance ✅ Succeeded 6m 1s View ↗
nx run ghost:test:legacy ✅ Succeeded 3m 11s View ↗
nx run ghost-monorepo:lint:boundaries ✅ Succeeded 20s View ↗
nx run @tryghost/koenig-lexical:test:acceptance ✅ Succeeded 2m 24s View ↗
nx run-many -t lint -p @tryghost/admin-x-settin... ✅ Succeeded 3m 7s 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-21 17:28:57 UTC

@9larsons
9larsons enabled auto-merge (squash) July 21, 2026 17:24
@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.59%. Comparing base (6d1f6e7) to head (bf3a92e).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #29496      +/-   ##
==========================================
- Coverage   74.60%   74.59%   -0.01%     
==========================================
  Files        1604     1604              
  Lines      140899   140903       +4     
  Branches    17212    17204       -8     
==========================================
- Hits       105111   105109       -2     
- Misses      34729    34760      +31     
+ Partials     1059     1034      -25     
Flag Coverage Δ
admin-tests 55.44% <ø> (ø)
e2e-tests 76.68% <ø> (-0.01%) ⬇️

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 merged commit 04fff3e into main Jul 21, 2026
55 checks passed
@9larsons
9larsons deleted the slars/knip-implementation-7c4d96 branch July 21, 2026 17:30
9larsons added a commit that referenced this pull request Jul 21, 2026
…nip (#29500)

no ref

Follow-up to #29496 (fixing knip's crash and its first batch of
unused-dependency findings). This PR works through the remaining
categories: unlisted dependencies, unlisted binaries, and unused
devDependencies.
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