feat/email-waitlist & nicer landing page - #2
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Warning Rate limit exceeded
⌛ 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. ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (6)
WalkthroughThis PR renames the app to "Biviant", redesigns the header and landing page (adds a waitlist signup), introduces a Feed and unsubscribe route, implements a backend waitlist with schema and mutations, and adds Resend-based email sending and documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LandingPage as Landing Page
participant Backend as Backend (waitlist)
participant Database as Database
participant Resend as Resend (Email)
User->>LandingPage: Submit waitlist form (email, name, referral)
LandingPage->>Backend: call addToWaitlist(args)
Backend->>Backend: validate & normalize email
Backend->>Database: query by email
Database-->>Backend: existing / null
alt existing entry
Backend-->>LandingPage: return existing position
else new entry
Backend->>Database: compute position & insert record
Database-->>Backend: return waitlistId
opt RESEND_API_KEY set
Backend->>Resend: send welcome email
Resend-->>Backend: return emailId
Backend->>Database: update lastEmailSentAt
end
Backend-->>LandingPage: return success, position, waitlistId
end
LandingPage->>User: show confirmation/status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 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)
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: 17
🤖 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/header.tsx`:
- Line 16: The <nav> element in the Header component lacks an accessible label;
update the navigation landmark in apps/web/src/components/header.tsx (the nav
with className "flex gap-6") to include an appropriate aria-label such as
aria-label="Primary" (or another descriptive label) so screen readers can
identify the navigation region.
In `@apps/web/src/routes/feed.tsx`:
- Around line 48-67: The topic filter buttons act like toggles but don't expose
their pressed state to assistive tech; update the Button instances (the "All
topics" button and the mapped topic buttons using topics.map) to include an
aria-pressed attribute tied to selectedTopic (e.g., aria-pressed={selectedTopic
=== "all"} for the All button and aria-pressed={selectedTopic === topic._id}
inside the map) so screen readers can announce the active topic; locate the
buttons where setSelectedTopic is used and add the boolean aria-pressed
accordingly.
In `@apps/web/src/routes/index.tsx`:
- Around line 88-124: The hero waitlist form and the CTA form are duplicated;
extract them into a reusable React component (e.g., WaitlistForm) to centralize
logic and markup: create a new WaitlistForm component that renders the two
Inputs, submit Button, message paragraph and accepts props for the controlled
state and handlers (name, email, setName, setEmail, status, message,
handleSubmit) or manages its own state and exposes an onSubmit callback; replace
both inline forms in routes/index.tsx with <WaitlistForm .../> and ensure the
component uses the existing Input and Button symbols and preserves the
disabled/required/status conditional rendering and class names.
- Around line 197-199: The Link currently wraps a Button creating nested
interactive elements (an anchor containing a button) which is invalid; fix by
removing the nesting: either render Button as the link (use Button's composition
prop, e.g., Button asChild and move <Link to="/feed"> inside the Button so the
anchor becomes the interactive element) or replace the Button with Link styled
like the button (apply the Button classes/variant to Link and ensure it has
role/button semantics if needed); update the component where Link and Button are
used so only one interactive element exists (referencing the Link and Button
symbols) and confirm keyboard/focus behavior and accessible attributes remain
correct.
- Around line 90-107: The Input fields for name and email currently rely on
placeholders and lack accessible labels; update the form by adding explicit
accessible labels for the name and email inputs (either visible <label> elements
associated via htmlFor/id or aria-label/aria-labelledby attributes) and ensure
the ids/handlers match the existing state setters (name, setName and email,
setEmail) and disabled logic (status === "loading"); also apply the same change
to the CTA form inputs so screen readers receive persistent labels rather than
only placeholders.
- Around line 30-66: The setTimeout used in handleSubmit can race with
subsequent submissions; store the timer ID in a ref (e.g., timerRef) inside the
LandingPage component, clear the previous timeout at the start of handleSubmit
(clearTimeout(timerRef.current)), assign timerRef.current to the new timeout ID,
and also clear that timeout in a useEffect cleanup on unmount so the previous
timer never resets status/message during a later submission or after unmount.
In `@apps/web/src/routes/unsubscribe.tsx`:
- Around line 7-9: RouteComponent currently renders a static placeholder and
must perform the unsubscribe flow: read the unsubscribe token or email from the
URL (e.g., via useSearchParams or location), call the backend unsubscribe
endpoint (e.g., POST/PUT to /api/unsubscribe) inside useEffect in
RouteComponent, and manage loading/success/error state with useState to render
accessible feedback (loading spinner, success message, error message) using
aria-live regions; also handle missing/invalid tokens by showing an error and
not calling the API. Ensure the request is debounced/guarded so it runs once,
surface server error details in the UI logs for debugging, and keep markup
semantic and performant for web accessibility.
In `@packages/backend/convex/emails.ts`:
- Around line 22-23: The unsubscribe URL currently embeds a raw email
(constructed as unsubUrl =
`${UNSUB_BASE}?email=${encodeURIComponent(args.email)}`), which is tamperable;
replace this with a signed/tokenized identifier: generate a short HMAC/JWT token
(e.g., createUnsubscribeToken(userId or email) using a server-side secret) and
build the URL with that token instead (e.g., ?t=token) and update the
unsubscribe handler to validate the token and resolve the intended user before
performing unsubscribe; reference UNSUB_BASE and args.email (and add functions
like createUnsubscribeToken / verifyUnsubscribeToken) so no raw emails are used
in the URL and ensure server-side verification enforces auth/authorization.
- Around line 43-47: Replace the raw payload/error console logs in the email
send try/catch blocks (the lines that do console.log("Welcome email sent:",
data) and console.error("Error sending welcome email:", error) and the analogous
logs at 86-90) so they never emit PII: log only stable identifiers and status
(e.g. data?.id, data?.status) and, on failure, log error?.message or a redacted
summary instead of the full error object; implement or call a small helper like
redactEmailPayload/redactError to strip addresses/headers before logging.
- Line 5: Add a startup guard that throws a clear error if RESEND_API_KEY is
missing before creating the Resend client: check process.env.RESEND_API_KEY at
module initialization and if falsy throw a new Error with a descriptive message,
then instantiate the Resend client into the existing resend constant (the new
Resend(...) expression) only after the guard passes so failures occur fast and
clearly; update any related exports in emails.ts accordingly.
In `@packages/backend/convex/README-EMAILS.md`:
- Around line 141-159: The scheduling delay calculation uses the loop index
variable i (0, 50, 100...) directly in ctx.scheduler.runAfter, which results in
non-obvious delays of 0s, 50s, 100s; change to compute an explicit batch counter
(e.g., batchIndex = Math.floor(i / 50)) and use batchIndex * 1000 when calling
ctx.scheduler.runAfter so each batch is delayed by 0s, 1s, 2s, ...; update the
loop around users.slice and the ctx.scheduler.runAfter calls to use that
batchIndex for clarity (referencing the variables i, batch, and the method
ctx.scheduler.runAfter).
- Around line 96-117: The example may dereference a null result from
ctx.db.query("waitlist").withIndex("by_status").filter(...).first(); update the
snippet to check the return of .first() (e.g., const user = ...; if (!user) { /*
handle no pending user: return/throw/log */ }) before accessing user._id,
user.email, or user.name, and only call ctx.db.patch(...) and
ctx.scheduler.runAfter(...) inside the non-null branch so inviteCode is
generated and used only when user is defined.
In `@packages/backend/convex/schema.ts`:
- Around line 186-205: The schema currently has two fields representing the same
state: the union field status (with "unsubscribed") and the boolean
unsubscribed; remove the boolean unsubscribed from the record schema and rely
solely on status for unsubscribe state (update the schema definition where
unsubscribed: v.boolean() is declared and ensure default/creation code sets
status to "unsubscribed" when needed). Also search for and update any code that
reads/writes the unsubscribed boolean (e.g., join/create/update logic and any
query predicates) to use status comparisons (status === "unsubscribed") and add
a migration or transformation plan to convert existing boolean values to the
status union for existing records.
In `@packages/backend/convex/waitlist.ts`:
- Around line 61-64: The patch currently sets lastEmailSentAt optimistically via
ctx.db.patch(waitlistId, { lastEmailSentAt: Date.now() }) before the scheduled
send runs; instead, remove that premature patch and update lastEmailSentAt only
after a successful send inside the scheduled action (the sendWelcomeEmail
handler invoked by ctx.scheduler.runAfter). Modify the sendWelcomeEmail action
to accept the waitlistId, perform the email send, and on success call
ctx.db.patch(waitlistId, { lastEmailSentAt: Date.now() }); ensure any failures
do not update lastEmailSentAt and propagate/log errors appropriately.
- Around line 5-10: The addToWaitlist mutation is public and lacks rate
limiting/authentication; update the mutation (addToWaitlist) to enforce basic
protections: verify the caller (session/user) or require a valid auth token if
available, deduplicate by checking the waitlist table for an existing email
before insert, and apply a rate-limit check (per-IP or per-session) using your
Convex rate-limiting pattern or a simple counter with timestamps in a separate
store to reject excessive requests; ensure the mutation returns a clear error
for rejected/rate-limited calls and keep the insertion logic only after these
checks pass.
- Around line 100-114: getWaitlistStats currently exposes internal waitlist
metrics without any auth; update the handler in getWaitlistStats to enforce
authorization before calling ctx.db.query("waitlist").collect(): check ctx.auth
(or ctx.auth.userId) and verify the caller is an admin (either via a
ctx.auth.role === "admin" check or by loading the user record from the DB and
checking an isAdmin/is_staff flag); if the check fails, throw/return an
unauthorized error and do not return counts. Ensure the auth check runs at the
top of the handler so only authorized admin requests can reach the ctx.db query
logic.
- Around line 34-40: The current logic in the waitlist position calculation
loads all rows via ctx.db.query("waitlist").collect() to compute max position;
replace this with a single indexed query using the by_position index in
descending order to fetch only the highest-position row (e.g., use the query
that sorts by "by_position" desc and returns the first result) and derive
position = highest.position + 1 (or 1 if none); update the code where position
is computed (the block using allWaitlist, maxPosition, and position) to use that
indexed/descending single-row lookup instead to avoid loading all waitlist
records.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/web/src/routeTree.gen.tsis excluded by!**/routeTree.gen.tspnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/pnpm-lock.yaml
📒 Files selected for processing (11)
README.mdapps/web/src/components/header.tsxapps/web/src/routes/__root.tsxapps/web/src/routes/feed.tsxapps/web/src/routes/index.tsxapps/web/src/routes/unsubscribe.tsxpackages/backend/convex/README-EMAILS.mdpackages/backend/convex/emails.tspackages/backend/convex/schema.tspackages/backend/convex/waitlist.tspackages/backend/package.json
Comment on lines
+5
to
+10
| export const addToWaitlist = mutation({ | ||
| args: { | ||
| email: v.string(), | ||
| name: v.optional(v.string()), | ||
| referralSource: v.optional(v.string()), | ||
| }, |
Contributor
There was a problem hiding this comment.
addToWaitlist is a public mutation with no rate limiting or auth.
Any unauthenticated client can call this mutation repeatedly, potentially flooding the waitlist table and triggering unbounded welcome emails. Consider adding basic protections — e.g., checking a rate limit per IP/session, or leveraging Convex's built-in rate-limiting patterns.
As per coding guidelines, packages/backend/**: "Verify authentication and authorization logic".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/backend/convex/waitlist.ts` around lines 5 - 10, The addToWaitlist
mutation is public and lacks rate limiting/authentication; update the mutation
(addToWaitlist) to enforce basic protections: verify the caller (session/user)
or require a valid auth token if available, deduplicate by checking the waitlist
table for an existing email before insert, and apply a rate-limit check (per-IP
or per-session) using your Convex rate-limiting pattern or a simple counter with
timestamps in a separate store to reject excessive requests; ensure the mutation
returns a clear error for rejected/rate-limited calls and keep the insertion
logic only after these checks pass.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/header.tsx`:
- Around line 11-31: Replace the outermost wrapper div in the Header component
with a semantic <header> element (keep the inner structure: the inner container
with className "flex ...", the existing <nav aria-label="Primary"> and the <hr
/>) so the component exposes a banner landmark for screen readers; update any
references to the wrapper if referenced by name and ensure the JSX still
imports/uses the Link and maps over links unchanged.
- Around line 13-27: The nav links and brand Link in header.tsx currently have
no visual active-state; update the Link rendering (the Link component usage
inside the links.map and the branding <Link to="/">) to include a visible active
style by: (1) adding an active className or CSS selector that targets TanStack
Router's aria-current="page" or the automatically applied "active" class (e.g.,
extend the existing className "text-sm font-medium hover:text-primary
transition-colors" to include a different text color/underline when
aria-current="page" or .active is present), and (2) for any Link that points to
"/" (the branding Link and the nav item where to === "/") pass activeOptions={{
exact: true }} so the root link only matches the exact path. Use the existing
links.map rendering and the branding Link to locate where to apply these
changes.
- Around line 4-8: The links array (the constant named links) is defined inside
the component body and gets reallocated on every render; move the declaration of
const links = [{ to: "/", label: "Home" }, { to: "/feed", label: "Feed" }, { to:
"/dashboard", label: "Dashboard" }] as const to module scope (i.e., outside the
Header component function) so it is created once, then reference that top-level
links constant from the component; keep the same name and the as const typing so
consumers (map/render logic inside Header) are unchanged.
In `@apps/web/src/routes/feed.tsx`:
- Around line 73-92: The events grid lacks an ARIA live region so screen readers
aren't notified when async content (variables status and events used in the JSX)
changes; wrap or modify the container div that renders the list (the div with
className "grid gap-4") to be an accessible live region by adding
aria-live="polite" and aria-atomic="true" (and/or role="status") so updates like
the "Loading…" state, the rendered EventCard items and the "No events found."
message are announced to assistive tech; ensure the same container (or a
dedicated visually-hidden element inside it) contains those messages so screen
readers receive the updates when status or events change.
- Around line 86-91: The conditional includes a redundant !events check because
usePaginatedQuery always returns an array; update the render logic inside the
component (where events and status are used) to remove the dead operand and only
test events.length === 0 together with status !== "LoadingFirstPage" (i.e.,
replace the combined condition {!events || events.length === 0} with
events.length === 0) so No events found renders correctly; ensure references are
to the existing events variable returned by usePaginatedQuery and the status
variable in the same scope.
Comment on lines
+4
to
+8
| const links = [ | ||
| { to: "/", label: "Home" }, | ||
| { to: "/feed", label: "Feed" }, | ||
| { to: "/dashboard", label: "Dashboard" }, | ||
| ] as const; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Hoist the links constant to module scope.
The array is pure data with no dependency on props or state; defining it inside the component body allocates a new array on every render unnecessarily.
♻️ Proposed refactor
+const links = [
+ { to: "/", label: "Home" },
+ { to: "/feed", label: "Feed" },
+ { to: "/dashboard", label: "Dashboard" },
+] as const;
+
export default function Header() {
- const links = [
- { to: "/", label: "Home" },
- { to: "/feed", label: "Feed" },
- { to: "/dashboard", label: "Dashboard" },
- ] as const;
-
return (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/header.tsx` around lines 4 - 8, The links array (the
constant named links) is defined inside the component body and gets reallocated
on every render; move the declaration of const links = [{ to: "/", label: "Home"
}, { to: "/feed", label: "Feed" }, { to: "/dashboard", label: "Dashboard" }] as
const to module scope (i.e., outside the Header component function) so it is
created once, then reference that top-level links constant from the component;
keep the same name and the as const typing so consumers (map/render logic inside
Header) are unchanged.
Comment on lines
+13
to
+27
| <Link to="/" className="text-xl font-bold"> | ||
| Biviant | ||
| </Link> | ||
| <nav aria-label="Primary" className="flex gap-6"> | ||
| {links.map(({ to, label }) => { | ||
| return ( | ||
| <Link | ||
| key={to} | ||
| to={to} | ||
| className="text-sm font-medium hover:text-primary transition-colors" | ||
| > | ||
| {label} | ||
| </Link> | ||
| ); | ||
| })} |
Contributor
There was a problem hiding this comment.
Add a visual active-state indicator to the navigation links.
TanStack Router automatically adds aria-current="page" and an active CSS class to active links, so assistive tech is already handled. However, there is no visual differentiation for sighted users — the active link looks identical to all other links.
Additionally, the branding <Link to="/"> at line 13 and the "Home" nav item both point to /. When linking to the root path, you should pass activeOptions={{ exact: true }} to prevent the link from matching all child routes.
♿ Proposed fix
- <Link to="/" className="text-xl font-bold">
+ <Link to="/" className="text-xl font-bold" activeOptions={{ exact: true }}>
Biviant
</Link>
<nav aria-label="Primary" className="flex gap-6">
{links.map(({ to, label }) => {
return (
<Link
key={to}
to={to}
- className="text-sm font-medium hover:text-primary transition-colors"
+ className="text-sm font-medium hover:text-primary transition-colors [&.active]:text-primary [&.active]:font-semibold"
+ activeOptions={to === "/" ? { exact: true } : undefined}
>
{label}
</Link>
);
})}
</nav>As per coding guidelines, apps/web/**: Check TanStack Router usage patterns; Focus on web performance and accessibility.
📝 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.
Suggested change
| <Link to="/" className="text-xl font-bold"> | |
| Biviant | |
| </Link> | |
| <nav aria-label="Primary" className="flex gap-6"> | |
| {links.map(({ to, label }) => { | |
| return ( | |
| <Link | |
| key={to} | |
| to={to} | |
| className="text-sm font-medium hover:text-primary transition-colors" | |
| > | |
| {label} | |
| </Link> | |
| ); | |
| })} | |
| <Link to="/" className="text-xl font-bold" activeOptions={{ exact: true }}> | |
| Biviant | |
| </Link> | |
| <nav aria-label="Primary" className="flex gap-6"> | |
| {links.map(({ to, label }) => { | |
| return ( | |
| <Link | |
| key={to} | |
| to={to} | |
| className="text-sm font-medium hover:text-primary transition-colors [&.active]:text-primary [&.active]:font-semibold" | |
| activeOptions={to === "/" ? { exact: true } : undefined} | |
| > | |
| {label} | |
| </Link> | |
| ); | |
| })} | |
| </nav> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/header.tsx` around lines 13 - 27, The nav links and
brand Link in header.tsx currently have no visual active-state; update the Link
rendering (the Link component usage inside the links.map and the branding <Link
to="/">) to include a visible active style by: (1) adding an active className or
CSS selector that targets TanStack Router's aria-current="page" or the
automatically applied "active" class (e.g., extend the existing className
"text-sm font-medium hover:text-primary transition-colors" to include a
different text color/underline when aria-current="page" or .active is present),
and (2) for any Link that points to "/" (the branding Link and the nav item
where to === "/") pass activeOptions={{ exact: true }} so the root link only
matches the exact path. Use the existing links.map rendering and the branding
Link to locate where to apply these changes.
Comment on lines
+73
to
+92
| <div className="grid gap-4"> | ||
| {status === "LoadingFirstPage" && ( | ||
| <div className="text-sm text-muted-foreground">Loading…</div> | ||
| )} | ||
|
|
||
| {events?.map((event) => ( | ||
| <EventCard | ||
| key={event._id} | ||
| event={event} | ||
| topicNamesById={topicNamesById} | ||
| /> | ||
| ))} | ||
|
|
||
| {status !== "LoadingFirstPage" && | ||
| (!events || events.length === 0) && ( | ||
| <div className="text-sm text-muted-foreground"> | ||
| No events found. | ||
| </div> | ||
| )} | ||
| </div> |
Contributor
There was a problem hiding this comment.
Announce dynamic content changes to screen readers via aria-live.
The events grid is populated asynchronously and changes whenever a topic filter is selected, but there is no live region. Screen-reader users get no notification when results load or the grid becomes empty.
♿ Proposed fix
- <div className="grid gap-4">
+ <div className="grid gap-4" aria-live="polite" aria-atomic="false">
{status === "LoadingFirstPage" && (
<div className="text-sm text-muted-foreground">Loading…</div>
)}As per coding guidelines, apps/web/**: Focus on web performance and accessibility.
📝 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.
Suggested change
| <div className="grid gap-4"> | |
| {status === "LoadingFirstPage" && ( | |
| <div className="text-sm text-muted-foreground">Loading…</div> | |
| )} | |
| {events?.map((event) => ( | |
| <EventCard | |
| key={event._id} | |
| event={event} | |
| topicNamesById={topicNamesById} | |
| /> | |
| ))} | |
| {status !== "LoadingFirstPage" && | |
| (!events || events.length === 0) && ( | |
| <div className="text-sm text-muted-foreground"> | |
| No events found. | |
| </div> | |
| )} | |
| </div> | |
| <div className="grid gap-4" aria-live="polite" aria-atomic="false"> | |
| {status === "LoadingFirstPage" && ( | |
| <div className="text-sm text-muted-foreground">Loading…</div> | |
| )} | |
| {events?.map((event) => ( | |
| <EventCard | |
| key={event._id} | |
| event={event} | |
| topicNamesById={topicNamesById} | |
| /> | |
| ))} | |
| {status !== "LoadingFirstPage" && | |
| (!events || events.length === 0) && ( | |
| <div className="text-sm text-muted-foreground"> | |
| No events found. | |
| </div> | |
| )} | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/routes/feed.tsx` around lines 73 - 92, The events grid lacks an
ARIA live region so screen readers aren't notified when async content (variables
status and events used in the JSX) changes; wrap or modify the container div
that renders the list (the div with className "grid gap-4") to be an accessible
live region by adding aria-live="polite" and aria-atomic="true" (and/or
role="status") so updates like the "Loading…" state, the rendered EventCard
items and the "No events found." message are announced to assistive tech; ensure
the same container (or a dedicated visually-hidden element inside it) contains
those messages so screen readers receive the updates when status or events
change.
Comment on lines
+86
to
+91
| {status !== "LoadingFirstPage" && | ||
| (!events || events.length === 0) && ( | ||
| <div className="text-sm text-muted-foreground"> | ||
| No events found. | ||
| </div> | ||
| )} |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
!events guard is dead code — usePaginatedQuery always returns an array.
Convex's usePaginatedQuery initialises results as [] and never returns null or undefined, so !events is always false. Simplify to events.length === 0.
♻️ Proposed fix
{status !== "LoadingFirstPage" &&
- (!events || events.length === 0) && (
+ events.length === 0 && (
<div className="text-sm text-muted-foreground">
No events found.
</div>
)}📝 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.
Suggested change
| {status !== "LoadingFirstPage" && | |
| (!events || events.length === 0) && ( | |
| <div className="text-sm text-muted-foreground"> | |
| No events found. | |
| </div> | |
| )} | |
| {status !== "LoadingFirstPage" && | |
| events.length === 0 && ( | |
| <div className="text-sm text-muted-foreground"> | |
| No events found. | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/routes/feed.tsx` around lines 86 - 91, The conditional includes
a redundant !events check because usePaginatedQuery always returns an array;
update the render logic inside the component (where events and status are used)
to remove the dead operand and only test events.length === 0 together with
status !== "LoadingFirstPage" (i.e., replace the combined condition {!events ||
events.length === 0} with events.length === 0) so No events found renders
correctly; ensure references are to the existing events variable returned by
usePaginatedQuery and the status variable in the same scope.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This was referenced Apr 25, 2026
Merged
This was referenced May 5, 2026
Merged
Merged
Merged
Merged
Merged
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
1 participant
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.
Summary by CodeRabbit
New Features
Documentation
Chores