fix: upgrade dependencies, enable React Compiler support, and improve Vite 8 compatibility#349
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
WalkthroughBumps runtime/dev dependencies, updates Biome schema and files.includes, modifies Vite base/alias and dev-server wiring, refactors ConnectionProvider and mirroring/capture hooks to remove useCallback and inline handlers, and applies small import/catch renames. ChangesDependency and Configuration Upgrade
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/contexts/ConnectionProvider.tsx (1)
33-34:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
childrenis dropped from props but still used, causing a hard failureLine 34 defines an empty destructuring pattern, but Line 199 renders
{children}. This leaveschildrenundefined in scope and breaks compilation.Proposed fix
-export function ConnectionProvider({ -}: { children: React.ReactNode }) { +export function ConnectionProvider({ + children, +}: { children: React.ReactNode }) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/contexts/ConnectionProvider.tsx` around lines 33 - 34, The ConnectionProvider function drops the children prop in its parameter destructuring but still uses {children} when rendering; restore children in the function signature by destructuring it from props (or accept props and extract children) so that the children variable is defined for use in ConnectionProvider (refer to the ConnectionProvider function and the children render usage).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@biome.json`:
- Around line 12-19: The current "includes" array contains negated globs that
won't match without a catch-all; update the biome.json "includes" entry to
either prepend an empty string ("") before the negated patterns so the negations
take effect (e.g., ensure "" appears before "!src/routeTree.gen.ts" and
"!src/styles.css"), or move those negated entries into a separate "ignores"
array and keep "includes" as the normal positive patterns (referencing the
"includes" key and the specific patterns "!src/routeTree.gen.ts" and
"!src/styles.css").
In `@package.json`:
- Line 62: The devDependency "vite-tsconfig-paths" is unused and should be
removed: delete the "vite-tsconfig-paths" entry from package.json's
devDependencies and run the package manager uninstall command (e.g., `npm
uninstall vite-tsconfig-paths`) to update package-lock.json/node_modules; verify
that vite.config.ts does not reference the plugin and run a test build to
confirm no breakage.
- Around line 22-28: The package `@tanstack/react-router` is pinned to 1.169.1 but
`@tanstack/router-plugin` requires ^1.169.2; update the dependency version for
"@tanstack/react-router" in package.json to ">=1.169.2" (e.g. "^1.169.2") and
then reinstall/update the lockfile (npm install / yarn install / pnpm install)
so the lockfile reflects the bumped version and peer dependency is satisfied.
- Line 37: The lockfile is out of sync: package.json declares "@biomejs/biome":
"^2.4.14" but package-lock.json still pins "^1.9.4", and biome.json is using the
v2 schema; run a fresh install to regenerate the lockfile (npm install) so
package-lock.json reflects `@biomejs/biome` v2.4.14, verify the updated
package-lock.json contains `@biomejs/biome` ^2.4.14 and that biome.json schema
still matches, then commit the updated package-lock.json.
---
Outside diff comments:
In `@src/contexts/ConnectionProvider.tsx`:
- Around line 33-34: The ConnectionProvider function drops the children prop in
its parameter destructuring but still uses {children} when rendering; restore
children in the function signature by destructuring it from props (or accept
props and extract children) so that the children variable is defined for use in
ConnectionProvider (refer to the ConnectionProvider function and the children
render usage).
🪄 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: 402369d1-e5f1-48a1-a9ad-d15e24fe36f2
📒 Files selected for processing (4)
biome.jsonpackage.jsonsrc/contexts/ConnectionProvider.tsxvite.config.ts
Contributor
|
@Arbaaz123676 I went through the PR and tested it locally. There are still a few outdated dependencies remaining, and I’m also seeing some issues/errors while running npm run dev on a fresh install. If you run into any issues while reproducing it, feel free to ask here. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Around line 27-28: The package.json currently pins pre-release packages
"nitro" (^3.0.260429-beta) and "nitro-nightly" (^3.0.1-20260501-164602-aee73f19)
in dependencies; change this by replacing "nitro" with a stable release (or
update the version to a stable semver) and move "nitro-nightly" from
dependencies to devDependencies if it's only needed for development/testing, or
add a brief rationale comment in your repo docs if you intentionally must keep
these bleeding-edge versions; update the package.json entries for "nitro" and
"nitro-nightly" accordingly and run install to verify no regressions.
In `@src/contexts/ConnectionProvider.tsx`:
- Around line 127-129: In ConnectionProvider (the catch around JSON parsing of
incoming WebSocket messages) stop silently swallowing errors: rename the catch
parameter from _e to e, log the error and the raw message payload in development
only (e.g. if (process.env.NODE_ENV === 'development') console.error('WS JSON
parse error', e, rawMessage)), and ensure the production branch remains silent
to avoid leaking sensitive data; update the code path that parses messages so
the debug log provides both the exception and the offending string for easier
debugging.
🪄 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: f7a493ea-4f95-4bee-9d45-3c1638b2a8fb
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
package.jsonsrc/components/Trackpad/ExtraKeys.tsxsrc/contexts/ConnectionProvider.tsxsrc/server/ydotool.ts
💤 Files with no reviewable changes (1)
- src/components/Trackpad/ExtraKeys.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useCaptureProvider.ts (1)
118-125:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUnmount cleanup should mirror
stopSharingand notify the peer.When the component unmounts mid-share, this cleanup clears the timer and stops local tracks but never sends
stop-mirrorto the WebSocket. The peer is left believing the provider is still streaming until it notices the connection drop or its own timeout. SincestopSharingalready centralizes this teardown (timer + tracks + WS notification), invoking it (or inlining the WS send) here keeps both paths symmetric.♻️ Suggested fix
useEffect(() => { return () => { if (timerRef.current) clearInterval(timerRef.current) if (streamRef.current) { for (const track of streamRef.current.getTracks()) track.stop() } + if (wsRef.current?.readyState === WebSocket.OPEN) { + wsRef.current.send(JSON.stringify({ type: "stop-mirror" })) + } } }, [])Note: calling
stopSharing()directly here would also work but introduces a render-scope dependency; the inline version is safer in a cleanup.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useCaptureProvider.ts` around lines 118 - 125, The cleanup in the useEffect currently clears timerRef and stops streamRef tracks but omits the WebSocket notification and shared state teardown performed by stopSharing; update the cleanup to mirror stopSharing by also sending the "stop-mirror" message over the WebSocket (using wsRef.current if present), clearing the interval (timerRef.current), stopping all tracks on streamRef.current, and resetting any sharing state (e.g., isSharing) as stopSharing does — inline these steps in the cleanup rather than calling stopSharing to avoid adding render-scope dependencies; reference useEffect cleanup, timerRef, streamRef, wsRef (or the WebSocket variable), and stopSharing to locate the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/contexts/ConnectionProvider.tsx`:
- Around line 173-182: The code directly mutates subscribersRef.current.pong and
adds handlePong, which bypasses the existing subscribe API; change to call the
subscribe function (e.g., subscribe('pong', handlePong)) instead of poking
subscribersRef.current.pong, capture the returned unsubscribe function and call
it in the effect cleanup so listeners are unregistered, and apply the same
change for the duplicate block that currently touches
subscribersRef.current.pong (the one around where handlePong/setLatency is used
and the other block at the later occurrence).
- Around line 126-137: The onclose handler schedules setTimeout(connect, delay)
but never stores or clears the timer; add a reconnectTimerRef (e.g.,
useRef<number | null>) and replace setTimeout(...) with
reconnectTimerRef.current = window.setTimeout(connect, delay), clear any
existing timer before scheduling, and clearTimeout(reconnectTimerRef.current)
and set it to null in the component cleanup (and after a successful connect) so
the pending timer is cancelled on unmount; references: socket.onclose,
reconnectCountRef, isMountedRef, connect, and the component cleanup function.
- Around line 38-48: The subscribe and send functions (and the inlined context
value containing wsRef, status, platform, latency, send, subscribe) must be
stabilized to avoid unnecessary consumer re-renders if React Compiler bails;
wrap subscribe and send in useCallback with correct dependency arrays
(referencing subscribersRef, wsRef, etc.) and memoize the context value with
useMemo (including wsRef, status, platform, latency, send, subscribe) inside
ConnectionProvider (and similarly for the other affected blocks), then run the
build and verify React Compiler reports no bailouts for this file to confirm the
change succeeded.
In `@src/hooks/useMirrorStream.ts`:
- Around line 38-41: The effect sends "start-mirror" unconditionally which can
throw if the socket is already CLOSING/CLOSED; change the setup to mirror the
teardown guard by checking ws.readyState === WebSocket.OPEN before calling
ws.send("start-mirror") (and similarly guard the other ws.send call around lines
referencing the second send at 69-73). Locate the useMirrorStream effect where
ws, status, setHasFrame are used and wrap both start-mirror sends with the same
readyState check used for stop-mirror so listeners are only wired when the
socket is actually OPEN.
- Line 90: In useMirrorStream, remove wsRef and canvasRef from the useEffect
dependency array and leave only status (or other real reactive values) because
wsRef and canvasRef are stable RefObjects; update the dependency list in the
effect attached to useMirrorStream so it reads only the necessary reactive
dependencies (e.g., [status]) to avoid unnecessary re-runs while keeping effect
logic in the same function.
---
Outside diff comments:
In `@src/hooks/useCaptureProvider.ts`:
- Around line 118-125: The cleanup in the useEffect currently clears timerRef
and stops streamRef tracks but omits the WebSocket notification and shared state
teardown performed by stopSharing; update the cleanup to mirror stopSharing by
also sending the "stop-mirror" message over the WebSocket (using wsRef.current
if present), clearing the interval (timerRef.current), stopping all tracks on
streamRef.current, and resetting any sharing state (e.g., isSharing) as
stopSharing does — inline these steps in the cleanup rather than calling
stopSharing to avoid adding render-scope dependencies; reference useEffect
cleanup, timerRef, streamRef, wsRef (or the WebSocket variable), and stopSharing
to locate the logic.
🪄 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: fb98d7b3-0d6a-4f83-a068-344686987453
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
biome.jsonsrc/contexts/ConnectionProvider.tsxsrc/hooks/useCaptureProvider.tssrc/hooks/useMirrorStream.tsvite.config.ts
Comprehensive update to support React Compiler and Vite 8: - Integrated React Compiler into the build pipeline via @vitejs/plugin-react. - Upgraded to Vite 8 and updated related dependencies. - Refactored ConnectionProvider and hooks (useCaptureProvider, useMirrorStream) to remove manual memoization (useCallback/useMemo), leveraging automatic compiler optimizations. - Improved ConnectionProvider with robust reconnection timer management to prevent memory leaks and redundant connection attempts. - Updated Biome configuration for better compatibility and schema synchronization.
Arbaaz123676
force-pushed
the
feat/compiler-upgrade-v2
branch
from
d3e54c8 to
41f391f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addressed Issues:
Fixes #327
Description
This PR focuses strictly on upgrading and stabilizing the Vite 8 migration while ensuring existing functionality remains unaffected.
Changes made:
babel-plugin-react-compileras anyworkaround previously used for Babel configuseCallbackusage for React Compiler compatibilityVerification performed
npm run devnpm run buildnpm run checkThe application runs successfully after the migration and configuration updates.
Screenshots/Recordings:
N/A
Functional Verification
Screen Mirror
Authentication
Basic Gestures
Modes & Settings
Advanced Input
Checklist
Summary by CodeRabbit
Chores
Build/Configuration
UI
Refactor