Skip to content

feat: Created packages/realtime-types/** (@repo/realtime-types):#9

Merged
BuckyMcYolo merged 6 commits intomainfrom
dev
Feb 24, 2026
Merged

feat: Created packages/realtime-types/** (@repo/realtime-types):#9
BuckyMcYolo merged 6 commits intomainfrom
dev

Conversation

@BuckyMcYolo
Copy link
Copy Markdown
Owner

@BuckyMcYolo BuckyMcYolo commented Feb 23, 2026

Overview

This PR extracts realtime type definitions and room helpers from the apps/realtime application into a new shared package, @repo/realtime-types, and updates consumers across the monorepo to use it. It also adds client-side realtime integration in the web app (socket client, context, adapters, optimistic messaging), UI loading skeletons, a theme switcher tweak, and a NEXT_PUBLIC_REALTIME_URL env var.

Changes

New Package: packages/realtime-types (@repo/realtime-types)

  • Added package.json with exports:
    • "." -> "./src/index.ts"
    • "./events" -> "./src/events.ts"
    • "./rooms" -> "./src/rooms.ts"
  • Added check-types script: "tsc --noEmit"
  • Declares runtime dependency: "zod": "^4.3.6"
  • Re-exports events and rooms from src/index.ts
  • Provides event schemas/types and room helper functions used by server and client code.

apps/realtime updates

  • package.json: added workspace dependency "@repo/realtime-types": "workspace:*"
  • Updated imports to consume types and room helpers from "@repo/realtime-types" (no runtime logic changes).

apps/web updates (real-time client integration + UI)

  • package.json: added "@repo/realtime-types": "workspace:*" and "socket.io-client": "^4.8.3"
  • Environment & config:
    • .env.example: added NEXT_PUBLIC_REALTIME_URL
    • packages/env/src/client.ts: added NEXT_PUBLIC_REALTIME_URL to validated client env (addProtocol updated to preserve ws/wss)
    • apps/web/vite.config.ts: exposes NEXT_PUBLIC_REALTIME_URL via define
  • Socket client & context:
    • apps/web/src/lib/socket.ts: AppSocket type and getSocket() singleton (configured with autoConnect: false, withCredentials: true, transports: ["websocket"])
    • apps/web/src/context/socket-context.tsx: SocketProvider and useSocket hook to manage socket lifecycle when enabled
    • apps/web/src/routes/_authenticated.tsx: wrapped layout with
  • Real-time message handling:
    • Channel and DM routes updated to join/leave rooms, listen for message:created events, and integrate optimistic send flow with nonce tracking and reconciliation (including replacement of optimistic messages on server confirmation and removal on failure)
    • apps/web/src/lib/realtime-adapter.ts: realtimeMessageToMessage and createOptimisticMessage helpers
    • MessageList and ChatSkeleton: improved loading skeletons and scroll handling; MessageList becomes a scrollable container with auto-scroll behavior when near bottom
    • apps/web/src/context/socket-context.tsx: lifecycle listeners for connect/disconnect/connect_error with proper cleanup on unmount
  • UI tweak:
    • Replaced ToggleGroup theme control with a compact ThemeSwitcher in user-bar.tsx

Public API / Exports

  • New package @repo/realtime-types exports event schemas/types and room helpers via package exports and is consumed by realtime server and web client.

Risk & Review Notes

  • Primary changes: new shared types package and added runtime surface in the web app.
  • Key areas to verify:
    • socket.io-client/server compatibility and typings alignment (server emits and client listeners should match).
    • Workspace package resolution (pnpm/yarn) and TypeScript cross-package build (run repo-wide tsc --noEmit and packages/realtime-types check-types).
    • Environment variable propagation (NEXT_PUBLIC_REALTIME_URL) in dev, build, and deployment; ensure correct ws/wss handling.
    • Correctness of optimistic flow and nonce reconciliation to avoid duplicate or lost messages.
    • SocketProvider lifecycle: ensure listeners are removed and sockets disconnected to prevent leaks or duplicate subscriptions.
    • Ensure package exports paths (./events, ./rooms) are correctly resolved by consumers.

Recommended checks: repo-wide typecheck, run the new package check-types script, smoke test real-time flows (join/leave, send/ack/failure), and validate socket behavior across reconnects.

Confidence Score: 3/5

Mostly solid refactor and feature addition, but the new runtime behavior (socket client, optimistic messaging) and workspace/package resolution introduce integration risk. Verify compatibility and end-to-end realtime flows before merging.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 23, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a new workspace package @repo/realtime-types, switches several imports to it, and integrates realtime support into the web app (typed socket client, provider, adapters, optimistic messaging, scroll-aware message list, and skeletons). Also adds NEXT_PUBLIC_REALTIME_URL and replaces a theme ToggleGroup with ThemeSwitcher.

Changes

Cohort / File(s) Summary
New Realtime Types Package
packages/realtime-types/package.json, packages/realtime-types/src/index.ts, packages/realtime-types/tsconfig.json
New workspace package exporting ./events and ./rooms, adds check-types script, runtime zod dependency, and package TS config; re-exports events/rooms.
Realtime app imports
apps/realtime/package.json, apps/realtime/src/index.ts, apps/realtime/src/services/messages.ts, apps/realtime/src/services/notifications.ts, apps/realtime/src/services/read-states.ts
Added dependency on @repo/realtime-types and updated several type import paths to that package only.
Web — socket client & provider
apps/web/src/lib/socket.ts, apps/web/src/context/socket-context.tsx
New typed socket singleton (getSocket) and SocketProvider + useSocket hook managing connection lifecycle and listeners.
Web — route integrations (channels & DMs)
apps/web/src/routes/_authenticated.tsx, apps/web/src/routes/_authenticated/$guildSlug/$channelId.tsx, apps/web/src/routes/_authenticated/dms/$dmId.tsx
Wraps authenticated layout with SocketProvider; adds join/leave room, message:created handlers, optimistic send with nonce reconciliation, and message fetch wiring.
Web — adapters, optimistic messages & UI
apps/web/src/lib/realtime-adapter.ts, apps/web/src/components/chat/message-list.tsx, apps/web/src/components/chat/chat-skeleton.tsx, apps/web/src/components/sidebar/channel-panel/user-bar.tsx
Adds conversion utilities and optimistic message creator; message list gains scroll handling and skeletons; ChatSkeleton added; ThemeSwitcher replaces previous ToggleGroup in UserBar.
Web — deps, env & config
apps/web/package.json, apps/web/vite.config.ts, .env.example, packages/env/src/client.ts
Adds socket.io-client and workspace realtime types dep; exposes NEXT_PUBLIC_REALTIME_URL in env schema, Vite define, and example; URL normalization updated to support ws/wss.
Misc — small import/path updates
apps/realtime/..., apps/web/...
Multiple files updated to import shared realtime types from @repo/realtime-types rather than local paths (no runtime behavior changes beyond type locations).

Sequence Diagram(s)

sequenceDiagram
participant UI as Client UI
participant Provider as SocketProvider
participant Socket as AppSocket
participant Server as Realtime Server

UI->>Provider: onSend(content)
Provider->>Socket: emit "message:send" {nonce, channelId, content}
Note right of UI: UI inserts optimistic message (nonce)
Socket-->>Server: "message:send" payload
Server-->>Socket: "message:created" {id, nonce?, content, author, channelId, createdAt}
Socket->>Provider: receive "message:created"
alt nonce matches optimistic
Provider->>UI: replace optimistic message (nonce) with server message (id)
else new message from others
Provider->>UI: prepend new message to list
end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • feat: added simple SocketIO server in apps/realtime #8 — adds/extracts @repo/realtime-types and its exported events/rooms types (directly related to import changes).
  • Dev #7 — modifies authenticated layout and onboarding/socket provider integration (overlaps _authenticated.tsx changes).
  • Dev #6 — touches DM route real-time UI and message handling (overlaps DM route changes).

Poem

🐰 I hopped through types and sockets bright,
I stitched a nonce and sent it light,
A skeleton waits, the chat takes flight,
The theme now switches soft and right,
Hopping home — the warren’s alight.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title focuses narrowly on creating the realtime-types package, but the PR also includes significant real-time socket integration across the web app (socket context, message synchronization, optimistic updates, etc.), making the title incomplete. Expand the title to reflect the broader scope, e.g., 'feat: Add real-time messaging with socket integration and realtime-types package' or similar.
Docstring Coverage ⚠️ Warning Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dev

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/web/src/components/sidebar/channel-panel/user-bar.tsx`:
- Around line 43-48: The theme toggle buttons in user-bar.tsx need an
aria-pressed attribute so screen readers can know which theme is active; update
the <button> that uses setTheme, value and label to include aria-pressed whose
boolean value is true when the current theme state (e.g., theme or currentTheme
in this component) equals value and false otherwise, ensuring the attribute is
present on each theme button to convey the active state to assistive
technologies.
- Around line 50-59: The motion.div using layoutId="theme-switcher-pill" in
user-bar.tsx is globally scoped and can conflict if rendered in multiple places;
wrap the component subtree that contains that motion.div and the
ThemeSwitcher/buttons in a framer-motion LayoutGroup (import LayoutGroup from
'framer-motion') so the layoutId is scoped per instance; ensure the LayoutGroup
encloses the conditional that renders motion.div (and any sibling elements that
should participate in the scoped animation) and adjust imports if needed.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7f307fc and d98e76b.

📒 Files selected for processing (1)
  • apps/web/src/components/sidebar/channel-panel/user-bar.tsx

Comment thread apps/web/src/components/sidebar/channel-panel/user-bar.tsx Outdated
Comment thread apps/web/src/components/sidebar/channel-panel/user-bar.tsx Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/env/src/client.ts (1)

20-30: ⚠️ Potential issue | 🟡 Minor

Guard against ws/wss URL values to avoid invalid protocol prefixes.
If someone sets NEXT_PUBLIC_REALTIME_URL to ws:// or wss://, addProtocol will prepend https://, yielding an invalid URL. Consider accepting ws/wss or validating and failing fast with a clear error.

🛠️ Suggested fix (extend protocol handling)
-const addProtocol = (url: string) => {
+const addProtocol = (url: string) => {
   const trimmed = url.trim()
-  if (trimmed.startsWith("http://") || trimmed.startsWith("https://"))
+  if (
+    trimmed.startsWith("http://") ||
+    trimmed.startsWith("https://") ||
+    trimmed.startsWith("ws://") ||
+    trimmed.startsWith("wss://")
+  )
     return trimmed
   const isLocal =
     trimmed.startsWith("localhost") || trimmed.startsWith("127.0.0.1")
   return isLocal ? `http://${trimmed}` : `https://${trimmed}`
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/env/src/client.ts` around lines 20 - 30, The
NEXT_PUBLIC_REALTIME_URL transform (addProtocol) can wrongly prepend https:// to
ws:// or wss:// values; update the handling so websocket schemes are preserved
or validated: modify addProtocol to detect and return values starting with ws://
or wss:// unchanged (or throw a clear ZodError for invalid schemes), and change
the clientSchema transform usage for NEXT_PUBLIC_REALTIME_URL to use this
updated helper (or a new addProtocolForRealtime) so realtime URLs accept ws/wss
or fail fast with a descriptive error referencing NEXT_PUBLIC_REALTIME_URL,
addProtocol, and clientSchema.
♻️ Duplicate comments (2)
apps/web/src/components/sidebar/channel-panel/user-bar.tsx (2)

50-59: layoutId="theme-switcher-pill" remains globally scoped — wrap in LayoutGroup.

If ThemeSwitcher (or UserBar) is ever mounted in more than one place simultaneously, the two instances will fight over the same shared layout animation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/components/sidebar/channel-panel/user-bar.tsx` around lines 50 -
59, The motion.div using layoutId="theme-switcher-pill" is globally scoped and
can clash if UserBar/ThemeSwitcher mounts multiple times; wrap the animated node
(or the parent component, e.g., UserBar or ThemeSwitcher) in a framer-motion
LayoutGroup and import LayoutGroup from "framer-motion" to scope the layoutId,
or alternatively make the layoutId unique per instance (pass a prop like
layoutIdSuffix and append it to "theme-switcher-pill") so the motion.div's
layoutId is not shared globally; update the component to use LayoutGroup around
the block containing motion.div (or accept and append a unique suffix) and
ensure the import and usage of LayoutGroup are added where layout animations are
defined.

43-48: aria-pressed is still missing from theme buttons.

Screen readers cannot determine which theme is currently active without aria-pressed.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/components/sidebar/channel-panel/user-bar.tsx` around lines 43 -
48, The theme toggle buttons lack an aria-pressed attribute so screen readers
can't tell which is active; update the button rendered in user-bar.tsx (the
element using key={value} and onClick={() => setTheme(value)}) to include
aria-pressed set to a boolean expression comparing the button's value to the
current theme (e.g., aria-pressed={value === theme}), ensuring the attribute
reflects the active state.
🤖 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/web/src/components/sidebar/channel-panel/user-bar.tsx`:
- Line 12: Remove the dead commented import and related commented usage: delete
the commented-out import line for useNavigate and remove the commented navigate
call inside handleOpenSettings (or keep only active code paths); specifically
clean up the unused commented import for useNavigate and the commented
navigate(...) reference in the handleOpenSettings function so the file contains
no commented-out routing code until settings routing is implemented.

In `@apps/web/src/context/socket-context.tsx`:
- Around line 20-45: The effect currently registers anonymous listeners on the
singleton socket from getSocket() and calls s.off("event") which removes all
listeners; instead capture each handler in a stable variable (e.g., const
onConnect = () => ..., onDisconnect = reason => ..., onConnectError = err =>
...) before calling s.on(...), and in the cleanup call s.off("connect",
onConnect), s.off("disconnect", onDisconnect), s.off("connect_error",
onConnectError); keep the same calls to s.connect(), s.disconnect(),
setSocket(s) and setSocket(null) but replace anonymous callbacks with referenced
handler functions so other listeners on the singleton socket are not removed.

In `@apps/web/src/routes/_authenticated/`$guildSlug/$channelId.tsx:
- Around line 57-66: The useEffect that joins/leaves the channel uses
socket.emit but only depends on socket?.connected and channelId; update the
dependency array to include the actual socket reference so the effect re-runs
when the socket instance changes. Locate the useEffect with
socket.emit("channel:join", { channelId }) / socket.emit("channel:leave", {
channelId }) and change its dependencies from [socket?.connected, channelId] to
[socket, channelId] (matching the other effect’s pattern) to avoid stale
closures.

---

Outside diff comments:
In `@packages/env/src/client.ts`:
- Around line 20-30: The NEXT_PUBLIC_REALTIME_URL transform (addProtocol) can
wrongly prepend https:// to ws:// or wss:// values; update the handling so
websocket schemes are preserved or validated: modify addProtocol to detect and
return values starting with ws:// or wss:// unchanged (or throw a clear ZodError
for invalid schemes), and change the clientSchema transform usage for
NEXT_PUBLIC_REALTIME_URL to use this updated helper (or a new
addProtocolForRealtime) so realtime URLs accept ws/wss or fail fast with a
descriptive error referencing NEXT_PUBLIC_REALTIME_URL, addProtocol, and
clientSchema.

---

Duplicate comments:
In `@apps/web/src/components/sidebar/channel-panel/user-bar.tsx`:
- Around line 50-59: The motion.div using layoutId="theme-switcher-pill" is
globally scoped and can clash if UserBar/ThemeSwitcher mounts multiple times;
wrap the animated node (or the parent component, e.g., UserBar or ThemeSwitcher)
in a framer-motion LayoutGroup and import LayoutGroup from "framer-motion" to
scope the layoutId, or alternatively make the layoutId unique per instance (pass
a prop like layoutIdSuffix and append it to "theme-switcher-pill") so the
motion.div's layoutId is not shared globally; update the component to use
LayoutGroup around the block containing motion.div (or accept and append a
unique suffix) and ensure the import and usage of LayoutGroup are added where
layout animations are defined.
- Around line 43-48: The theme toggle buttons lack an aria-pressed attribute so
screen readers can't tell which is active; update the button rendered in
user-bar.tsx (the element using key={value} and onClick={() => setTheme(value)})
to include aria-pressed set to a boolean expression comparing the button's value
to the current theme (e.g., aria-pressed={value === theme}), ensuring the
attribute reflects the active state.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d98e76b and 6723b7d.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (13)
  • .env.example
  • apps/web/package.json
  • apps/web/src/components/chat/chat-skeleton.tsx
  • apps/web/src/components/chat/message-list.tsx
  • apps/web/src/components/sidebar/channel-panel/user-bar.tsx
  • apps/web/src/context/socket-context.tsx
  • apps/web/src/lib/realtime-adapter.ts
  • apps/web/src/lib/socket.ts
  • apps/web/src/routes/_authenticated.tsx
  • apps/web/src/routes/_authenticated/$guildSlug/$channelId.tsx
  • apps/web/src/routes/_authenticated/dms/$dmId.tsx
  • apps/web/vite.config.ts
  • packages/env/src/client.ts

Comment thread apps/web/src/components/sidebar/channel-panel/user-bar.tsx Outdated
Comment thread apps/web/src/context/socket-context.tsx
Comment thread apps/web/src/routes/_authenticated/$guildSlug/$channelId.tsx Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
apps/web/src/routes/_authenticated/dms/$dmId.tsx (1)

21-161: 🛠️ Refactor suggestion | 🟠 Major

Significant duplication with $guildSlug/$channelId.tsx — extract a shared hook.

The real-time messaging logic (join/leave effect, message:created listener with nonce-based reconciliation, handleSend with optimistic updates) is nearly identical between this DM component and the channel component. The only differences are the query key prefix and the fetch endpoint.

Consider extracting a useRealtimeMessages(channelId, queryKey, fetchFn) hook (or similar) to centralize the nonce tracking, join/leave lifecycle, event listener setup, and optimistic send logic. This avoids maintaining the same complex reconciliation logic in two places.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/routes/_authenticated/dms/`$dmId.tsx around lines 21 - 161, The
DM component duplicates complex real-time messaging logic (join/leave,
"message:created" listener, nonce reconciliation, optimistic send) found in the
channel component; extract that into a reusable hook (e.g.,
useRealtimeMessages(channelId, queryKey, fetchFn)) and replace DMConversation's
logic with the hook. Move pendingNonces, join/leave socket effect (useSocket +
socket.emit "channel:join"/"channel:leave"), the socket.on/off "message:created"
handler (which uses realtimeMessageToMessage), and the optimistic send
implementation (handleSend: crypto.randomUUID nonce, createOptimisticMessage,
queryClient.setQueryData updates, socket.emit "message:send" callback) into the
hook, have it return messagesData, messagesLoading/isPending, and a sendMessage
function; update DMConversation to call useRealtimeMessages(dmId, ["messages",
dmId], () => apiClient.v1.dms[":dmId"].messages.$get({ param: { dmId } })) and
remove duplicate logic (references: DMConversation, handleSend, pendingNonces,
realtimeMessageToMessage, createOptimisticMessage, queryClient, useSocket,
useQuery).
apps/web/src/components/sidebar/channel-panel/user-bar.tsx (1)

136-142: ⚠️ Potential issue | 🟠 Major

Accessibility blocker: nested interactive controls inside role="menuitem".

DropdownMenuItem renders as div[role="menuitem"]. The three <button> elements inside ThemeSwitcher create a nested-interactive structure that violates the ARIA spec and the axe nested-interactive rule: "ensure that no focusable child elements are present in any interactive controls." A Radix dropdown manages focus exclusively via arrow-key navigation between menuitem nodes; the inner buttons are not reachable through that model, so keyboard-only users and screen-reader users can't operate the theme switcher.

The fix is to swap the DropdownMenuItem wrapper for a non-interactive container so the buttons are direct focusable descendants of the menu content:

♿ Proposed fix
-            <DropdownMenuItem
-              className="cursor-default"
-              onSelect={(e) => e.preventDefault()}
-            >
-              Theme
-              <ThemeSwitcher theme={theme} setTheme={setTheme} />
-            </DropdownMenuItem>
+            <div className="flex items-center px-2 py-1.5 text-sm select-none">
+              Theme
+              <ThemeSwitcher theme={theme} setTheme={setTheme} />
+            </div>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/components/sidebar/channel-panel/user-bar.tsx` around lines 136
- 142, DropdownMenuItem (renders as div[role="menuitem"]) contains focusable
buttons from ThemeSwitcher which creates a nested interactive control and breaks
ARIA/axe rules; replace the DropdownMenuItem wrapper with a non-interactive
container so the three buttons are direct focusable children of the menu
content. Locate the DropdownMenuItem usage in user-bar.tsx (the node wrapping
ThemeSwitcher) and change it to a plain non-interactive element (for example a
simple div/section with no role or with role="none" and same classes) and remove
the onSelect handler so ThemeSwitcher’s buttons remain accessible and reachable
by keyboard/screen readers while preserving layout and styling; keep
ThemeSwitcher(theme, setTheme) props unchanged.
♻️ Duplicate comments (1)
apps/web/src/routes/_authenticated/$guildSlug/$channelId.tsx (1)

57-66: Previous feedback about socket in the dependency array has been addressed.

The dependency array now correctly includes [socket, channelId] instead of the previous [socket?.connected, channelId].

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/routes/_authenticated/`$guildSlug/$channelId.tsx around lines 57
- 66, The dependency array for the useEffect that joins/leaves the socket room
was previously wrong; keep it as [socket, channelId] and ensure the effect uses
socket.emit("channel:join", { channelId }) on mount and
socket.emit("channel:leave", { channelId }) on cleanup in the same useEffect
(function name/useEffect hook and symbols: useEffect, socket, channelId,
"channel:join", "channel:leave"); no further changes required since the earlier
[socket?.connected, channelId] issue has been fixed.
🤖 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/web/src/routes/_authenticated/`$guildSlug/$channelId.tsx:
- Around line 57-66: The effect that joins/leaves the channel room currently
guards on socket?.connected which can skip emitting if the socket reference
exists but isn’t connected yet; change the guard in the useEffect for channel
join/leave to only check for socket presence (if (!socket) return) and remove
the socket?.connected check so socket.emit("channel:join", { channelId }) and
the cleanup socket.emit("channel:leave", { channelId }) run and rely on
socket.io buffering (or alternatively expose an explicit isConnected from the
provider if you prefer). Ensure you update the useEffect that references socket,
channelId and the "channel:join"/"channel:leave" emits accordingly.

In `@apps/web/src/routes/_authenticated/dms/`$dmId.tsx:
- Around line 51-59: The effect in the DM route currently guards on
socket?.connected which can prevent "channel:join" from ever firing when the
SocketProvider sets the socket before it is connected; remove the connected
guard so the useEffect emits "channel:join" and "channel:leave" whenever socket
is non-null (i.e. keep the effect that depends on socket and dmId but call
socket.emit unconditionally when socket exists), relying on socket.io's built-in
buffering, or alternatively update SocketProvider to expose an isConnected flag
via onConnect/onDisconnect and use that if you prefer the connection-aware
approach; reference the useEffect in
apps/web/src/routes/_authenticated/dms/$dmId.tsx and the SocketProvider's
s.connect()/setSocket(s).

In `@packages/env/src/client.ts`:
- Line 20: clientSchema currently requires NEXT_PUBLIC_REALTIME_URL at module
import (NEXT_PUBLIC_REALTIME_URL: z.string().min(1).transform(addProtocol)),
which causes clientSchema.parse() to throw when the env var is missing; change
the schema to make NEXT_PUBLIC_REALTIME_URL optional or provide a sensible
default (e.g., .optional() with .transform(addProtocol) or
.default("http://localhost:...") depending on desired behavior) and update any
call sites that assume its presence to guard usage (check for undefined before
using, e.g., in functions referencing NEXT_PUBLIC_REALTIME_URL or in the client
initialization code); also apply the same change where the field appears again
(the other occurrence mentioned in the comment).

---

Outside diff comments:
In `@apps/web/src/components/sidebar/channel-panel/user-bar.tsx`:
- Around line 136-142: DropdownMenuItem (renders as div[role="menuitem"])
contains focusable buttons from ThemeSwitcher which creates a nested interactive
control and breaks ARIA/axe rules; replace the DropdownMenuItem wrapper with a
non-interactive container so the three buttons are direct focusable children of
the menu content. Locate the DropdownMenuItem usage in user-bar.tsx (the node
wrapping ThemeSwitcher) and change it to a plain non-interactive element (for
example a simple div/section with no role or with role="none" and same classes)
and remove the onSelect handler so ThemeSwitcher’s buttons remain accessible and
reachable by keyboard/screen readers while preserving layout and styling; keep
ThemeSwitcher(theme, setTheme) props unchanged.

In `@apps/web/src/routes/_authenticated/dms/`$dmId.tsx:
- Around line 21-161: The DM component duplicates complex real-time messaging
logic (join/leave, "message:created" listener, nonce reconciliation, optimistic
send) found in the channel component; extract that into a reusable hook (e.g.,
useRealtimeMessages(channelId, queryKey, fetchFn)) and replace DMConversation's
logic with the hook. Move pendingNonces, join/leave socket effect (useSocket +
socket.emit "channel:join"/"channel:leave"), the socket.on/off "message:created"
handler (which uses realtimeMessageToMessage), and the optimistic send
implementation (handleSend: crypto.randomUUID nonce, createOptimisticMessage,
queryClient.setQueryData updates, socket.emit "message:send" callback) into the
hook, have it return messagesData, messagesLoading/isPending, and a sendMessage
function; update DMConversation to call useRealtimeMessages(dmId, ["messages",
dmId], () => apiClient.v1.dms[":dmId"].messages.$get({ param: { dmId } })) and
remove duplicate logic (references: DMConversation, handleSend, pendingNonces,
realtimeMessageToMessage, createOptimisticMessage, queryClient, useSocket,
useQuery).

---

Duplicate comments:
In `@apps/web/src/routes/_authenticated/`$guildSlug/$channelId.tsx:
- Around line 57-66: The dependency array for the useEffect that joins/leaves
the socket room was previously wrong; keep it as [socket, channelId] and ensure
the effect uses socket.emit("channel:join", { channelId }) on mount and
socket.emit("channel:leave", { channelId }) on cleanup in the same useEffect
(function name/useEffect hook and symbols: useEffect, socket, channelId,
"channel:join", "channel:leave"); no further changes required since the earlier
[socket?.connected, channelId] issue has been fixed.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6723b7d and 0aaef33.

📒 Files selected for processing (5)
  • apps/web/src/components/sidebar/channel-panel/user-bar.tsx
  • apps/web/src/context/socket-context.tsx
  • apps/web/src/routes/_authenticated/$guildSlug/$channelId.tsx
  • apps/web/src/routes/_authenticated/dms/$dmId.tsx
  • packages/env/src/client.ts

Comment thread apps/web/src/routes/_authenticated/$guildSlug/$channelId.tsx
Comment thread apps/web/src/routes/_authenticated/dms/$dmId.tsx
Comment thread packages/env/src/client.ts
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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/web/src/routes/_authenticated/`$guildSlug/$channelId.tsx:
- Around line 107-169: handleSend currently returns early when socket.connected
or session is missing, but MessageInput still clears the user's text; change
handleSend to return a boolean (or Promise<boolean>) indicating success (true
only after the socket emit callback confirms send succeeded) and update
MessageInput to call onSend and only clear the input when it returns true;
specifically modify the handleSend function to return false immediately when
bailing, return true after the optimistic insert and successful server
confirmation (or true after optimistic insert if you prefer immediate clear) and
ensure MessageInput awaits onSend and only clears the input on a truthy result,
referencing handleSend, MessageInput, socket, session, and the onSend prop to
locate the changes.
- Around line 42-55: The messages useQuery currently swallows failures (leaving
messages cache undefined) so realtime inserts and the UI show empty/silent
states; update the useQuery call for queryKey ["messages", channelId] (the
queryFn that returns messagesData) to surface errors (include isError/error in
the destructure) and add an onError handler that initializes the cache to an
empty array via the queryClient (setQueryData(["messages", channelId], [])) so
the UI can render a controlled empty state; additionally, in the realtime insert
logic check queryClient.getQueryData(["messages", channelId]) and if undefined
initialize it to [] before inserting new messages so realtime updates don't get
dropped.

In `@apps/web/src/routes/_authenticated/dms/`$dmId.tsx:
- Around line 98-160: handleSend currently bails early when socket or session is
missing and returns void, causing MessageInput to clear user text prematurely;
change handleSend to return a boolean or Promise<boolean> and only clear the
input on success. Specifically, update the handleSend function (symbol:
handleSend) to: 1) return false immediately if !socket?.connected ||
!session?.user; 2) wrap socket.emit(..., (result)=>{}) in a Promise and resolve
true on result.ok and false on failure (ensuring pendingNonces manipulation and
queryClient updates remain the same); and 3) update the MessageInput caller to
await the returned boolean and clear its input only when true (or handle false
by leaving text intact). This preserves existing optimistic UI logic
(pendingNonces, createOptimisticMessage, queryClient) but surfaces
success/failure to the input component.
- Around line 37-48: The messages query can fail and leave the cache undefined,
causing the UI to silently show an empty state and skip realtime inserts; update
the useQuery call for messages (the block that defines
messagesData/messagesLoading and calls apiClient.v1.dms[":dmId"].messages.$get)
to handle errors and initialize the cache: either return an empty array from
queryFn on catch or add initialData: [] to the useQuery options, and provide an
onError handler (or set isError via react-query) so the component can render an
error state instead of silently empty — modify the queryFn and options
(initialData/onError) around useQuery to ensure messagesData is never undefined
and errors are surfaced.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0aaef33 and 016698a.

📒 Files selected for processing (2)
  • apps/web/src/routes/_authenticated/$guildSlug/$channelId.tsx
  • apps/web/src/routes/_authenticated/dms/$dmId.tsx

Comment on lines +42 to +55
const { data: messagesData, isPending: messagesLoading } = useQuery({
queryKey: ["messages", channelId],
queryFn: async () => {
const res = await apiClient.v1.guilds[":guildSlug"].channels[
":channelId"
].messages.$get({
param: { guildSlug, channelId },
query: {},
})
if (!res.ok) throw new Error("Failed to fetch messages")
return res.json()
},
enabled: !!data,
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Handle message-query errors to avoid silent empty states.

If the messages request fails, the UI shows an empty list and realtime inserts are skipped because the cache stays undefined. Consider surfacing isError/error or initializing cache on first realtime message.

🛠️ Possible fix
-  const { data: messagesData, isPending: messagesLoading } = useQuery({
+  const {
+    data: messagesData,
+    isPending: messagesLoading,
+    isError: messagesError,
+    error: messagesErrorObj,
+  } = useQuery({
     queryKey: ["messages", channelId],
     queryFn: async () => {
       const res = await apiClient.v1.guilds[":guildSlug"].channels[
         ":channelId"
       ].messages.$get({
         param: { guildSlug, channelId },
         query: {},
       })
       if (!res.ok) throw new Error("Failed to fetch messages")
       return res.json()
     },
     enabled: !!data,
   })
+  if (messagesError) {
+    return (
+      <div className="flex flex-1 items-center justify-center">
+        <span className="text-sm text-muted-foreground">
+          {messagesErrorObj instanceof Error
+            ? messagesErrorObj.message
+            : "Failed to load messages"}
+        </span>
+      </div>
+    )
+  }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/routes/_authenticated/`$guildSlug/$channelId.tsx around lines 42
- 55, The messages useQuery currently swallows failures (leaving messages cache
undefined) so realtime inserts and the UI show empty/silent states; update the
useQuery call for queryKey ["messages", channelId] (the queryFn that returns
messagesData) to surface errors (include isError/error in the destructure) and
add an onError handler that initializes the cache to an empty array via the
queryClient (setQueryData(["messages", channelId], [])) so the UI can render a
controlled empty state; additionally, in the realtime insert logic check
queryClient.getQueryData(["messages", channelId]) and if undefined initialize it
to [] before inserting new messages so realtime updates don't get dropped.

Comment thread apps/web/src/routes/_authenticated/$guildSlug/$channelId.tsx
Comment thread apps/web/src/routes/_authenticated/dms/$dmId.tsx
Comment thread apps/web/src/routes/_authenticated/dms/$dmId.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant