-
-
Notifications
You must be signed in to change notification settings - Fork 766
chore: add Knip and fix unused dependencies, exports, and files #3006
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.
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
nperez0111
wants to merge
4
commits into
main
Choose a base branch
from
feat/add-knip-fix-issues
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| --- | ||
| name: knip-skill | ||
| description: How Knip is configured in this repo and how to act on its findings. Should be used automatically whenever Knip is run, mentioned, configured, or reports unused files/dependencies/exports — including when deciding whether a flagged item is safe to delete. | ||
| --- | ||
|
|
||
| Knip checks for unused files, dependencies, and exports. Config lives in `knip.json` at the repo root. Several parts of this repo look "unused" to static analysis but are load-bearing, so **verify every finding before deleting** — the config encodes a lot of hard-won exceptions. | ||
|
|
||
| # Running Knip | ||
|
|
||
| 1. **Always regenerate first**: `vp run --filter @blocknote/dev-scripts gen`. Example `package.json`s, `playground/src/examples.gen.tsx`, and part of `docs/package.json`'s dependencies are generated from `.bnexample.json` manifests. Running Knip against stale generated files produces phantom findings. | ||
| 2. Run via the root script: `pnpm knip` (this is `knip --no-config-hints`). Don't run bare `knip` from a subdirectory. | ||
| 3. Knip respects `.gitignore` for entry/project file discovery — gitignored files (e.g. `docs/components/example/generated/`) are never picked up as project files, so they don't appear in reports. That is expected, not a coverage bug. | ||
| 4. Knip is **not wired into CI**. CI only checks that `gen` output is committed and current (`build.yml` "Check generated files are up to date"). Keeping `pnpm knip` at zero findings is a manual discipline — run it before finishing any branch that touches dependencies or exports. | ||
|
|
||
| # Triaging findings — check these before deleting anything | ||
|
|
||
| A flagged item is a **false positive** (fix the config, not the code) when it is: | ||
|
|
||
| - **Referenced by a string path**, not an import — e.g. `docs/components/typstCompilerWasmUrl.ts` via turbopack aliases in `docs/next.config.ts`. Add it as an `entry` if it's real source that should stay analyzed; use the root `ignore` list only when suppressing all issue types for it is acceptable (`ignore` silences everything about a file, not just "unused file" — `ignoreFiles` is the narrower option that only suppresses the unused-file report). | ||
| - **A binary invoked from `node_modules/.bin` in a script** — e.g. `wasm-pack` in `packages/xl-typst-compiler/scripts/ensure-wasm.mjs`. Add to that workspace's `ignoreDependencies`. | ||
| - **A subpath entry of a package's exports map** — packages with entries beyond `"."` need them listed in the workspace `entry` config (see `packages/math-block`, `packages/diagram-block` with `src/*-exporter/index.{ts,tsx}`, and `packages/xl-pdf-exporter` with `src/react-pdf/index.ts`). Check the package.json `exports` field before deleting an "unused file" that is an `index.ts`. | ||
| - **A workspace dep that exists for build ordering, not imports** — e.g. `@blocknote/shared` and `@blocknote/xl-typst-compiler` in root `ignoreDependencies`, and all of playground's `@blocknote/*` deps (see below). Removing them breaks topological build order (locally and on Vercel) even though no source file imports them. | ||
| - **Consumed via a namespace import** (`import * as x` then `x.fn(...)` passed around) — Knip can miss this. Keep the export and mark it with a `@public` JSDoc tag, which exempts it (see `getLLMText` in `docs/lib/source/examples.ts`). | ||
|
|
||
| Only after ruling those out is a finding a **true positive**: delete the file, remove the dep, or un-export the symbol. | ||
|
|
||
| **Be very careful with `knip --fix`.** It once un-exported `getLLMText`, which broke the docs production build because only `next build` (not `types:check`) runs the full route typecheck. Prefer manual fixes; if you use `--fix`, review every hunk. | ||
|
|
||
| # Non-standard workspace configs (the "why" behind knip.json) | ||
|
|
||
| - **`docs`**: `ignoreDependencies: [".+"]` — dependency checking is disabled **by design**. `gen` syncs deps from docs-embedded example manifests into `docs/package.json`, and docs consumes things through paths Knip can't follow. Do not "improve" this by narrowing the pattern; do not hand-prune docs deps that gen will re-add. | ||
| - **`playground`**: `@blocknote/.+` deps drive Vercel's `--filter '@blocknote/example-editor...'` build ordering — never remove them. `katex` and `tex2typst` are resolved from the playground's own `node_modules` by build-mode vite aliases in `playground/vite.config.ts` (a Vercel filtered-install workaround) — keep the ignore list in sync with those aliases: if an alias is added/removed there, mirror it in the dep list and `knip.json`. Everything else was deliberately pruned; playground examples resolve their own deps via per-example `node_modules` (relative imports). | ||
| - **`examples/*/*`**: `.bnexample.json` lists only what the example's source imports; the generator auto-injects `@blocknote/core`, `@blocknote/react`, the `uiLib` package (+ `@mantine/core`/`@mantine/hooks` for mantine), `react`, `react-dom` — hence the shared `ignoreDependencies`. A few examples have per-example configs extending it. **Never edit a generated example `package.json` to satisfy Knip** — fix `.bnexample.json` and rerun `gen`. | ||
| - Example `"@blocknote/*": "latest"` version specs are deliberate (the StackBlitz open-a-directory repro contract, guarded by `fresh-install-tests.yml`); pnpm's workspace settings resolve them to workspace packages locally. Knip flagging or you pinning these is wrong. | ||
| - **`tests`**: has `paths` for `@examples/*`/`@shared/*`; the ambient `declare module` for `@examples` is deliberate (avoids TS6059) — don't convert it to tsconfig paths. `@blocknote/ariakit`/`@blocknote/shadcn` are used by e2e fixtures in ways Knip misses. | ||
| - **`packages/dev-scripts`**: templates (`examples/template-react/*.template.tsx`) are entries; `vite-env.d.ts.template.tsx` is ignored (it's emitted verbatim, never imported). | ||
|
|
||
| # When adding new packages or deps | ||
|
|
||
| - New package with subpath exports → add an `entry` config for it in `knip.json`. | ||
| - New workspace dep added only for build ordering → add it to root `ignoreDependencies` with the others, don't leave Knip red. | ||
| - New string-referenced source file → prefer an `entry` for its workspace; fall back to the root `ignore` list (accepting the blind spot: `ignore` suppresses all issue types for that file). Generated files should be gitignored, which keeps them out of analysis entirely. | ||
| - After any of the above, `pnpm knip` must exit 0 before the branch is done. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| # Committed output of `pnpm run gen` — collapsed in GitHub diffs and excluded | ||
| # from language stats. CI (build.yml) verifies it stays in sync with examples/. | ||
| playground/src/examples.gen.tsx linguist-generated=true |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.