Uh oh!
There was an error while loading. Please reload this page.
fix(tauri): Extract IPC types to shared module for Windows compatibility - #16
Conversation
The Windows build was failing because tcp_ipc was trying to import types from the ipc module, which is Unix-only (#[cfg(unix)]). This caused: error[E0432]: unresolved import `crate::ipc` Solution: - Created ipc_common.rs with IpcCommand, IpcResponse, and process_command - This module compiles on all platforms (no cfg restrictions) - Both ipc.rs (Unix socket) and tcp_ipc.rs (TCP) now import from ipc_common - Removed duplicate code from ipc.rs This fixes the Windows release build failure in v0.5.1.
📝 WalkthroughWalkthroughExtract and consolidate IPC command types and processing logic into a shared Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/tauri/src-tauri/src/ipc_common.rs (1)
8-9:#[serde(default)]onOption<String>is redundant.Serde already deserializes a missing JSON field as
Nonefor anyOption<T>without needing the attribute. The annotation is harmless but adds noise.♻️ Suggested cleanup
- #[serde(default)] pub path: Option<String>,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src-tauri/src/ipc_common.rs` around lines 8 - 9, The #[serde(default)] attribute on the struct field `path: Option<String>` is redundant; remove the #[serde(default)] attribute applied to `path` (the `Option<String>` field) so Serde will naturally treat a missing field as None—update the declaration that contains `path` to simply be `pub path: Option<String>` and ensure no other serde attributes are needed for that field.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/tauri/src-tauri/src/ipc_common.rs`:
- Around line 25-27: std::fs::canonicalize produces Windows extended-length
paths prefixed with "\\?\" which breaks downstream consumers (e.g., Node.js);
update the canonicalization in ipc_common.rs where canonicalize(&path) and
path_str are produced to remove the "\\?\" prefix on Windows (e.g., check if
path_str starts with r"\\?\" and strip that prefix) or replace the call with
dunce::canonicalize (add dunce = "1" to Cargo.toml) so the returned path string
is safe for the frontend "open-file" handler; ensure the adjusted path_str is
used in the existing code paths that emit the IPC event.
---
Nitpick comments:
In `@apps/tauri/src-tauri/src/ipc_common.rs`:
- Around line 8-9: The #[serde(default)] attribute on the struct field `path:
Option<String>` is redundant; remove the #[serde(default)] attribute applied to
`path` (the `Option<String>` field) so Serde will naturally treat a missing
field as None—update the declaration that contains `path` to simply be `pub
path: Option<String>` and ensure no other serde attributes are needed for that
field.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/tauri/src-tauri/src/ipc.rsapps/tauri/src-tauri/src/ipc_common.rsapps/tauri/src-tauri/src/lib.rsapps/tauri/src-tauri/src/tcp_ipc.rs
| match std::fs::canonicalize(&path) { | ||
| Ok(abs_path) => { | ||
| let path_str = abs_path.to_string_lossy().to_string(); |
There was a problem hiding this comment.
std::fs::canonicalize emits \\?\-prefixed UNC paths on Windows.
On Windows, canonicalize converts the path to use extended-length path syntax (\\?\...), which may be incompatible with other applications. Node.js crashes when given such a \\?\-prefixed UNC path, meaning the frontend "open-file" event handler will receive a broken path string on Windows — the primary platform this PR adds support for.
Consider stripping the prefix after canonicalization, or using the dunce crate, which is frequently used as a wrapper over std::fs::canonicalize to strip the \\?\ prefix.
🛡️ Suggested fix (manual prefix strip, no extra dep)
- Ok(abs_path) => {- let path_str = abs_path.to_string_lossy().to_string();+ Ok(abs_path) => {+ // Strip Windows extended-length prefix so the frontend+ // receives a plain path like C:\... instead of \\?\C:\...+ let path_str = abs_path.to_string_lossy()+ .strip_prefix(r"\\?\")+ .map(str::to_string)+ .unwrap_or_else(|| abs_path.to_string_lossy().to_string());Alternatively, add dunce = "1" to Cargo.toml and use:
let abs_path = dunce::canonicalize(&path).map_err(...)?;📝 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.
| match std::fs::canonicalize(&path){ | |
| Ok(abs_path) => { | |
| let path_str = abs_path.to_string_lossy().to_string(); | |
| match std::fs::canonicalize(&path){ | |
| Ok(abs_path) => { | |
| // Strip Windows extended-length prefix so the frontend | |
| // receives a plain path like C:\... instead of \\?\C:\... | |
| let path_str = abs_path.to_string_lossy() | |
| .strip_prefix(r"\\?\") | |
| .map(str::to_string) | |
| .unwrap_or_else(|| abs_path.to_string_lossy().to_string()); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/tauri/src-tauri/src/ipc_common.rs` around lines 25 - 27,
std::fs::canonicalize produces Windows extended-length paths prefixed with
"\\?\" which breaks downstream consumers (e.g., Node.js); update the
canonicalization in ipc_common.rs where canonicalize(&path) and path_str are
produced to remove the "\\?\" prefix on Windows (e.g., check if path_str starts
with r"\\?\" and strip that prefix) or replace the call with dunce::canonicalize
(add dunce = "1" to Cargo.toml) so the returned path string is safe for the
frontend "open-file" handler; ensure the adjusted path_str is used in the
existing code paths that emit the IPC event.
Uh oh!
There was an error while loading. Please reload this page.
Summary
Fixes the Windows build failure in release v0.5.1.
The Windows build was failing with:
Root Cause
The
tcp_ipcmodule was trying to import types from theipcmodule, which is Unix-only (#[cfg(unix)]). On Windows, this module doesn't exist, causing the import to fail.Solution
ipc_common.rsmodule with shared types and logic:IpcCommandstructIpcResponsestructprocess_command()functionipc.rs(Unix socket) andtcp_ipc.rs(TCP) now import fromipc_commonipc.rsTest Plan
cargo checkRelated
Summary by CodeRabbit