chore: improve typing for ContextualbarResizable resize handler#38595
chore: improve typing for ContextualbarResizable resize handler#38595TheRazorbill wants to merge 6 commits into
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: 932285e The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
WalkthroughExtracts the inline resize callback in ContextualbarResizable into a named, typed Changes
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. 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR enhances TypeScript type safety in the ContextualbarResizable component by refactoring the resize event handler. Instead of using an inline anonymous function with manual type casts, the handler is now extracted into a named function with explicit ResizeCallback typing from the re-resizable library. This improves code readability, maintainability, and allows TypeScript to properly infer parameter types without manual casting.
Changes:
- Extract inline resize handler into a named
handleResizefunction - Add explicit
ResizeCallbacktype annotation fromre-resizable - Remove inline type casts in favor of TypeScript inference
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Extract resize handler to named function - Type handler with ResizeCallback from re-resizable - Remove type cast, let TypeScript infer parameter types - Improve code readability and maintainability Instead of inline cast 'as ResizeCallback', we now define handleResize as a named function with explicit ResizeCallback type. This makes the code cleaner and lets TypeScript automatically infer the correct types for all parameters (_e: MouseEvent | TouchEvent, _dir: Direction, elRef: HTMLElement)
TheRazorbill
force-pushed
the
fix/improve-header-and-contextualbar-typing
branch
from
a415fe6 to
05b0171
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/ui-client/src/components/Contextualbar/ContextualbarResizable.tsx (1)
22-27: Consider moving the type to the generic parameter rather than the variable annotation.The current pattern
const handleResize: ResizeCallback = useCallback(...)relies on contextual typing flowing from the variable annotation throughuseCallback's generic constraint (T extends Function) to type_e,_dir, andelRef. This works correctly in TypeScript 5.x, but placing the type as an explicit generic argument is more direct and makes the intent unambiguous:The
re-resizablesource exportsResizeCallbackas(event: MouseEvent | TouchEvent, direction: Direction, elementRef: HTMLElement, delta: NumberSize) => void, confirmingelRef.style.widthaccess is safe regardless of approach.♻️ Suggested alternative for more explicit typing
- const handleResize: ResizeCallback = useCallback( + const handleResize = useCallback<ResizeCallback>( (_e, _dir, elRef) => { setContextualbarWidth(elRef.style.width); }, [setContextualbarWidth], );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui-client/src/components/Contextualbar/ContextualbarResizable.tsx` around lines 22 - 27, The handler's type should be applied as a generic to useCallback instead of as a variable annotation; change the declaration of handleResize so it uses useCallback<ResizeCallback>(...) (keeping the same parameters and body that call setContextualbarWidth(elRef.style.width)) rather than const handleResize: ResizeCallback = useCallback(...), referencing ResizeCallback, useCallback, handleResize, setContextualbarWidth and elRef.style.width to make the typing explicit and unambiguous.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/rotten-hounds-destroy.mdpackages/ui-client/src/components/Contextualbar/ContextualbarResizable.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/ui-client/src/components/Contextualbar/ContextualbarResizable.tsx
🔇 Additional comments (3)
.changeset/rotten-hounds-destroy.md (1)
1-5: LGTM!The patch-level bump is correct, and the description accurately reflects both the typing improvement and the minor memoization benefit introduced by
useCallback.packages/ui-client/src/components/Contextualbar/ContextualbarResizable.tsx (2)
4-6: LGTM!
import typeforResizeCallbackis correct (erased at emit), anduseCallbackis properly sourced fromreact.
29-44: LGTM!
onResize={handleResize}placed after{...props}is consistent with the previous inline handler's position, preserving the intentional override of any caller-suppliedonResize.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/ui-client/src/components/Contextualbar/ContextualbarResizable.tsx`:
- Around line 22-27: The handler's type should be applied as a generic to
useCallback instead of as a variable annotation; change the declaration of
handleResize so it uses useCallback<ResizeCallback>(...) (keeping the same
parameters and body that call setContextualbarWidth(elRef.style.width)) rather
than const handleResize: ResizeCallback = useCallback(...), referencing
ResizeCallback, useCallback, handleResize, setContextualbarWidth and
elRef.style.width to make the typing explicit and unambiguous.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38595 +/- ##
===========================================
+ Coverage 70.66% 70.68% +0.01%
===========================================
Files 3191 3191
Lines 112965 112965
Branches 20451 20504 +53
===========================================
+ Hits 79829 79846 +17
+ Misses 31088 31069 -19
- Partials 2048 2050 +2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Updates completed and tested. |
Proposed changes
This PR improves TypeScript type safety in
ContextualbarResizableby refining theonResizehandler implementation:handleResize)ResizeCallbackfromre-resizableas ResizeCallback) and rely on TypeScript inferenceInstead of using an inline cast, we now define
handleResizewith an explicitResizeCallbacktype.This keeps the code cleaner and allows TypeScript to automatically infer the correct parameter types:
_e: MouseEvent | TouchEvent_dir: DirectionelRef: HTMLElementNo runtime behavior changes — this is purely a typing and code quality improvement.
Issue(s)
Closes: #38759
Steps to test or reproduce
Example:
yarn lint yarn testBefore
After
Summary by CodeRabbit