Upgrade project to Vite 8 and update dependencies#347
Conversation
…rg#343) perf: throttle pointer move events and guard pointer capture
WalkthroughThis PR encompasses tooling upgrades (Biome 2.4.14, Vite 8.0.10, TanStack packages, nitro), refactors ScreenMirror to handle pointer gestures internally with absolute coordinate support, extends input handling with a new "absolute" coordinate mode, updates UI error/not-found pages, and applies miscellaneous code cleanup and icon imports. ChangesDependency and Configuration Updates
Trackpad Gesture Handling Refactor
Remote Input Handling System
UI and Provider Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.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: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/routes/settings.tsx (1)
324-344:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWait for the
config-updatedack before navigating.This redirect is still timer-driven, so any server-side failure will send the user to the new port anyway, even though
src/server/websocket.ts:233-335reportsconfig-updatedsuccess/failure on the same socket. Also, Line 342 should use the validatedportvalue;parseInt("3000foo", 10)currently passes Line 313, but${frontendPort}produces an invalid destination.Suggested fix
socket.onerror = () => { alert("Failed to connect to the server.") } + + socket.onmessage = (event) => { + try { + const data = JSON.parse(event.data) + if (data.type !== "config-updated") return + + if (!data.success) { + alert(data.error ?? "Failed to update the server config.") + socket.close() + return + } + + window.setTimeout(() => { + socket.close() + const newUrl = `${window.location.protocol}//${window.location.hostname}:${port}/settings` + window.location.href = newUrl + }, 2500) + } catch { + alert("Failed to parse the server response.") + socket.close() + } + } socket.onopen = () => { socket.send( JSON.stringify({ type: "update-config", @@ }, }), ) - - setTimeout(() => { - socket.close() - const newProtocol = window.location.protocol - const newHostname = window.location.hostname - const newUrl = `${newProtocol}//${newHostname}:${frontendPort}/settings` - window.location.href = newUrl - }, 2500) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/settings.tsx` around lines 324 - 344, The timer-based redirect in socket.onopen should be replaced by waiting for the server's "config-updated" ack on the same WebSocket before navigating: send the update in socket.onopen (as already done), remove the setTimeout/forced close, add a socket.onmessage handler that listens for a message with type "config-updated", check the ack payload to ensure success (and handle failure by showing an error instead of redirecting), then close the socket and navigate; also use the validated numeric port variable (port) when building the redirect URL (not frontendPort string) and ensure port was parsed/validated (e.g., parseInt result is a valid port) before interpolating it into the destination URL.vite.config.ts (1)
31-35:⚠️ Potential issue | 🟠 Major
tanstackStart()must come beforenitro()in the plugins array.The current plugin order has
nitro()(line 32) beforetanstackStart()(line 34), but the official TanStack Start and Nitro documentation specifiestanstackStart()should come beforenitro(). Per the Nitro official example, the correct order is:viteTsConfigPaths,tanstackStart,viteReact,tailwindcss,nitro(last).nitro()must be positioned last to properly consume the Vite environments configured by earlier plugins, preventing conflicts like duplicate client entries.🔧 Proposed fix
plugins: [ { name: "websocket-server", ... }, devtools(), - nitro(), - tanstackStart(), + nitro(), viteReact(), ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vite.config.ts` around lines 31 - 35, Reorder the Vite plugins so tanstackStart() runs before nitro() and nitro() is last; specifically, in the plugins array move tanstackStart() to appear before nitro() (ideally follow the sequence viteTsConfigPaths, tanstackStart(), viteReact(), tailwindcss(), nitro()) to ensure Nitro consumes environments configured by earlier plugins and avoid duplicate client entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@biome.json`:
- Around line 12-19: The includes array in biome.json is missing the glob "**"
as the first entry so the negation patterns (!src/routeTree.gen.ts,
!src/styles.css) don't take effect; update the files.includes array to insert
"**" as the first entry (before "src/**/*") so Biome will match all files first
and then correctly apply the negations to exclude src/routeTree.gen.ts and
src/styles.css.
In `@package.json`:
- Around line 27-28: The package.json currently lists both "nitro" and
"nitro-nightly" in dependencies which risks non-reproducible production
installs; decide whether you need the nightly at runtime—if not, remove
"nitro-nightly" from dependencies (or move it to devDependencies) and/or express
any necessary pinning via "overrides" or "resolutions" instead; if you do need a
specific nightly, replace the caret range with an exact pinned version for
"nitro-nightly" (or remove the caret from "nitro" too) so installs are
deterministic, and ensure references in package.json and any build/deploy
scripts use the chosen package (nitro vs nitro-nightly).
- Line 42: The package dependency "@tanstack/react-router-devtools" in
package.json is pinned at ^1.132.0 and is out-of-sync with the rest of the
TanStack router packages; update the "@tanstack/react-router-devtools" entry to
match the router package version (e.g., ^1.169.1 or the exact version used by
"@tanstack/react-router"), then run your package manager to update node_modules
and the lockfile (npm/yarn/pnpm install), rebuild and run typechecks/tests to
ensure no type/runtime mismatches from the version bump.
- Around line 22-25: The package "@tanstack/router-plugin" is a build-time
Vite/codegen plugin and should be moved out of dependencies into
devDependencies; update package.json by removing the "@tanstack/router-plugin"
entry from the top-level "dependencies" object and add the same version under
"devDependencies" (preserve the version string), then run your package manager
to update the lockfile (npm/yarn/pnpm) so installs reflect the change; verify
nothing else imports it at runtime and adjust CI/build steps if they reference
installing production-only deps.
- Line 62: Remove the unused dependency "vite-tsconfig-paths" from package.json
(delete the "vite-tsconfig-paths": "^6.0.2" entry), then update the lockfile by
running the project's package manager install command (npm/yarn/pnpm) so
package-lock.json / yarn.lock / pnpm-lock.yaml reflect the removal; verify there
are no imports of vite-tsconfig-paths in vite.config.ts or elsewhere and commit
the updated package.json and lockfile.
In `@src/components/Trackpad/ScreenMirror.tsx`:
- Around line 9-12: The isTracking property in ScreenMirrorProps is unused and
should be removed or actually utilized; update the ScreenMirror component by
deleting isTracking from the ScreenMirrorProps interface and from the
component's prop destructuring/usage, and then remove the corresponding prop
passed in the caller (trackpad.tsx) so callers match the new API; alternatively,
if tracking state is required, wire the existing isTracking prop into the
ScreenMirror component logic (e.g., enable/disable rendering or CSS based on
isTracking) and keep callers as-is — choose one approach and make the
corresponding change consistently for ScreenMirror and its callers.
- Around line 92-96: The scroll sensitivity multiplier 1.5 in the scroll
handling block should be extracted to a named constant for clarity; define a
descriptive constant (e.g., SCROLL_SENSITIVITY or SCROLL_MULTIPLIER) near the
top of the ScreenMirror component or module and replace the literal 1.5 in the
if (scrollMode && lastPos.current) branch where send({ type: "scroll", dx: -dx *
1.5, dy: -dy * 1.5 }) is called with the constant (e.g., -dx *
SCROLL_SENSITIVITY), keeping lastPos.current and send unchanged.
In `@src/routes/__root.tsx`:
- Around line 23-26: Replace the hardcoded user-facing strings in the root error
UI ("Something went wrong!" and "404 - Not Found") with calls to your i18n
resource lookup so they are loaded from locale files; locate the strings in the
root route component (the JSX that renders the error block using
props.error.message and the 404 branch) and swap the literals for the i18n
accessor your app uses (e.g., t('error.somethingWrong') and t('error.notFound')
or the equivalent translation utility), then add corresponding keys to the
locale resource files.
In `@src/routes/trackpad.tsx`:
- Line 227: Remove the unused isTracking prop from the ScreenMirror component
call in trackpad.tsx: delete the isTracking={isTracking} attribute from the
<ScreenMirror ... /> JSX (after you update the ScreenMirror component/interface
to no longer declare or accept isTracking). Verify there are no other references
to isTracking inside the ScreenMirror import or Trackpad component that rely on
passing it; if any logic depended on it, migrate that logic into the parent or
into ScreenMirror's internal state as appropriate.
In `@src/server/InputHandler.ts`:
- Around line 161-182: The "absolute" branch in InputHandler.ts currently
processes every incoming absolute position (case "absolute") and can be abused
by clients to flood the server; add server-side throttling similar to the
existing "move" handling by tracking the last-processed timestamp (reuse or
mirror the same lastMove/lastMoveTs logic) and ignore or coalesce absolute
events that arrive faster than the configured minimum interval (e.g., ~16ms),
then only call moveRelative(msg.x,msg.y,true) and fallback to mouse.setPosition
when the throttle allows; reference the case "absolute" block, moveRelative,
mouse.setPosition, and ScreenMirror.tsx so the change mirrors the existing
"move" throttling behavior.
In `@src/server/ydotool.ts`:
- Around line 53-69: The moveRelative function builds ydotool mousemove args
incorrectly using -x and -y flags; update the arg construction in moveRelative
(which calls checkYdotool and uses ydotoolPath and execFileAsync) to push the
coordinates as positional arguments instead of flags — replace the
args.push("-x", String(dx), "-y", String(dy)) usage with args.push(String(dx),
String(dy)) so the final command becomes ydotool mousemove [--absolute] <x> <y>.
---
Outside diff comments:
In `@src/routes/settings.tsx`:
- Around line 324-344: The timer-based redirect in socket.onopen should be
replaced by waiting for the server's "config-updated" ack on the same WebSocket
before navigating: send the update in socket.onopen (as already done), remove
the setTimeout/forced close, add a socket.onmessage handler that listens for a
message with type "config-updated", check the ack payload to ensure success (and
handle failure by showing an error instead of redirecting), then close the
socket and navigate; also use the validated numeric port variable (port) when
building the redirect URL (not frontendPort string) and ensure port was
parsed/validated (e.g., parseInt result is a valid port) before interpolating it
into the destination URL.
In `@vite.config.ts`:
- Around line 31-35: Reorder the Vite plugins so tanstackStart() runs before
nitro() and nitro() is last; specifically, in the plugins array move
tanstackStart() to appear before nitro() (ideally follow the sequence
viteTsConfigPaths, tanstackStart(), viteReact(), tailwindcss(), nitro()) to
ensure Nitro consumes environments configured by earlier plugins and avoid
duplicate client entries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 06455215-9491-4885-9bde-fa08697974b1
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
biome.jsonpackage.jsonsrc/components/Trackpad/ExtraKeys.tsxsrc/components/Trackpad/ScreenMirror.tsxsrc/contexts/ConnectionProvider.tsxsrc/routes/__root.tsxsrc/routes/settings.tsxsrc/routes/trackpad.tsxsrc/server/InputHandler.tssrc/server/websocket.tssrc/server/ydotool.tssrc/utils/logger.tsvite.config.ts
💤 Files with no reviewable changes (2)
- src/utils/logger.ts
- src/components/Trackpad/ExtraKeys.tsx
| "includes": [ | ||
| "src/**/*", | ||
| ".vscode/**/*", | ||
| "index.html", | ||
| "vite.config.ts", | ||
| "!src/routeTree.gen.ts", | ||
| "!src/styles.css" | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Biome files.includes negation pattern documentation ** first entry
💡 Result:
In Biome, the first-entry documentation for negation patterns under files.includes is: “includes also supports negated patterns, or exceptions. These are patterns that start with ! … When using a negated pattern, you should always specify first to match all files and folders, otherwise the negated pattern will not match any files.” Example from the docs: { "files": { "includes": ["", "!/*.test.js", "/special.test.js", "!test"] } } This means ! entries in files.includes are “exceptions” that stop Biome from processing paths that match them, but they require a preceding positive include like (and exceptions are applied in order).
Citations:
Add ** as the first entry in files.includes to enable the negation patterns to work correctly.
The Biome documentation specifies: "When using a negated pattern, you should always specify ** first to match all files and folders, otherwise the negated pattern will not match any files." Without ** as the first entry, the negation patterns !src/routeTree.gen.ts and !src/styles.css will silently have no effect, causing the auto-generated route tree file to be processed by the linter and formatter.
Proposed fix
"files": {
"ignoreUnknown": false,
"includes": [
+ "**",
"src/**/*",
".vscode/**/*",
"index.html",
"vite.config.ts",
"!src/routeTree.gen.ts",
"!src/styles.css"
]
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@biome.json` around lines 12 - 19, The includes array in biome.json is missing
the glob "**" as the first entry so the negation patterns
(!src/routeTree.gen.ts, !src/styles.css) don't take effect; update the
files.includes array to insert "**" as the first entry (before "src/**/*") so
Biome will match all files first and then correctly apply the negations to
exclude src/routeTree.gen.ts and src/styles.css.
| "@tanstack/react-router": "^1.169.1", | ||
| "@tanstack/react-router-ssr-query": "^1.166.12", | ||
| "@tanstack/react-start": "^1.167.61", | ||
| "@tanstack/router-plugin": "^1.167.32", |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
@tanstack/router-plugin belongs in devDependencies, not dependencies.
@tanstack/router-plugin is a Vite/build-time code-generation plugin with no runtime role. Listing it under dependencies unnecessarily inflates production install footprints.
📦 Proposed fix
"dependencies": {
"@nut-tree-fork/nut-js": "^4.2.6",
"@tanstack/react-router": "^1.169.1",
"@tanstack/react-router-ssr-query": "^1.166.12",
"@tanstack/react-start": "^1.167.61",
- "@tanstack/router-plugin": "^1.167.32",
...
},
"devDependencies": {
+ "@tanstack/router-plugin": "^1.167.32",
...
},📝 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.
| "@tanstack/react-router": "^1.169.1", | |
| "@tanstack/react-router-ssr-query": "^1.166.12", | |
| "@tanstack/react-start": "^1.167.61", | |
| "@tanstack/router-plugin": "^1.167.32", | |
| "dependencies": { | |
| "@nut-tree-fork/nut-js": "^4.2.6", | |
| "@tanstack/react-router": "^1.169.1", | |
| "@tanstack/react-router-ssr-query": "^1.166.12", | |
| "@tanstack/react-start": "^1.167.61", | |
| }, | |
| "devDependencies": { | |
| "@tanstack/router-plugin": "^1.167.32", | |
| ... | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 22 - 25, The package "@tanstack/router-plugin" is
a build-time Vite/codegen plugin and should be moved out of dependencies into
devDependencies; update package.json by removing the "@tanstack/router-plugin"
entry from the top-level "dependencies" object and add the same version under
"devDependencies" (preserve the version string), then run your package manager
to update the lockfile (npm/yarn/pnpm) so installs reflect the change; verify
nothing else imports it at runtime and adjust CI/build steps if they reference
installing production-only deps.
| "nitro": "^3.0.260429-beta", | ||
| "nitro-nightly": "^3.0.1-20260501-164602-aee73f19", |
There was a problem hiding this comment.
Both nitro and nitro-nightly listed in production dependencies.
Having two overlapping beta/nightly Nitro packages in production dependencies raises concerns:
nitro-nightly(a nightly build) is a volatile, unpinned-in-practice dependency in a production context.- With
^ranges on both, anynpm installmay pull different nightly builds across environments, risking non-reproducible builds. - If
nitro-nightlyexists solely as a sub-dependency pin, it should be expressed viaoverrides/resolutionsrather than as a directdependency.
Clarify the intent — if only nitro is needed at runtime, remove nitro-nightly or demote it to overrides.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 27 - 28, The package.json currently lists both
"nitro" and "nitro-nightly" in dependencies which risks non-reproducible
production installs; decide whether you need the nightly at runtime—if not,
remove "nitro-nightly" from dependencies (or move it to devDependencies) and/or
express any necessary pinning via "overrides" or "resolutions" instead; if you
do need a specific nightly, replace the caret range with an exact pinned version
for "nitro-nightly" (or remove the caret from "nitro" too) so installs are
deterministic, and ensure references in package.json and any build/deploy
scripts use the chosen package (nitro vs nitro-nightly).
| "@tanstack/react-devtools": "^0.7.0", | ||
| "@tanstack/devtools-vite": "^0.6.0", | ||
| "@tanstack/react-devtools": "^0.10.2", | ||
| "@tanstack/react-router-devtools": "^1.132.0", |
There was a problem hiding this comment.
@tanstack/react-router-devtools is ~37 minor versions behind the rest of the TanStack ecosystem.
@tanstack/react-router-devtools is pinned at ^1.132.0 while @tanstack/react-router was bumped to ^1.169.1. Devtools packages are typically tightly versioned against the router and divergence this large can cause type mismatches or devtools panels to misread internal router state at runtime.
📦 Proposed fix
- "@tanstack/react-router-devtools": "^1.132.0",
+ "@tanstack/react-router-devtools": "^1.169.1",📝 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.
| "@tanstack/react-router-devtools": "^1.132.0", | |
| "@tanstack/react-router-devtools": "^1.169.1", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 42, The package dependency
"@tanstack/react-router-devtools" in package.json is pinned at ^1.132.0 and is
out-of-sync with the rest of the TanStack router packages; update the
"@tanstack/react-router-devtools" entry to match the router package version
(e.g., ^1.169.1 or the exact version used by "@tanstack/react-router"), then run
your package manager to update node_modules and the lockfile (npm/yarn/pnpm
install), rebuild and run typechecks/tests to ensure no type/runtime mismatches
from the version bump.
| "typescript": "^5.7.2", | ||
| "vite": "^7.1.7", | ||
| "vite": "^8.0.10", | ||
| "vite-tsconfig-paths": "^6.0.2", |
There was a problem hiding this comment.
vite-tsconfig-paths is a dead dependency — it is no longer imported in vite.config.ts.
Path aliasing was migrated to resolve.alias in vite.config.ts, making the vite-tsconfig-paths package unused. It should be removed to keep the dependency graph clean.
📦 Proposed fix
- "vite-tsconfig-paths": "^6.0.2",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 62, Remove the unused dependency "vite-tsconfig-paths"
from package.json (delete the "vite-tsconfig-paths": "^6.0.2" entry), then
update the lockfile by running the project's package manager install command
(npm/yarn/pnpm) so package-lock.json / yarn.lock / pnpm-lock.yaml reflect the
removal; verify there are no imports of vite-tsconfig-paths in vite.config.ts or
elsewhere and commit the updated package.json and lockfile.
| if (scrollMode && lastPos.current) { | ||
| const dx = e.clientX - lastPos.current.x | ||
| const dy = e.clientY - lastPos.current.y | ||
| send({ type: "scroll", dx: -dx * 1.5, dy: -dy * 1.5 }) | ||
| lastPos.current = { x: e.clientX, y: e.clientY } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Extract scroll sensitivity multiplier to a named constant.
The magic number 1.5 for scroll sensitivity should be a named constant for clarity and maintainability.
Suggested refactor
+const SCROLL_SENSITIVITY = 1.5
+
export const ScreenMirror = ({ scrollMode }: ScreenMirrorProps) => {
// ...
if (scrollMode && lastPos.current) {
const dx = e.clientX - lastPos.current.x
const dy = e.clientY - lastPos.current.y
- send({ type: "scroll", dx: -dx * 1.5, dy: -dy * 1.5 })
+ send({ type: "scroll", dx: -dx * SCROLL_SENSITIVITY, dy: -dy * SCROLL_SENSITIVITY })📝 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.
| if (scrollMode && lastPos.current) { | |
| const dx = e.clientX - lastPos.current.x | |
| const dy = e.clientY - lastPos.current.y | |
| send({ type: "scroll", dx: -dx * 1.5, dy: -dy * 1.5 }) | |
| lastPos.current = { x: e.clientX, y: e.clientY } | |
| const SCROLL_SENSITIVITY = 1.5 | |
| export const ScreenMirror = ({ scrollMode }: ScreenMirrorProps) => { | |
| // ... other component code ... | |
| if (scrollMode && lastPos.current) { | |
| const dx = e.clientX - lastPos.current.x | |
| const dy = e.clientY - lastPos.current.y | |
| send({ type: "scroll", dx: -dx * SCROLL_SENSITIVITY, dy: -dy * SCROLL_SENSITIVITY }) | |
| lastPos.current = { x: e.clientX, y: e.clientY } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Trackpad/ScreenMirror.tsx` around lines 92 - 96, The scroll
sensitivity multiplier 1.5 in the scroll handling block should be extracted to a
named constant for clarity; define a descriptive constant (e.g.,
SCROLL_SENSITIVITY or SCROLL_MULTIPLIER) near the top of the ScreenMirror
component or module and replace the literal 1.5 in the if (scrollMode &&
lastPos.current) branch where send({ type: "scroll", dx: -dx * 1.5, dy: -dy *
1.5 }) is called with the constant (e.g., -dx * SCROLL_SENSITIVITY), keeping
lastPos.current and send unchanged.
| <div className="p-4 text-red-500"> | ||
| <h1 className="text-xl font-bold mb-2">Something went wrong!</h1> | ||
| <pre className="text-sm bg-base-300 p-4 rounded overflow-auto"> | ||
| {props.error.message} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
User-visible strings should be externalized for i18n.
The error message "Something went wrong!" and "404 - Not Found" are hardcoded. Per coding guidelines, user-visible strings should be externalized to resource files for internationalization.
Also applies to: 36-36
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/__root.tsx` around lines 23 - 26, Replace the hardcoded
user-facing strings in the root error UI ("Something went wrong!" and "404 - Not
Found") with calls to your i18n resource lookup so they are loaded from locale
files; locate the strings in the root route component (the JSX that renders the
error block using props.error.message and the 404 branch) and swap the literals
for the i18n accessor your app uses (e.g., t('error.somethingWrong') and
t('error.notFound') or the equivalent translation utility), then add
corresponding keys to the locale resource files.
| scrollMode={scrollMode} | ||
| handlers={handlers} | ||
| /> | ||
| <ScreenMirror isTracking={isTracking} scrollMode={scrollMode} /> |
There was a problem hiding this comment.
Consider removing unused isTracking prop.
As noted in the ScreenMirror.tsx review, the isTracking prop is not used by ScreenMirror. Consider removing it from the component call once the interface is updated.
Proposed fix (after updating ScreenMirror interface)
-<ScreenMirror isTracking={isTracking} scrollMode={scrollMode} />
+<ScreenMirror scrollMode={scrollMode} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/trackpad.tsx` at line 227, Remove the unused isTracking prop from
the ScreenMirror component call in trackpad.tsx: delete the
isTracking={isTracking} attribute from the <ScreenMirror ... /> JSX (after you
update the ScreenMirror component/interface to no longer declare or accept
isTracking). Verify there are no other references to isTracking inside the
ScreenMirror import or Trackpad component that rely on passing it; if any logic
depended on it, migrate that logic into the parent or into ScreenMirror's
internal state as appropriate.
| case "absolute": | ||
| if ( | ||
| typeof msg.x === "number" && | ||
| typeof msg.y === "number" && | ||
| Number.isFinite(msg.x) && | ||
| Number.isFinite(msg.y) | ||
| ) { | ||
| try { | ||
| // Attempt ydotool absolute movement first | ||
| const success = await moveRelative(msg.x, msg.y, true) | ||
|
|
||
| // Fallback to absolute positioning if ydotool is unavailable or fails | ||
| if (!success) { | ||
| await mouse.setPosition( | ||
| new Point(Math.round(msg.x), Math.round(msg.y)), | ||
| ) | ||
| } | ||
| } catch (err) { | ||
| console.error("Absolute move event failed:", err) | ||
| } | ||
| } | ||
| break |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider adding throttling for "absolute" messages.
The "move" type has throttling logic (lines 92-110) to prevent high-frequency events, but "absolute" bypasses this. Since ScreenMirror.tsx sends absolute positions on every pointer move (throttled to ~60fps client-side), the server-side throttling would be redundant. However, if other clients send absolute events without client-side throttling, this could cause performance issues.
The current client-side throttling at ~16ms intervals should be sufficient, but if you anticipate other clients connecting, consider adding server-side throttling or documenting the expectation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/InputHandler.ts` around lines 161 - 182, The "absolute" branch in
InputHandler.ts currently processes every incoming absolute position (case
"absolute") and can be abused by clients to flood the server; add server-side
throttling similar to the existing "move" handling by tracking the
last-processed timestamp (reuse or mirror the same lastMove/lastMoveTs logic)
and ignore or coalesce absolute events that arrive faster than the configured
minimum interval (e.g., ~16ms), then only call moveRelative(msg.x,msg.y,true)
and fallback to mouse.setPosition when the throttle allows; reference the case
"absolute" block, moveRelative, mouse.setPosition, and ScreenMirror.tsx so the
change mirrors the existing "move" throttling behavior.
| export async function moveRelative( | ||
| dx: number, | ||
| dy: number, | ||
| absolute = false, | ||
| ): Promise<boolean> { | ||
| if (!(await checkYdotool()) || !ydotoolPath) { | ||
| return false | ||
| } | ||
|
|
||
| try { | ||
| // ydotool mousemove -x <dx> -y <dy> | ||
| await execFileAsync(ydotoolPath, [ | ||
| "mousemove", | ||
| "-x", | ||
| String(dx), | ||
| "-y", | ||
| String(dy), | ||
| ]) | ||
| const args = ["mousemove"] | ||
| if (absolute) { | ||
| args.push("--absolute") | ||
| } | ||
| args.push("-x", String(dx), "-y", String(dy)) | ||
|
|
||
| await execFileAsync(ydotoolPath, args) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check ydotool mousemove help for absolute positioning syntax
if command -v ydotool &> /dev/null; then
ydotool mousemove --help 2>&1 | head -30
else
echo "ydotool not installed in sandbox"
fiRepository: AOSSIE-Org/Rein
Length of output: 92
🏁 Script executed:
#!/bin/bash
# Search for ydotool mousemove usage in the codebase
rg "mousemove" --type ts --type tsx --type js --type jsx -B 3 -A 3Repository: AOSSIE-Org/Rein
Length of output: 87
🏁 Script executed:
#!/bin/bash
# Look for any ydotool-related documentation or tests
find . -type f \( -name "*test*" -o -name "*spec*" -o -name "*.md" \) | xargs rg -l "ydotool" 2>/dev/null || echo "No test/doc files found with ydotool"Repository: AOSSIE-Org/Rein
Length of output: 71
🏁 Script executed:
#!/bin/bash
# Check the entire ydotool.ts file for context and other usages
wc -l src/server/ydotool.ts && echo "---" && cat -n src/server/ydotool.tsRepository: AOSSIE-Org/Rein
Length of output: 2550
🌐 Web query:
ydotool mousemove --absolute syntax documentation
💡 Result:
The syntax for ydotool mousemove --absolute is: ydotool mousemove [--absolute] The --absolute (or -a) flag specifies absolute screen coordinates instead of relative movement. Without it, are relative offsets. Example from official documentation: ydotool mousemove --absolute 100 100 This moves the mouse pointer to absolute position (100, 100) on the screen. Note: Requires the ydotoold daemon to be running with access to /dev/uinput (typically needs root or proper permissions). For accurate absolute positioning, disable mouse acceleration in your desktop environment settings, as ydotool simulates input via uinput which can be affected by acceleration profiles.
Citations:
- 1: https://github.com/ReimuNotMoe/ydotool/blob/master/manpage/ydotool.1.scd
- 2: https://man.archlinux.org/man/ydotool.1.en
Remove -x and -y flags from ydotool mousemove invocation.
The ydotool mousemove command accepts coordinates as positional arguments, not flags. The current implementation at line 67 incorrectly uses -x and -y flags which are not recognized by ydotool. Change args.push("-x", String(dx), "-y", String(dy)) to args.push(String(dx), String(dy)). The correct syntax is ydotool mousemove [--absolute] <x> <y>.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/ydotool.ts` around lines 53 - 69, The moveRelative function builds
ydotool mousemove args incorrectly using -x and -y flags; update the arg
construction in moveRelative (which calls checkYdotool and uses ydotoolPath and
execFileAsync) to push the coordinates as positional arguments instead of flags
— replace the args.push("-x", String(dx), "-y", String(dy)) usage with
args.push(String(dx), String(dy)) so the final command becomes ydotool mousemove
[--absolute] <x> <y>.
|
From reviewing this PR, it looks like there are several unnecessary changes mixed in with the actual fixes you’re proposing. I’d recommend limiting the commits and file changes strictly to what’s relevant to the issue being addressed. |
2 participants
Addressed Issues:
Fixes #327
Description
Screen Mirror
Authentication
Basic Gestures
Modes & Settings
Advanced Input
Any other gesture or input behavior introduced:
Additional Notes:
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Chores