Conversation
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughRemoved old deeplink types, commands, and extension trait; introduced a new notification-focused DeepLink parsing surface (requiring a "key" query param) with an event wrapper; updated plugin exports/specta config; added desktop route Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant DesktopApp as Desktop App
participant DeeplinkPlugin as Deeplink Plugin
Client->>DesktopApp: Navigate to /notification
DesktopApp->>DesktopApp: beforeLoad hook runs
DesktopApp->>DesktopApp: Redirect to /app/main
Note over DeeplinkPlugin: DeepLink parsing flow (new)
Client->>DeeplinkPlugin: Send deeplink URL (e.g., hypr://notification?key=...)
DeeplinkPlugin->>DeeplinkPlugin: Parse URL (FromStr)
alt Path == "notification"
DeeplinkPlugin->>DeeplinkPlugin: Require "key" query param
alt "key" present
DeeplinkPlugin->>DeeplinkPlugin: Emit DeepLinkEvent( Notification(NotificationSearch) )
else "key" missing
DeeplinkPlugin-->>Client: Return MissingQueryParam error
end
else unknown path
DeeplinkPlugin-->>Client: Return UnknownPath error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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). (10)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/desktop/src/routeTree.gen.tsis excluded by!**/*.gen.tsplugins/deeplink2/js/bindings.gen.tsis excluded by!**/*.gen.ts
📒 Files selected for processing (10)
apps/desktop/src/routes/__root.tsx(1 hunks)apps/desktop/src/routes/notification.tsx(1 hunks)plugins/deeplink2/build.rs(1 hunks)plugins/deeplink2/src/commands.rs(0 hunks)plugins/deeplink2/src/error.rs(1 hunks)plugins/deeplink2/src/ext.rs(0 hunks)plugins/deeplink2/src/lib.rs(1 hunks)plugins/deeplink2/src/types.rs(0 hunks)plugins/deeplink2/src/types/mod.rs(1 hunks)plugins/deeplink2/src/types/notification.rs(1 hunks)
💤 Files with no reviewable changes (3)
- plugins/deeplink2/src/commands.rs
- plugins/deeplink2/src/ext.rs
- plugins/deeplink2/src/types.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/routes/__root.tsxapps/desktop/src/routes/notification.tsx
🧠 Learnings (3)
📚 Learning: 2025-11-24T16:32:19.706Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:19.706Z
Learning: Applies to **/*.{ts,tsx} : Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props. Just inline them.
Applied to files:
apps/desktop/src/routes/__root.tsx
📚 Learning: 2025-11-24T16:32:23.036Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:23.036Z
Learning: Applies to **/*.{ts,tsx} : Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Applied to files:
apps/desktop/src/routes/__root.tsx
📚 Learning: 2025-11-24T16:32:19.706Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:19.706Z
Learning: Applies to **/*.{ts,tsx} : Use `motion/react` instead of `framer-motion`.
Applied to files:
apps/desktop/src/routes/__root.tsx
🧬 Code graph analysis (2)
plugins/deeplink2/src/types/mod.rs (1)
plugins/deeplink2/src/lib.rs (1)
tauri_specta(9-13)
apps/desktop/src/routes/notification.tsx (1)
apps/desktop/src/routes/__root.tsx (1)
Route(22-26)
⏰ 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). (7)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
- GitHub Check: ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: ci (macos, macos-15)
🔇 Additional comments (7)
plugins/deeplink2/build.rs (1)
1-4: LGTM!The empty
COMMANDSslice aligns with the removal of the commands module from the plugin.apps/desktop/src/routes/notification.tsx (1)
3-7: Query parameters are lost during redirect.The
/notificationdeep link expects akeyquery parameter (as defined inNotificationSearch), but the redirect discards all search params. If the notification key needs to be preserved, pass it through:export const Route = createFileRoute("/notification")({ - beforeLoad: async () => { - throw redirect({ to: "/app/main" }); + beforeLoad: async ({ search }) => { + throw redirect({ to: "/app/main", search }); }, });If the key is intentionally discarded (e.g., processed elsewhere before this redirect), please disregard.
apps/desktop/src/routes/__root.tsx (1)
18-20: LGTM!Clever compile-time assertion ensuring
DeepLink["to"]always maps to a valid router path. This will catch mismatches at build time.plugins/deeplink2/src/error.rs (1)
13-14: LGTM!The new
MissingQueryParamvariant follows the existing error pattern and integrates well with theFromStrimplementation forDeepLink.plugins/deeplink2/src/types/mod.rs (2)
1-19: LGTM!The module structure is clean. The
DeepLinkEventwrapper integrates properly withtauri_specta, and the serde tagging produces a clear JSON structure.
21-49: LGTM with minor note.The
FromStrimplementation correctly parses deep link URLs. Note that usingHashMapfor query params means duplicate keys will keep the last value only—this is typically acceptable behavior.One minor suggestion: consider adding
#[inline]to this implementation if it's called frequently in hot paths, though this is optional.plugins/deeplink2/src/lib.rs (1)
11-13: Code changes verified — all type definitions properly configured.Verification confirms all types referenced in the specta builder are correctly defined with the necessary derives for tauri-specta integration:
DeepLinkEvent(line 12 of types/mod.rs): Struct withtauri_specta::Eventderive ✓DeepLink(lines 15-19 of types/mod.rs): Enum withSerialize, Deserialize, Typederives ✓NotificationSearch(notification.rs): Properly defined and exported ✓The builder configuration at lines 11-13 is correct: empty commands list, proper event binding, and type export via
.typ::<>().
…2 types Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
1 participant
Summary
Refactors the deeplink2 plugin to provide type-safe path handling with compile-time validation that deep link paths match valid application routes.
Key changes:
ping,get_available_deep_links) and extension trait (DeepLink2PluginExt)FromStrimplementation for URL parsing with query parameter validation/notificationroute that redirects to/app/main__root.tsxto ensureDeepLink["to"]matches valid routesReview & Testing Checklist for Human
bindings.gen.tsshowsOnboardingSearch/OnboardingSteptypes but Rust code definesNotificationSearch. Verify bindings are regenerated correctly (cargo testin plugins/deeplink2 should regenerate)0 as DeepLink["to"] extends NonNullable<LinkProps["to"]>...) actually catches invalid routes at build timehyprnote://notification?key=test- should parse correctlykeyparam - should returnMissingQueryParamerror/notificationroute redirects to/app/mainas expectedRecommended test plan:
pnpm -F desktop tauri devand verify app builds without type errors/notification?key=testand verify redirect behavior/notificationroute causes a compile error due to the type assertionNotes
This is a breaking change that removes the
DeepLink2PluginExttrait and associated commands. Any code depending onparse_deep_link()orget_available_deep_links()will need updates.Link to Devin run: https://app.devin.ai/sessions/c57f639984e64d2a8d1dc55be07cc869
Requested by: yujonglee (@yujonglee)