chore(ci): add knip configuration for unused dep linting#75
Merged
Conversation
📝 WalkthroughWalkthroughAdds Knip unused-export detection (config, script, wireit task) and removes many previously exported types/functions across multiple packages; adjusts workspace manifests and some import paths and package.json entries. No runtime behavior changes described. ChangesKnip Tooling Integration
API Surface Reduction
Workspace & Manifest Cleanup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
coryrylan
commented
May 5, 2026
| import { type ToolAnnotations } from '@modelcontextprotocol/sdk/types.js'; | ||
|
|
||
| export const VERSION = '0.0.0'; | ||
| export const BUILD_SHA = '__NVE_BUILD_CHECKSUM__'; |
Collaborator
Author
There was a problem hiding this comment.
This is used but there was already a declaration in the index.ts of the package
coryrylan
commented
May 5, 2026
|
|
||
| export const banner = `"░██████████ ░██ ░██ \\n░██ ░██ ░██ \\n░██ ░██ ░███████ ░█████████████ ░███████ ░████████ ░████████ ░███████ \\n░█████████ ░██ ░██ ░██ ░██ ░██ ░██ ░██ ░██ ░██ ░██ ░██ ░██ \\n░██ ░██ ░█████████ ░██ ░██ ░██ ░█████████ ░██ ░██ ░██ ░███████ \\n░██ ░██ ░██ ░██ ░██ ░██ ░██ ░██ ░██ ░██ ░██ \\n░██████████ ░██ ░███████ ░██ ░██ ░██ ░███████ ░██ ░██ ░████ ░███████"`; | ||
|
|
||
| export type ArgInputType = 'string' | 'number' | 'boolean' | 'object' | 'array'; |
Collaborator
Author
There was a problem hiding this comment.
knip will catch interfaces, vars, functions that are not used outside a module and flag them. This is helpful to see what functions are internal to the module or may need dedicated tests
coryrylan
commented
May 5, 2026
| return resizeObserver; | ||
| } | ||
|
|
||
| export type ControlLayouts = 'vertical' | 'vertical-inline' | 'horizontal' | 'horizontal-inline'; |
Collaborator
Author
There was a problem hiding this comment.
knip will follow the code paths to determine if the type/interface is exported up to a public entrypoint. This is why the knip config requires explicit entrypoint paths to do this safely.
coryrylan
commented
May 5, 2026
| /** [MDN Reference](https://developer.mozilla.org/docs/Web/API/ValidityState/valueMissing) */ | ||
| readonly valueMissing: boolean; | ||
| } | ||
|
|
Collaborator
Author
There was a problem hiding this comment.
No longer needed, provided by the DOM types built in to TS
coryrylan
commented
May 5, 2026
| "@nvidia-elements/lint": "workspace:*", | ||
| "@nvidia-elements/monaco": "workspace:*", | ||
| "@nvidia-elements/styles": "workspace:*", | ||
| "@nvidia-elements/themes": "workspace:*", |
Collaborator
Author
There was a problem hiding this comment.
Had references to other workspace projects but were missing from the deps
![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)
projects/internals/tools/src/project/starters.ts (1)
206-213:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate HTTP response before persisting starter archive.
Line 208 currently accepts any HTTP status. If the server returns 404/500, non-zip content gets written and fails later during extraction.
💡 Suggested fix
async function downloadStarter(starterPath: string, outPath: string) { console.log('⏳ Downloading starter...'); const response = await fetch(starterPath); - const blob = await response.blob(); - const arrayBuffer = await blob.arrayBuffer(); + if (!response.ok) { + throw new Error(`Failed to download starter (${response.status} ${response.statusText}) from ${starterPath}`); + } + const arrayBuffer = await response.arrayBuffer(); const buffer = Buffer.from(arrayBuffer); writeFileSync(outPath, buffer); }🤖 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 `@projects/internals/tools/src/project/starters.ts` around lines 206 - 213, downloadStarter currently writes whatever fetch returns without checking HTTP status; update downloadStarter to validate the fetch response before writing the file by checking response.ok (or response.status) and throwing an error when not OK so no invalid body is persisted, and optionally verify response.headers.get('content-type') contains 'zip' or 'octet-stream' to guard against non-zip payloads; keep the rest of the flow (blob -> arrayBuffer -> Buffer -> writeFileSync) unchanged and surface a clear error that includes response.status and the starterPath when throwing.
🤖 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 `@projects/internals/tools/src/project/starters.ts`:
- Around line 206-213: downloadStarter currently writes whatever fetch returns
without checking HTTP status; update downloadStarter to validate the fetch
response before writing the file by checking response.ok (or response.status)
and throwing an error when not OK so no invalid body is persisted, and
optionally verify response.headers.get('content-type') contains 'zip' or
'octet-stream' to guard against non-zip payloads; keep the rest of the flow
(blob -> arrayBuffer -> Buffer -> writeFileSync) unchanged and surface a clear
error that includes response.status and the starterPath when throwing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 5bdd7490-f90f-49d8-802e-e61527962cfc
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (29)
knip.config.jspackage.jsonpnpm-workspace.yamlprojects/cli/src/mcp/mcp.tsprojects/cli/src/utils.tsprojects/core/src/forms/utils/layout.tsprojects/core/src/forms/utils/states.tsprojects/core/src/forms/utils/types.tsprojects/core/src/sparkline/sparkline.utils.tsprojects/internals/metadata/src/tasks/lighthouse.utils.tsprojects/internals/metadata/src/utils/reports.tsprojects/internals/patterns/package.jsonprojects/internals/tools/src/distill/examples.tsprojects/internals/tools/src/examples/utils.tsprojects/internals/tools/src/internal/tools.tsprojects/internals/tools/src/internal/utils.tsprojects/internals/tools/src/project/setup-agent.tsprojects/internals/tools/src/project/starters.tsprojects/internals/tools/src/project/update.tsprojects/lint/src/eslint/internals/attributes.tsprojects/lint/src/eslint/internals/element-attributes.tsprojects/lint/src/eslint/internals/tailwind.tsprojects/lint/src/eslint/rule-types.tsprojects/monaco/src/internal/base/input.tsprojects/monaco/src/internal/formats/problems-format.tsprojects/site/.gitignoreprojects/site/src/_11ty/layouts/docs.tsprojects/starters/hugo/package.jsonprojects/styles/package.json
💤 Files with no reviewable changes (6)
- projects/internals/tools/src/internal/utils.ts
- projects/starters/hugo/package.json
- projects/core/src/forms/utils/types.ts
- projects/lint/src/eslint/rule-types.ts
- projects/site/.gitignore
- pnpm-workspace.yaml
coryrylan
commented
May 5, 2026
| "version": "0.0.0", | ||
| "description": "Hugo + Elements Starter", | ||
| "private": true, | ||
| "main": "index.js", |
Collaborator
Author
There was a problem hiding this comment.
knip will also catch bad package export paths, similar to our publish lint
- Introduced `knip.config.js` for managing package and project file exclusions. - Updated `package.json` to include a new linting script for knip. - Adjusted workspace configurations in `pnpm-workspace.yaml` to reflect new project structures. - Refactored various TypeScript files to improve type definitions and remove unnecessary exports. Signed-off-by: Cory Rylan <crylan@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
projects/internals/tools/src/project/starters.ts (1)
206-213:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd HTTP response validation before writing archive.
downloadStarterwrites the response body unconditionally; non-2xx responses will be saved as invalid zip files and cause misleading downstream errors.💡 Proposed fix
async function downloadStarter(starterPath: string, outPath: string) { console.log('⏳ Downloading starter...'); const response = await fetch(starterPath); - const blob = await response.blob(); - const arrayBuffer = await blob.arrayBuffer(); - const buffer = Buffer.from(arrayBuffer); - writeFileSync(outPath, buffer); + if (!response.ok) { + throw new Error(`Failed to download starter: ${response.status} ${response.statusText}`); + } + const arrayBuffer = await response.arrayBuffer(); + writeFileSync(outPath, Buffer.from(arrayBuffer)); }🤖 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 `@projects/internals/tools/src/project/starters.ts` around lines 206 - 213, In downloadStarter, validate the HTTP response before writing: check response.ok (or response.status) after fetch(starterPath) and if it's not a 2xx response throw or return an error that includes response.status and response.statusText (optionally content-type) instead of writing the body; only proceed to blob/arrayBuffer/Buffer/writeFileSync when the response is successful. Ensure the thrown/returned error provides clear context so callers won't save invalid zip files.projects/internals/tools/src/project/update.ts (1)
76-77:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix malformed update command in
execSync.The command string has an erroneous trailing
}and duplicated package pattern, which will cause dependency updates to fail.💡 Proposed fix
- execSync(`cd ${cwd} && ${packageManager} update '@nvidia-elements/*' '@nvidia-elements/*'}`); + execSync(`${packageManager} update "@nvidia-elements/*"`, { cwd });🤖 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 `@projects/internals/tools/src/project/update.ts` around lines 76 - 77, The execSync command string is malformed: it contains an extra trailing "}" and repeats the package pattern; update the execSync invocation (the line that currently calls execSync with packageManager, cwd and the patterns) to form a valid shell command — remove the duplicated '@nvidia-elements/*' and the stray brace so the call becomes something like execSync(`cd ${cwd} && ${packageManager} update '@nvidia-elements/*'`) (ensure proper backtick/quote pairing); keep writeFileSync(updatedPackageJson...) unchanged.
🤖 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 `@projects/internals/metadata/src/tasks/lighthouse.utils.ts`:
- Line 22: The declaration emit fails because LighthouseScores references
LighthouseElementReport but that interface is not exported; export the interface
so the type is visible in d.ts output—update the declaration for
LighthouseElementReport to be exported (e.g., add "export" to the interface) or
explicitly re-export it alongside LighthouseScores so the compiler can emit
correct declarations for LighthouseElementReport referenced by LighthouseScores.
In `@projects/lint/src/eslint/internals/attributes.ts`:
- Around line 39-40: Update the preceding comment for the function
isComplexAttributeValue to reflect its current non-exported visibility: remove
or change the phrase "also used in `@internals/metadata`" and replace it with a
short, accurate description such as "internal helper" or "used only within this
module" so it doesn't imply external usage; ensure the comment still explains
purpose ("values that often confuse agents...") and references the function name
isComplexAttributeValue for clarity.
---
Outside diff comments:
In `@projects/internals/tools/src/project/starters.ts`:
- Around line 206-213: In downloadStarter, validate the HTTP response before
writing: check response.ok (or response.status) after fetch(starterPath) and if
it's not a 2xx response throw or return an error that includes response.status
and response.statusText (optionally content-type) instead of writing the body;
only proceed to blob/arrayBuffer/Buffer/writeFileSync when the response is
successful. Ensure the thrown/returned error provides clear context so callers
won't save invalid zip files.
In `@projects/internals/tools/src/project/update.ts`:
- Around line 76-77: The execSync command string is malformed: it contains an
extra trailing "}" and repeats the package pattern; update the execSync
invocation (the line that currently calls execSync with packageManager, cwd and
the patterns) to form a valid shell command — remove the duplicated
'@nvidia-elements/*' and the stray brace so the call becomes something like
execSync(`cd ${cwd} && ${packageManager} update '@nvidia-elements/*'`) (ensure
proper backtick/quote pairing); keep writeFileSync(updatedPackageJson...)
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: ASSERTIVE
Plan: Enterprise
Run ID: 209e3da6-ab15-4556-9aaf-7bcadb486610
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (29)
knip.config.jspackage.jsonpnpm-workspace.yamlprojects/cli/src/mcp/mcp.tsprojects/cli/src/utils.tsprojects/core/src/forms/utils/layout.tsprojects/core/src/forms/utils/states.tsprojects/core/src/forms/utils/types.tsprojects/core/src/sparkline/sparkline.utils.tsprojects/internals/metadata/src/tasks/lighthouse.utils.tsprojects/internals/metadata/src/utils/reports.tsprojects/internals/patterns/package.jsonprojects/internals/tools/src/distill/examples.tsprojects/internals/tools/src/examples/utils.tsprojects/internals/tools/src/internal/tools.tsprojects/internals/tools/src/internal/utils.tsprojects/internals/tools/src/project/setup-agent.tsprojects/internals/tools/src/project/starters.tsprojects/internals/tools/src/project/update.tsprojects/lint/src/eslint/internals/attributes.tsprojects/lint/src/eslint/internals/element-attributes.tsprojects/lint/src/eslint/internals/tailwind.tsprojects/lint/src/eslint/rule-types.tsprojects/monaco/src/internal/base/input.tsprojects/monaco/src/internal/formats/problems-format.tsprojects/site/.gitignoreprojects/site/src/_11ty/layouts/docs.tsprojects/starters/hugo/package.jsonprojects/styles/package.json
💤 Files with no reviewable changes (6)
- projects/lint/src/eslint/rule-types.ts
- projects/internals/tools/src/internal/utils.ts
- projects/site/.gitignore
- projects/core/src/forms/utils/types.ts
- pnpm-workspace.yaml
- projects/starters/hugo/package.json
| } | ||
|
|
||
| export interface LighthouseElementReport { | ||
| interface LighthouseElementReport { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check declaration emit settings =="
fd -i 'tsconfig*.json' | xargs -r rg -n '"declaration"|"composite"|"emitDeclarationOnly"'
echo
echo "== Confirm exported types referencing LighthouseElementReport =="
rg -n -C2 'export interface LighthouseScores|LighthouseElementReport' projects/internals/metadata/src/tasks/lighthouse.utils.tsRepository: NVIDIA/elements
Length of output: 2750
Re-export LighthouseElementReport to fix declaration emit failure.
TypeScript declaration emit is enabled in this package (declaration: true, emitDeclarationOnly: true). The exported LighthouseScores interface references LighthouseElementReport on lines 13–19, but LighthouseElementReport at line 22 is no longer exported. This will fail with TS4023 during declaration emit.
Proposed fix
-interface LighthouseElementReport {
+export interface LighthouseElementReport {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| interface LighthouseElementReport { | |
| export interface LighthouseElementReport { |
🤖 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 `@projects/internals/metadata/src/tasks/lighthouse.utils.ts` at line 22, The
declaration emit fails because LighthouseScores references
LighthouseElementReport but that interface is not exported; export the interface
so the type is visible in d.ts output—update the declaration for
LighthouseElementReport to be exported (e.g., add "export" to the interface) or
explicitly re-export it alongside LighthouseScores so the compiler can emit
correct declarations for LighthouseElementReport referenced by LighthouseScores.
johnyanarella
approved these changes
May 5, 2026
Collaborator
Author
|
🎉 This issue has been resolved in version 0.0.10 🎉 |
Collaborator
Author
|
🎉 This issue has been resolved in version 0.0.8 🎉 |
Collaborator
Author
|
🎉 This issue has been resolved in version 0.0.9 🎉 |
Collaborator
Author
|
🎉 This issue has been resolved in version 0.1.1 🎉 |
Collaborator
Author
|
🎉 This issue has been resolved in version 0.3.1 🎉 |
Collaborator
Author
|
🎉 This issue has been resolved in version 0.0.9 🎉 |
1 similar comment
Collaborator
Author
|
🎉 This issue has been resolved in version 0.0.9 🎉 |
Collaborator
Author
|
🎉 This issue has been resolved in version 0.0.11 🎉 |
Collaborator
Author
|
🎉 This issue has been resolved in version 0.0.8 🎉 |
This was referenced May 15, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
knip.config.jsfor managing package and project file exclusions.package.jsonto include a new linting script for knip.pnpm-workspace.yamlto reflect new project structures.Summary by CodeRabbit
Chores
Refactor
Docs