-
Notifications
You must be signed in to change notification settings - Fork 4
Make it work by bumping dependencies to latest and fixing types #5
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
Conversation
📝 WalkthroughWalkthroughThe pull request updates package.json with new metadata fields and refreshes dependency/devDependency versions, refactors the API by removing a type annotation from the focusApp function, and systematically updates UI components to use a new property naming convention ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/api.ts (1)
69-78: Consider adding type annotation for params to restore type safety.The removal of
AppFocusParamssuggests the type no longer exists in@beeper/desktop-api4.x. While the untyped params allows the code to compile, it sacrifices type safety.🔎 Consider adding an inline type for params
-export const focusApp = async (params = {}) => { +export const focusApp = async (params: Record<string, unknown> = {}) => { try { await getBeeperDesktop().focus(params); await closeMainWindow();Or define an inline interface if the shape is known:
+interface FocusParams { + chatID?: string; + [key: string]: unknown; +} + -export const focusApp = async (params = {}) => { +export const focusApp = async (params: FocusParams = {}) => { try { await getBeeperDesktop().focus(params); await closeMainWindow();
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
metadata/beeper-1.pngis excluded by!**/*.pngmetadata/beeper-2.pngis excluded by!**/*.pngpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
package.jsonsrc/api.tssrc/find-chat.tsxsrc/list-chats.tsxsrc/search-chats.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/find-chat.tsx (1)
src/api.ts (1)
focusApp(69-78)
🔇 Additional comments (7)
package.json (2)
8-24: LGTM: Metadata additions align with Raycast extension standards.The addition of contributors, platforms, keywords, and categories enriches the extension manifest without affecting functionality.
56-66: All dependency versions exist and are secure.The major version bumps for
@beeper/desktop-api(0.x → 4.2.3) and@raycast/utils(1.x → 2.x) align with the breaking API changes elsewhere in the PR. All specified versions are available on npm with no known vulnerabilities.src/find-chat.tsx (2)
9-11: LGTM: Data fetching logic correctly handles empty search and API response structure.The conditional params logic appropriately fetches all chats when search is empty, and the
result.items || []fallback safely handles undefined responses.
18-34: LGTM: Chat property and action handlers updated correctly.The migration from
chat.chatIDtochat.idis applied consistently, and thefocusApp({ chatID: chat.id })call correctly maps the new property to the expected parameter name.src/list-chats.tsx (1)
13-38: LGTM: API response and chat property updates are consistent.The changes from
result.datatoresult.items || []and fromchat.chatIDtochat.idalign with the @beeper/desktop-api 4.x update and are applied consistently throughout the component.src/search-chats.tsx (1)
10-32: LGTM: API response structure and property naming updated consistently.The shift from
result.datatoresult.items || []and fromchat.chatIDtochat.idmatches the changes across all other files and aligns with the @beeper/desktop-api 4.x breaking changes.src/api.ts (1)
69-78: No issues identified. Thefocusmethod exists on BeeperDesktop client in version 4.2.3 and the implementation correctly callsgetBeeperDesktop().focus(params)with the proper API signature.
Overview
When I installed it and ran locally only the "List Accounts" command was working the others "Focus Desktop App" and "Find Chat" were broken. I know it is a work in progress. Since I'd like to use it I wrote this PR to make it work.
This PR does not add new features. It just bumps deps and updates types to make existing stuff work.
Summary of changes
Testing
Ran all three commands locally and confirmed no errors 🙆♂️
Tested using Beeper Desktop v4.2.367
Note
Can't wait to add some features 👁️👄👁️