fix: add platform abstraction for deeplink #251
Reference in New Issue
Block a user
Delete Branch "fix-web-deep-link"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This was crashing App.tsx due to the deep link listener calling methods on window which do not exist.
Summary by CodeRabbit
New Features
Bug Fixes
No actionable comments were generated in the recent review. 🎉
ℹ️ Recent review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID:
0d8e5e91-eae0-4cb5-85d8-5753aabe69a8📥 Commits
Reviewing files that changed from the base of the PR and between
5275f382b4and5013975f18.📒 Files selected for processing (1)
js/desktop/src/App.tsx🚧 Files skipped from review as they are similar to previous changes (1)
📝 Walkthrough
Walkthrough
Adds a platform.deepLink API (types, Electron wiring, web stub) and migrates App's deep-link listener to use it; NetworkRoot's setStatusTab now always writes
statusto the URL and two render-only JSX comments were removed.Changes
Deep-Link Platform Abstraction and URL Parameter Updates
js/desktop/src/lib/platform/types.ts,js/desktop/src/lib/platform/electron.ts,js/desktop/src/lib/platform/web.ts,js/desktop/src/App.tsxPlatform.app.deepLinkwithgetPending()andonNavigate(). Electron wires these towindow.electronDeepLink; web provides inert stubs. App'sDeepLinkNavigationListenerusesplatform.deepLink.getPending()andplatform.deepLink.onNavigate(...), with added error logging for pending resolution.js/desktop/src/features/network-root.tsxsetStatusTabnow always sets thestatusquery parameter (openorclosed) instead of deleting it foropen. Two render-only JSX comments were removed.Sequence Diagram
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
🚥 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)
fix-web-deep-linkComment
@coderabbitai helpto get the list of available commands and usage tips.Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID:
3cf2cd46-c9e1-45a8-9e6e-a22427a49fa6📥 Commits
Reviewing files that changed from the base of the PR and between
a94a986fbfand198add3931.📒 Files selected for processing (5)
js/desktop/src/App.tsxjs/desktop/src/features/network-root.tsxjs/desktop/src/lib/platform/electron.tsjs/desktop/src/lib/platform/types.tsjs/desktop/src/lib/platform/web.ts@@ -56,10 +56,16 @@ function DeepLinkNavigationListener() {const navigate = useNavigate();⚠️ Potential issue | 🟠 Major | ⚡ Quick win
Add error handling for
getPending()promise.The
getPending()call has no error handler. If the underlying Electron IPC or platform implementation throws or rejects, this will result in an unhandled promise rejection that could crash the app or surface console errors.🛡️ Proposed fix to add error handling
📝 Committable suggestion
🤖 Prompt for AI Agents
✅ Addressed in commit
5275f38Fixes Applied Successfully
Fixed 1 file(s) based on 1 unresolved review comment.
Files modified:
js/desktop/src/App.tsxCommit:
5275f382b43ca2328b5676002c0a15076fc37d8eThe changes have been pushed to the
fix-web-deep-linkbranch.Time taken:
1m 29s@@ -56,10 +56,16 @@ function DeepLinkNavigationListener() {const navigate = useNavigate();Shouldn't your fix use either reportError or logError?
@@ -56,10 +56,16 @@ function DeepLinkNavigationListener() {const navigate = useNavigate();🧩 Analysis chain
🏁 Script executed:
Length of output: 397
🏁 Script executed:
Length of output: 28070
@talksik— you're right. The codebase has a clear distinction injs/desktop/src/lib/errors.ts:logError— "Expected-but-recordable failures. Breadcrumb only — never pages anyone." (dev-only console output)reportError— "Unexpected failures the user may not see. Always captured." (always sent to the observability sink)For
getPending(), a rejection is recoverable (navigation simply doesn't happen) and low-impact, which fitslogError's contract. This also matches the pattern used for similar call-site errors likeautoplay.beepandsoundEffects.preload. Updated suggestion:That said, if the team treats any IPC rejection as unexpected (similar to how
ipc-utils.tsusesreportErrorfor IPC handler errors on the main-process side),reportErrorwould also be justified. The call is yours based on the severity convention you want to apply here.@@ -56,10 +56,16 @@ function DeepLinkNavigationListener() {const navigate = useNavigate();@coderabbitai help
Chat
There are 3 ways to chat with CodeRabbit:
I pushed a fix in commit <commit_id>, please review it.Open a follow-up GitHub issue for this discussion.@coderabbitaiin a new review comment at the desired location with your query.@coderabbitaiin a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.CodeRabbit commands
@coderabbitai pauseto pause the reviews on a PR.@coderabbitai resumeto resume the paused reviews.@coderabbitai reviewto trigger an incremental review. This is useful when automatic reviews are disabled for the repository.@coderabbitai full reviewto do a full review from scratch and review all the files again.@coderabbitai rate limitto show your current review rate limit status.@coderabbitai summaryto regenerate the summary of the PR.@coderabbitai evaluate custom pre-merge check --instructions <custom-checks-instructions> --name <custom-checks-title> [--mode <error|warning>]to validate the custom pre-merge checks instructions. Defaults toerrorwhen--modeis omitted.@coderabbitai ignore pre-merge checksto override pre-merge checks and get an approval on PR.@coderabbitai run pre-merge checksto run pre-merge checks on the pull request.@coderabbitai generate docstringsto generate docstrings for this PR.@coderabbitai generate sequence diagramto generate a sequence diagram of the changes in this PR.@coderabbitai generate unit teststo generate unit tests for this PR.@coderabbitai emit path instructionsto emit generated path-specific review instructions for this repository.@coderabbitai resolve merge conflictto automatically resolve merge conflicts.@coderabbitai autofixto automatically fix issues identified in unresolved review comments.@coderabbitai resolveto resolve all the CodeRabbit review comments.@coderabbitai configurationto show the current CodeRabbit configuration for the repository.@coderabbitai generate configurationto create a PR that adds the current resolved configuration as.coderabbit.yaml(or show it if already present).@coderabbitai helpto get help.Other keywords and placeholders
@coderabbitai ignoreor@coderabbit ignoreanywhere in the PR description to prevent this PR from being reviewed.@coderabbitai summaryto generate the high-level summary at a specific location in the PR description.@coderabbitaianywhere in the PR title to generate the title automatically.CodeRabbit configuration file (
.coderabbit.yaml).coderabbit.yamlfile to the root of your repository.# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.jsonStatus, support, documentation and community