fix: add explicit type annotations to fix Fresh Install Tests CI#2815
Conversation
…ual typing When @ariakit/react updates its internal module structure (e.g. from @ariakit/react-core to @ariakit/react-components), contextual typing from external library component props can break, causing TS7006 'Parameter implicitly has an any type' errors in the Fresh Install Tests CI workflow. Add explicit type annotations to 5 callback parameters that depend on external library types for inference: - packages/ariakit/src/panel/Panel.tsx: setActiveId callback - packages/ariakit/src/toolbar/ToolbarButton.tsx: onMouseDown callback - packages/ariakit/src/panel/PanelFileInput.tsx: onChange callback - packages/ariakit/src/badge/Badge.tsx: onClick callback - packages/xl-ai/src/components/AIMenu/AIMenuController.tsx: outsidePress callback All annotations use React/DOM built-in types (MouseEvent, ChangeEvent) rather than library-specific types, making them resilient to future dependency updates.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughFive components receive explicit TypeScript type annotations for event handlers and callbacks: Badge's ChangesEvent Handler Type Safety Improvements
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-email-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
commit: |
There was a problem hiding this comment.
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 `@packages/ariakit/src/panel/PanelFileInput.tsx`:
- Line 27: The onChange handler in PanelFileInput blindly accesses
e.target.files![0], which can be null and throw; update the handler in the
PanelFileInput component to guard the nullable files before indexing (e.g.,
check e.target.files and files.length or use optional chaining like
e.target.files?.[0]) and only call onChange?.(...) when a File exists, otherwise
do nothing or pass undefined as intended.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fabd8535-c8a3-4e4e-abe9-d51bce6dc089
📒 Files selected for processing (5)
packages/ariakit/src/badge/Badge.tsxpackages/ariakit/src/panel/Panel.tsxpackages/ariakit/src/panel/PanelFileInput.tsxpackages/ariakit/src/toolbar/ToolbarButton.tsxpackages/xl-ai/src/components/AIMenu/AIMenuController.tsx
| accept={accept} | ||
| value={value ? value.name : undefined} | ||
| onChange={async (e) => onChange?.(e.target.files![0])} | ||
| onChange={async (e: ChangeEvent<HTMLInputElement>) => onChange?.(e.target.files![0])} |
There was a problem hiding this comment.
Guard nullable files before indexing.
e.target.files! can be null on some input states, which can throw at runtime when [0] is accessed.
Proposed fix
- onChange={async (e: ChangeEvent<HTMLInputElement>) => onChange?.(e.target.files![0])}
+ onChange={async (e: ChangeEvent<HTMLInputElement>) => {
+ const file = e.target.files?.[0];
+ if (file) {
+ onChange?.(file);
+ }
+ }}📝 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.
| onChange={async (e: ChangeEvent<HTMLInputElement>) => onChange?.(e.target.files![0])} | |
| onChange={async (e: ChangeEvent<HTMLInputElement>) => { | |
| const file = e.target.files?.[0]; | |
| if (file) { | |
| onChange?.(file); | |
| } | |
| }} |
🤖 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 `@packages/ariakit/src/panel/PanelFileInput.tsx` at line 27, The onChange
handler in PanelFileInput blindly accesses e.target.files![0], which can be null
and throw; update the handler in the PanelFileInput component to guard the
nullable files before indexing (e.g., check e.target.files and files.length or
use optional chaining like e.target.files?.[0]) and only call onChange?.(...)
when a File exists, otherwise do nothing or pass undefined as intended.
Problem
The Fresh Install Tests CI workflow (failing run) updates production dependencies to their latest semver-compatible versions before building. When
@ariakit/reactupdated its internal module structure (from@ariakit/react-coreto@ariakit/react-components), contextual typing from external library component props broke, causingTS7006: Parameter implicitly has an 'any' typeerrors.Fix
Add explicit type annotations to 5 callback parameters that relied on external library types for inference:
packages/ariakit/src/panel/Panel.tsx:35activeIdstring | null | undefinedpackages/ariakit/src/toolbar/ToolbarButton.tsx:49eMouseEvent<HTMLButtonElement>packages/ariakit/src/panel/PanelFileInput.tsx:27eChangeEvent<HTMLInputElement>packages/ariakit/src/badge/Badge.tsx:39eventMouseEvent<HTMLButtonElement>packages/xl-ai/src/components/AIMenu/AIMenuController.tsx:73eventMouseEventAll annotations use React/DOM built-in types rather than library-specific types, making them resilient to future dependency updates.
Verification
Full fresh install simulation was run locally:
pnpm install(from lockfile)pnpm update --prod(for all published packages — simulates fresh user install)pnpm dedupepnpm run build— 0 errorspnpm run test— 27/27 targets succeededSummary by CodeRabbit