Repository navigation
feat: Tanstack multi-room example - #385
Conversation
|
Cursor Agent can help with this pull request. Just |
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new "multi-room" example app (React + Vite + TypeScript + Tailwind) with persisted per-room stores, localStorage-backed room list CRUD and seeding, room detail pages with a table browser, and accompanying config, docs, and build tooling. No backend code changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser
participant Router as TanStack Router
participant RoomsList as RoomsListPage
participant RoomLoader as RoomLoader/RoomPage
participant StoreFactory as createRoomStore
participant Storage as localStorage
participant DuckDB as DuckDB WASM
Browser->>Router: Navigate to "/" or "/room/{id}"
Router->>RoomsList: Render list (if "/")
RoomsList->>Storage: getRoomsList()
Storage-->>RoomsList: rooms[]
RoomsList->>Browser: User selects or creates room
Browser->>Router: Navigate to "/room/{id}"
Router->>RoomLoader: Render RoomLoader for id
RoomLoader->>Storage: getRoom(id)
Storage-->>RoomLoader: RoomListEntry (dataSources)
RoomLoader->>StoreFactory: createRoomStore(id, dataSources)
StoreFactory->>Storage: init per-room persist key (multi-room-{id})
StoreFactory->>DuckDB: instantiate/attach DuckDB WASM per-room
DuckDB-->>StoreFactory: ready
StoreFactory-->>RoomLoader: store ready
RoomLoader->>Browser: render RoomShell + TableBrowser with data
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ilyabo
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Line 41: The heading "Cursor Cloud specific instructions" should use a
hyphenated compound modifier; update the heading text (the line containing
"Cursor Cloud specific instructions") to "Cursor-Cloud-specific instructions" so
the compound modifier correctly links to "specific instructions."
|
|
||
| When adding features, update the relevant documentation and verify that example applications work with your changes. See `CONTRIBUTING.md` for full guidelines. | ||
|
|
||
| ## Cursor Cloud specific instructions |
There was a problem hiding this comment.
Use hyphenated compound modifier in heading.
Line 41 reads a bit awkwardly; use a hyphenated form for the modifier.
✏️ Suggested edit
-## Cursor Cloud specific instructions
+## Cursor Cloud-specific instructions📝 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.
| ## Cursor Cloud specific instructions | |
| ## Cursor Cloud-specific instructions |
🧰 Tools
🪛 LanguageTool
[grammar] ~41-~41: Use a hyphen to join words.
Context: ...d` for full guidelines. ## Cursor Cloud specific instructions ### Environment ...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` at line 41, The heading "Cursor Cloud specific instructions"
should use a hyphenated compound modifier; update the heading text (the line
containing "Cursor Cloud specific instructions") to "Cursor-Cloud-specific
instructions" so the compound modifier correctly links to "specific
instructions."
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
examples/multi-room/src/rooms-db.ts (1)
111-113: Seed defaults in one transaction instead of per-room DB open/close.The current loop calls
putRoom()for each default, which repeatedly opens/closes IndexedDB connections. A singlereadwritetransaction is lighter and more robust.Suggested refactor
export async function seedDefaultRooms(): Promise<void> { const existing = await getAllRooms(); if (existing.length > 0) return; @@ - for (const room of defaults) { - await putRoom(room); - } + const db = await openDb(); + try { + const tx = db.transaction(STORE_NAME, 'readwrite'); + const store = tx.objectStore(STORE_NAME); + for (const room of defaults) { + store.put(room); + } + await new Promise<void>((resolve, reject) => { + tx.oncomplete = () => resolve(); + tx.onerror = () => reject(tx.error); + tx.onabort = () => reject(tx.error); + }); + } finally { + db.close(); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/multi-room/src/rooms-db.ts` around lines 111 - 113, The seed routine currently calls putRoom() inside a loop which opens/closes the IndexedDB connection per room; instead, open the DB once and perform all writes in a single readwrite transaction to be efficient and atomic. Modify the seeding code so it obtains the same DB instance used by putRoom (or refactor putRoom to accept an existing IDBTransaction/IDBObjectStore), create a single transaction ('readwrite') and call objectStore.put(...) for each room in defaults, then await the transaction completion before closing; update references to putRoom, the DB-opening function, and the transaction handling to ensure all default rooms are seeded in that single transaction.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/multi-room/package.json`:
- Line 31: Update the TypeScript dependency in examples/multi-room's
package.json by changing the pinned version of "typescript" from "5.9.2" to the
workspace baseline "5.9.3" so the project uses the repo-wide TypeScript version
and avoids inconsistent typechecking; ensure the package.json entry for
"typescript" is updated accordingly and run a quick npm/yarn install and
typecheck to confirm compatibility.
In `@examples/multi-room/src/index.css`:
- Around line 5-9: The Tailwind v4 `@source` directives in
examples/multi-room/src/index.css are being flagged by linting; update the
Stylelint config to allow the "source" at-rule by adding an at-rule-no-unknown
exception (e.g., set "at-rule-no-unknown": [true, { "ignoreAtRules": ["source"]
}]) in .stylelintrc.json, and update biome.json's CSS lint/parser options to
accept unknown Tailwind at-rules (e.g., add an allow/ignore entry for "source"
in the CSS lint rules or parser options) so the `@source` directives are not
reported as unknown.
In `@examples/multi-room/src/main.tsx`:
- Around line 56-62: Currently the app mount is nested inside
seedDefaultRooms().then(...), which prevents rendering if seeding fails; instead
call
createRoot(document.getElementById('root')!).render(<StrictMode><RouterProvider
router={router} /></StrictMode>) immediately to mount the app, and run
seedDefaultRooms() separately in the background (e.g.,
seedDefaultRooms().catch(err => /* log/handle error */) or use async IIFE) so
seeding failures are handled without blocking createRoot/render; update
references to seedDefaultRooms and the createRoot/render block accordingly.
In `@examples/multi-room/src/room-store.ts`:
- Around line 17-29: The store currently runs room.initialize() immediately via
an async IIFE inside room-store.ts (calling store.getState().room.initialize(),
then store.setState(... draft.room.initialized = true) and catching errors with
store.getState().room.captureException), which creates a race with the
component; remove that IIFE from the store factory entirely and instead call
await room.initialize() from the RoomPage component's useEffect, then set the
component's ready state only after initialization completes and update the
store's initialized flag via the existing store.setState code; keep the existing
error handling by calling room.captureException(error) from the component if
initialize() throws.
In `@examples/multi-room/src/RoomPage.tsx`:
- Around line 20-26: The useEffect call that invokes getRoom(roomId) doesn't
handle promise rejection, which can leave the spinner indefinitely; update the
effect to handle errors by adding a .catch (or use an async function with
try/catch) on the getRoom(roomId) call, and in the catch branch set an
appropriate state (e.g., call setNotFound(true) or an error state) while still
respecting the cancelled flag before updating state; ensure the promise chain
also ignores updates when cancelled is true so setRoomRecord or setNotFound are
only called when not cancelled.
- Around line 173-176: selectedTable is interpolated directly into the SQL
string passed to QueryDataTable (query={`SELECT * FROM "${selectedTable}"`})
which breaks if the table name contains double-quotes; fix by escaping any
double-quote characters in selectedTable before composing the query (e.g.,
replace all " with ""), or add a small helper like quoteIdentifier(name) that
returns the identifier safely quoted, and use that helper when building the
query for QueryDataTable.
In `@examples/multi-room/src/RoomsListPage.tsx`:
- Around line 43-51: confirmRename is being called from both onBlur and onSubmit
which can cause duplicate putRoom() writes; add a simple guard (e.g., component
state boolean like isSaving or isSubmitting) and check it at the top of
confirmRename (return early if true), set it true immediately before calling
await putRoom({...}) and clear it after the await (in finally), and ensure the
same guard is applied to the other rename handler that also calls putRoom so
blur+submit cannot run concurrently.
- Around line 17-23: The mount logic currently calls loadRooms() which awaits
getAllRooms() without handling rejections; update loadRooms (and the useEffect
caller) to handle errors: wrap the await getAllRooms() call inside try/catch in
the loadRooms function (or call loadRooms().catch(...) in the effect), on error
set an error/loading state (e.g., setError, setLoading) and optionally clear or
preserve rooms (e.g., setRooms([]) or leave previous value) and surface the
error to the UI (toast, message, or retry button) so failures no longer silently
leave the page stale; reference functions/useEffect: loadRooms, getAllRooms,
setRooms, and the useEffect that invokes loadRooms.
In `@examples/multi-room/vite.config.ts`:
- Around line 5-7: Add "zod" to examples/multi-room/package.json dependencies
and validate the exported Vite config using a Zod schema: define a ConfigSchema
(e.g., const ConfigSchema = z.object({ plugins: z.array(z.any()) })) and infer
its type via z.infer<typeof ConfigSchema>, parse/validate the object you pass to
defineConfig (the default export) with ConfigSchema.parse(...) before returning
it; update the exported symbol (the object passed to defineConfig in the default
export) to use the validated/typed value so the repo's config-validation rule is
satisfied.
---
Nitpick comments:
In `@examples/multi-room/src/rooms-db.ts`:
- Around line 111-113: The seed routine currently calls putRoom() inside a loop
which opens/closes the IndexedDB connection per room; instead, open the DB once
and perform all writes in a single readwrite transaction to be efficient and
atomic. Modify the seeding code so it obtains the same DB instance used by
putRoom (or refactor putRoom to accept an existing
IDBTransaction/IDBObjectStore), create a single transaction ('readwrite') and
call objectStore.put(...) for each room in defaults, then await the transaction
completion before closing; update references to putRoom, the DB-opening
function, and the transaction handling to ensure all default rooms are seeded in
that single transaction.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
examples/multi-room/index.htmlexamples/multi-room/package.jsonexamples/multi-room/src/RoomPage.tsxexamples/multi-room/src/RoomsListPage.tsxexamples/multi-room/src/index.cssexamples/multi-room/src/main.tsxexamples/multi-room/src/room-store.tsexamples/multi-room/src/rooms-db.tsexamples/multi-room/tsconfig.app.jsonexamples/multi-room/tsconfig.jsonexamples/multi-room/tsconfig.node.jsonexamples/multi-room/vite.config.ts
✅ Files skipped from review due to trivial changes (1)
- examples/multi-room/index.html
| "@vitejs/plugin-react": "^5.0.3", | ||
| "eslint": "^9.36.0", | ||
| "tailwindcss": "^4.1.18", | ||
| "typescript": "5.9.2", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check the specific file mentioned in the review
cat -n examples/multi-room/package.jsonRepository: sqlrooms/sqlrooms
Length of output: 1172
🏁 Script executed:
#!/bin/bash
# Run the verification script from the review to check all packages
python - <<'PY'
import json, pathlib
expected = "5.9.3"
for p in sorted(pathlib.Path(".").rglob("package.json")):
try:
data = json.loads(p.read_text())
except Exception:
continue
ts = (data.get("devDependencies", {}) or {}).get("typescript") or (data.get("dependencies", {}) or {}).get("typescript")
if ts:
status = "OK" if ts == expected else "MISMATCH"
print(f"{status}: {p} -> {ts}")
PYRepository: sqlrooms/sqlrooms
Length of output: 1411
Align TypeScript to the workspace baseline.
The package pins typescript to 5.9.2, which drifts from the repo-wide baseline of 5.9.3 and can produce inconsistent typecheck behavior.
🔧 Proposed fix
- "typescript": "5.9.2",
+ "typescript": "5.9.3",📝 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.
| "typescript": "5.9.2", | |
| "typescript": "5.9.3", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/multi-room/package.json` at line 31, Update the TypeScript
dependency in examples/multi-room's package.json by changing the pinned version
of "typescript" from "5.9.2" to the workspace baseline "5.9.3" so the project
uses the repo-wide TypeScript version and avoids inconsistent typechecking;
ensure the package.json entry for "typescript" is updated accordingly and run a
quick npm/yarn install and typecheck to confirm compatibility.
| @source '../index.html'; | ||
| @source 'src/**/*.{ts,tsx}'; | ||
| @source '../../../packages/*/src/**/*.{ts,tsx}'; | ||
| @source '{./,../../../}node_modules/@sqlrooms/*/dist/**/*.js'; | ||
| @source '{./,../../../}node_modules/.pnpm/node_modules/@sqlrooms/*/dist/**/*.js'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Tailwind directives in this file ==="
rg -n '@source|@import' examples/multi-room/src/index.css
echo
echo "=== Stylelint config entries related to unknown at-rules ==="
fd -HI 'stylelint.config.*|.stylelintrc.*' -a | xargs -r rg -n 'at-rule-no-unknown|scss/at-rule-no-unknown|ignoreAtRules|source'
echo
echo "=== Biome config entries related to CSS parsing ==="
fd -HI 'biome.json|biome.jsonc' -a | xargs -r rg -n 'css|tailwind|parser'Repository: sqlrooms/sqlrooms
Length of output: 494
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== All config files in repo ==="
fd -HI 'stylelint|biome|eslint|prettier' --type f -e json -e js -e jsonc -e yaml -e yml -e config | head -30
echo
echo "=== Checking package.json for linting deps ==="
find . -name "package.json" -type f | xargs grep -l "stylelint\|biome" | head -10
echo
echo "=== Root level config files ==="
ls -la | grep -E 'stylelint|biome|eslint|prettier' || echo "No root level linting configs found"
echo
echo "=== Examples/multi-room package.json ==="
cat examples/multi-room/package.json 2>/dev/null || echo "File not found"Repository: sqlrooms/sqlrooms
Length of output: 1143
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== .stylelintrc.json content ==="
cat .stylelintrc.json
echo
echo "=== biome.json content ==="
cat biome.json
echo
echo "=== examples/multi-room/package.json ==="
cat examples/multi-room/package.json 2>/dev/null || echo "File not found"Repository: sqlrooms/sqlrooms
Length of output: 3818
Configure lint tooling for Tailwind v4 @source directives.
The @source directives in lines 5-9 are valid Tailwind v4 syntax, but the Stylelint and Biome configurations do not explicitly allow them. The .stylelintrc.json extends stylelint-config-standard-scss without disabling or configuring the at-rule-no-unknown rule, and biome.json has CSS linting enabled with default rules that will flag @source as unknown. Add explicit configuration (e.g., ignoreAtRules in Stylelint or CSS parser options in Biome) to allow these Tailwind directives.
🧰 Tools
🪛 Biome (2.4.4)
[error] 5-5: Tailwind-specific syntax is disabled.
(parse)
[error] 6-6: Tailwind-specific syntax is disabled.
(parse)
[error] 7-7: Tailwind-specific syntax is disabled.
(parse)
[error] 8-8: Tailwind-specific syntax is disabled.
(parse)
[error] 9-9: Tailwind-specific syntax is disabled.
(parse)
🪛 Stylelint (17.3.0)
[error] 5-5: Unexpected unknown at-rule "@source" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
[error] 6-6: Unexpected unknown at-rule "@source" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
[error] 7-7: Unexpected unknown at-rule "@source" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
[error] 8-8: Unexpected unknown at-rule "@source" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
[error] 9-9: Unexpected unknown at-rule "@source" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/multi-room/src/index.css` around lines 5 - 9, The Tailwind v4
`@source` directives in examples/multi-room/src/index.css are being flagged by
linting; update the Stylelint config to allow the "source" at-rule by adding an
at-rule-no-unknown exception (e.g., set "at-rule-no-unknown": [true, {
"ignoreAtRules": ["source"] }]) in .stylelintrc.json, and update biome.json's
CSS lint/parser options to accept unknown Tailwind at-rules (e.g., add an
allow/ignore entry for "source" in the CSS lint rules or parser options) so the
`@source` directives are not reported as unknown.
| useEffect(() => { | ||
| let cancelled = false; | ||
| getRoom(roomId).then((record) => { | ||
| if (cancelled) return; | ||
| if (record) setRoomRecord(record); | ||
| else setNotFound(true); | ||
| }); |
There was a problem hiding this comment.
Handle getRoom rejection to avoid indefinite loading state.
If getRoom(roomId) rejects, neither roomRecord nor notFound updates, so the spinner can render forever and the rejection is unhandled.
Suggested fix
useEffect(() => {
let cancelled = false;
- getRoom(roomId).then((record) => {
- if (cancelled) return;
- if (record) setRoomRecord(record);
- else setNotFound(true);
- });
+ getRoom(roomId)
+ .then((record) => {
+ if (cancelled) return;
+ if (record) setRoomRecord(record);
+ else setNotFound(true);
+ })
+ .catch((error) => {
+ if (cancelled) return;
+ console.error('Failed to load room:', error);
+ setNotFound(true);
+ });
return () => {
cancelled = true;
};
}, [roomId]);📝 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.
| useEffect(() => { | |
| let cancelled = false; | |
| getRoom(roomId).then((record) => { | |
| if (cancelled) return; | |
| if (record) setRoomRecord(record); | |
| else setNotFound(true); | |
| }); | |
| useEffect(() => { | |
| let cancelled = false; | |
| getRoom(roomId) | |
| .then((record) => { | |
| if (cancelled) return; | |
| if (record) setRoomRecord(record); | |
| else setNotFound(true); | |
| }) | |
| .catch((error) => { | |
| if (cancelled) return; | |
| console.error('Failed to load room:', error); | |
| setNotFound(true); | |
| }); | |
| return () => { | |
| cancelled = true; | |
| }; | |
| }, [roomId]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/multi-room/src/RoomPage.tsx` around lines 20 - 26, The useEffect
call that invokes getRoom(roomId) doesn't handle promise rejection, which can
leave the spinner indefinitely; update the effect to handle errors by adding a
.catch (or use an async function with try/catch) on the getRoom(roomId) call,
and in the catch branch set an appropriate state (e.g., call setNotFound(true)
or an error state) while still respecting the cancelled flag before updating
state; ensure the promise chain also ignores updates when cancelled is true so
setRoomRecord or setNotFound are only called when not cancelled.
| <QueryDataTable | ||
| key={selectedTable} | ||
| query={`SELECT * FROM "${selectedTable}"`} | ||
| fontSize="text-xs" |
There was a problem hiding this comment.
Escape table identifiers before composing SQL text.
selectedTable is interpolated directly into the query. If a table name contains ", this can break the query and produce unintended SQL text.
Suggested fix
function TableBrowser() {
const store = useRoomStoreApi<RoomState>();
const initialized = useStore(store, (s) => s.room.initialized);
const tables = useStore(store, (s) => s.db.tables);
const [selectedTable, setSelectedTable] = useState<string | null>(null);
+ const quoteIdentifier = (name: string) => `"${name.replace(/"/g, '""')}"`;
@@
{selectedTable ? (
<QueryDataTable
key={selectedTable}
- query={`SELECT * FROM "${selectedTable}"`}
+ query={`SELECT * FROM ${quoteIdentifier(selectedTable)}`}
fontSize="text-xs"
/>
) : (📝 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.
| <QueryDataTable | |
| key={selectedTable} | |
| query={`SELECT * FROM "${selectedTable}"`} | |
| fontSize="text-xs" | |
| <QueryDataTable | |
| key={selectedTable} | |
| query={`SELECT * FROM ${quoteIdentifier(selectedTable)}`} | |
| fontSize="text-xs" | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/multi-room/src/RoomPage.tsx` around lines 173 - 176, selectedTable
is interpolated directly into the SQL string passed to QueryDataTable
(query={`SELECT * FROM "${selectedTable}"`}) which breaks if the table name
contains double-quotes; fix by escaping any double-quote characters in
selectedTable before composing the query (e.g., replace all " with ""), or add a
small helper like quoteIdentifier(name) that returns the identifier safely
quoted, and use that helper when building the query for QueryDataTable.
| const loadRooms = useCallback(async () => { | ||
| setRooms(await getAllRooms()); | ||
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| loadRooms(); | ||
| }, [loadRooms]); |
There was a problem hiding this comment.
Handle room-list load failures so mount loading doesn’t silently fail.
If getAllRooms() rejects, the promise from loadRooms() is currently unhandled in the mount effect, which can leave the page stale without recovery.
Suggested fix
const loadRooms = useCallback(async () => {
- setRooms(await getAllRooms());
+ try {
+ setRooms(await getAllRooms());
+ } catch (error) {
+ console.error('Failed to load rooms:', error);
+ }
}, []);
useEffect(() => {
- loadRooms();
+ void loadRooms();
}, [loadRooms]);📝 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.
| const loadRooms = useCallback(async () => { | |
| setRooms(await getAllRooms()); | |
| }, []); | |
| useEffect(() => { | |
| loadRooms(); | |
| }, [loadRooms]); | |
| const loadRooms = useCallback(async () => { | |
| try { | |
| setRooms(await getAllRooms()); | |
| } catch (error) { | |
| console.error('Failed to load rooms:', error); | |
| } | |
| }, []); | |
| useEffect(() => { | |
| void loadRooms(); | |
| }, [loadRooms]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/multi-room/src/RoomsListPage.tsx` around lines 17 - 23, The mount
logic currently calls loadRooms() which awaits getAllRooms() without handling
rejections; update loadRooms (and the useEffect caller) to handle errors: wrap
the await getAllRooms() call inside try/catch in the loadRooms function (or call
loadRooms().catch(...) in the effect), on error set an error/loading state
(e.g., setError, setLoading) and optionally clear or preserve rooms (e.g.,
setRooms([]) or leave previous value) and surface the error to the UI (toast,
message, or retry button) so failures no longer silently leave the page stale;
reference functions/useEffect: loadRooms, getAllRooms, setRooms, and the
useEffect that invokes loadRooms.
| const confirmRename = async () => { | ||
| if (!editingId) return; | ||
| const room = rooms.find((r) => r.id === editingId); | ||
| if (room && editName.trim()) { | ||
| await putRoom({...room, name: editName.trim()}); | ||
| await loadRooms(); | ||
| } | ||
| setEditingId(null); | ||
| }; |
There was a problem hiding this comment.
Prevent duplicate rename writes from blur + submit firing together.
The same async mutator runs from both onBlur and onSubmit. Clicking Save can trigger both paths and issue duplicate putRoom() calls.
Suggested fix
-import {useCallback, useEffect, useState} from 'react';
+import {useCallback, useEffect, useRef, useState} from 'react';
@@
const [rooms, setRooms] = useState<RoomRecord[]>([]);
const [editingId, setEditingId] = useState<string | null>(null);
const [editName, setEditName] = useState('');
+ const renameInFlightRef = useRef(false);
@@
const confirmRename = async () => {
- if (!editingId) return;
- const room = rooms.find((r) => r.id === editingId);
- if (room && editName.trim()) {
- await putRoom({...room, name: editName.trim()});
- await loadRooms();
+ if (!editingId || renameInFlightRef.current) return;
+ renameInFlightRef.current = true;
+ try {
+ const room = rooms.find((r) => r.id === editingId);
+ if (room && editName.trim()) {
+ await putRoom({...room, name: editName.trim()});
+ await loadRooms();
+ }
+ setEditingId(null);
+ } finally {
+ renameInFlightRef.current = false;
}
- setEditingId(null);
};Also applies to: 80-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/multi-room/src/RoomsListPage.tsx` around lines 43 - 51,
confirmRename is being called from both onBlur and onSubmit which can cause
duplicate putRoom() writes; add a simple guard (e.g., component state boolean
like isSaving or isSubmitting) and check it at the top of confirmRename (return
early if true), set it true immediately before calling await putRoom({...}) and
clear it after the await (in finally), and ensure the same guard is applied to
the other rename handler that also calls putRoom so blur+submit cannot run
concurrently.
| export default defineConfig({ | ||
| plugins: [react(), tailwindcss()], | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if Zod is in project dependencies
echo "=== Checking for Zod in workspace ==="
find . -name "package.json" -type f | head -20 | xargs -I {} sh -c 'echo "File: {}"; cat "{}" | grep -A2 -B2 "zod" || true'
echo ""
echo "=== Looking for all config files in the project ==="
fd -e "config.ts" -e "config.tsx" -e "config.js" | head -20Repository: sqlrooms/sqlrooms
Length of output: 3251
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if vite.config.ts exists and read it
echo "=== Current vite.config.ts content ==="
if [ -f "examples/multi-room/vite.config.ts" ]; then
cat -n "examples/multi-room/vite.config.ts"
else
echo "File not found at examples/multi-room/vite.config.ts"
# Search for it
fd "vite.config.ts"
fi
echo ""
echo "=== Checking root package.json for Zod ==="
if [ -f "package.json" ]; then
cat "package.json" | jq '.dependencies, .devDependencies' 2>/dev/null | grep -A5 -B5 "zod" || echo "Zod not found in dependencies"
fiRepository: sqlrooms/sqlrooms
Length of output: 709
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check multi-room package.json
echo "=== examples/multi-room/package.json ==="
if [ -f "examples/multi-room/package.json" ]; then
cat "examples/multi-room/package.json" | jq '.dependencies, .devDependencies' 2>/dev/null
else
echo "File not found"
fi
echo ""
echo "=== Checking a few other vite.config.ts files ==="
for file in "apps/sqlrooms-cli-ui/vite.config.ts" "examples/ai/vite.config.ts" "examples/app-builder/vite.config.ts"; do
if [ -f "$file" ]; then
echo "--- $file ---"
cat -n "$file" | head -15
echo ""
fi
doneRepository: sqlrooms/sqlrooms
Length of output: 2144
Add Zod to package.json and validate the config object using a Zod schema.
This file violates the repository config-validation rule. The proposed refactor requires Zod to be added to examples/multi-room/package.json as a dependency:
{
"dependencies": {
"zod": "^4.1.8"
}
}Then apply the validation pattern:
🔧 Proposed refactor
import {defineConfig} from 'vite';
import tailwindcss from '@tailwindcss/vite';
import react from '@vitejs/plugin-react';
+import {z} from 'zod';
+const ViteConfigSchema = z.object({
+ plugins: z.array(z.any()),
+});
+
+type ViteConfig = z.infer<typeof ViteConfigSchema>;
+
+const config: ViteConfig = {
+ plugins: [react(), tailwindcss()],
+};
+
export default defineConfig({
- plugins: [react(), tailwindcss()],
+ ...ViteConfigSchema.parse(config),
});As per coding guidelines: "Validate all configuration objects using Zod schemas and infer types via z.infer<typeof ConfigSchema>".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/multi-room/vite.config.ts` around lines 5 - 7, Add "zod" to
examples/multi-room/package.json dependencies and validate the exported Vite
config using a Zod schema: define a ConfigSchema (e.g., const ConfigSchema =
z.object({ plugins: z.array(z.any()) })) and infer its type via z.infer<typeof
ConfigSchema>, parse/validate the object you pass to defineConfig (the default
export) with ConfigSchema.parse(...) before returning it; update the exported
symbol (the object passed to defineConfig in the default export) to use the
validated/typed value so the repo's config-validation rule is satisfied.
There was a problem hiding this comment.
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 `@examples/multi-room/README.md`:
- Line 3: Update the display text "Github repo" to use the correct
capitalization "GitHub" in the README entry (the link label in the
examples/multi-room README where the bracketed text reads "Github repo"); keep
the URL unchanged and only modify the visible link text to "GitHub repo".
- Line 5: Remove the invalid alt attribute from the <video> element and provide
accessible alternatives: either add descriptive fallback text (or a link)
between the opening and closing <video> tags, or include an appropriate
aria-label/aria-labelledby on the <video> element to convey the same
description; update the <video> tag in the README so browsers no longer use an
ignored alt attribute and screen readers receive the descriptive text.
- Line 5: Remove the invalid alt attribute from the <video> element and instead
add accessible fallback content between the opening and closing <video> tags
(e.g., a short descriptive text, captions, or a link to the media) so assistive
technologies and browsers without video support can present a meaningful
alternative; update the existing <video src="/media/examples/multi-room.mp4"
width="450" controls loop muted> element accordingly.
- Around line 19-21: Update the README instruction to specify that pnpm install
must be run from the repository root (so workspace:* dependencies resolve) and
change the dev command to include the workspace filter for the example (use pnpm
dev --filter sqlrooms-multi-room); ensure the README shows a comment like "From
the repo root" before the install command and replace the plain "pnpm dev" line
with "pnpm dev --filter sqlrooms-multi-room" so users run the example correctly
in the monorepo context.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
docs/media/examples/multi-room.mp4is excluded by!**/*.mp4pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
docs/examples.mdexamples/multi-room/README.md
| @@ -0,0 +1,22 @@ | |||
| ### Multi-Room | |||
|
|
|||
| [Github repo](https://github.com/sqlrooms/sqlrooms/tree/main/examples/multi-room) | |||
There was a problem hiding this comment.
Github → GitHub (capital "H")
The official name is GitHub.
✏️ Proposed fix
-[Github repo](https://github.com/sqlrooms/sqlrooms/tree/main/examples/multi-room)
+[GitHub repo](https://github.com/sqlrooms/sqlrooms/tree/main/examples/multi-room)📝 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.
| [Github repo](https://github.com/sqlrooms/sqlrooms/tree/main/examples/multi-room) | |
| [GitHub repo](https://github.com/sqlrooms/sqlrooms/tree/main/examples/multi-room) |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~3-~3: The official name of this software platform is spelled with a capital “H”.
Context: ### Multi-Room [Github repo](https://github.com/sqlrooms/sqlro...
(GITHUB)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/multi-room/README.md` at line 3, Update the display text "Github
repo" to use the correct capitalization "GitHub" in the README entry (the link
label in the examples/multi-room README where the bracketed text reads "Github
repo"); keep the URL unchanged and only modify the visible link text to "GitHub
repo".
|
|
||
| [Github repo](https://github.com/sqlrooms/sqlrooms/tree/main/examples/multi-room) | ||
|
|
||
| <video src="/media/examples/multi-room.mp4" alt="SQLRooms Multi-Room example" width="450" controls loop muted></video> |
There was a problem hiding this comment.
alt is not a valid attribute on <video>
The HTML5 specification intentionally omits an alt attribute for <video> — it cannot be used within the opening <video> tag; this is by design as the video element can contain fallback options. The attribute is silently ignored by browsers. Use inline fallback text between the tags, or aria-label for an accessible label.
♿ Proposed fix
-<video src="/media/examples/multi-room.mp4" alt="SQLRooms Multi-Room example" width="450" controls loop muted></video>
+<video src="/media/examples/multi-room.mp4" aria-label="SQLRooms Multi-Room example" width="450" controls loop muted>
+ SQLRooms Multi-Room example (video)
+</video>📝 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.
| <video src="/media/examples/multi-room.mp4" alt="SQLRooms Multi-Room example" width="450" controls loop muted></video> | |
| <video src="/media/examples/multi-room.mp4" aria-label="SQLRooms Multi-Room example" width="450" controls loop muted> | |
| SQLRooms Multi-Room example (video) | |
| </video> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/multi-room/README.md` at line 5, Remove the invalid alt attribute
from the <video> element and provide accessible alternatives: either add
descriptive fallback text (or a link) between the opening and closing <video>
tags, or include an appropriate aria-label/aria-labelledby on the <video>
element to convey the same description; update the <video> tag in the README so
browsers no longer use an ignored alt attribute and screen readers receive the
descriptive text.
🧩 Analysis chain
🌐 Web query:
HTML5 alt attribute video element specification validity
💡 Result:
The HTML (Living Standard) <video> element does not define an alt attribute, so alt on <video> is not spec-valid (a validator should flag it as an invalid attribute). The only attributes defined for <video> are global attributes plus media/video-specific ones like src, poster, preload, autoplay, loop, muted, controls, width, height, etc.—not alt. [1]
What to do instead (spec-valid patterns):
- Put a text alternative / download links as fallback content inside
<video>…</video>(shown in MDN examples). [2] - Provide accessibility alternatives appropriate to video: captions/subtitles (and usually a transcript / audio description depending on content). [3]
- If you need an accessible name for the control, use ARIA labeling (e.g.,
aria-label) rather thanalt(ARIA is allowed via global attributes). [1]
Also, the poster image is configured via the poster attribute, and the spec historically did not allow alt on <video> as a way to describe either the video or its poster. [1] [4]
Sources: [1] WHATWG HTML Standard – media elements (<video> attributes) (html.spec.whatwg.org); [2] MDN <source> / <video> example showing fallback content inside <video> (developer.mozilla.org); [3] W3C WAI guidance on captions requirements/needs (w3.org); [4] W3C HTML WG wiki (historical change proposal noting alt not allowed on <video>) (w3.org).
Citations:
- 1: https://html.spec.whatwg.org/media-elements.html?utm_source=openai
- 2: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/source?utm_source=openai
- 3: https://www.w3.org/WAI/media/av/captions/?utm_source=openai
- 4: https://www.w3.org/html/wg/wiki/ChangeProposals/PosterElement?utm_source=openai
🏁 Script executed:
cat examples/multi-room/README.md | head -20Repository: sqlrooms/sqlrooms
Length of output: 897
Remove invalid alt attribute from <video> element
The alt attribute is not defined in the HTML5 specification for <video> elements and will be silently ignored by browsers. Place accessible fallback content (descriptive text, captions, or a link) between the opening and closing <video> tags instead.
Proposed fix
-<video src="/media/examples/multi-room.mp4" alt="SQLRooms Multi-Room example" width="450" controls loop muted></video>
+<video src="/media/examples/multi-room.mp4" width="450" controls loop muted>
+ SQLRooms Multi-Room example (video)
+</video>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/multi-room/README.md` at line 5, Remove the invalid alt attribute
from the <video> element and instead add accessible fallback content between the
opening and closing <video> tags (e.g., a short descriptive text, captions, or a
link to the media) so assistive technologies and browsers without video support
can present a meaningful alternative; update the existing <video
src="/media/examples/multi-room.mp4" width="450" controls loop muted> element
accordingly.
| ```sh | ||
| pnpm install | ||
| pnpm dev |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm workspace setup and multi-room package name
echo "=== Checking pnpm-workspace.yaml ==="
cat pnpm-workspace.yaml 2>/dev/null || echo "No pnpm-workspace.yaml found"
echo -e "\n=== Checking multi-room package.json ==="
fd "package.json" examples/multi-room --max-depth 1 --exec cat {}
echo -e "\n=== Checking current README.md at lines 15-25 for context ==="
sed -n '15,25p' examples/multi-room/README.mdRepository: sqlrooms/sqlrooms
Length of output: 1319
Clarify where pnpm install should be run in monorepo context
The README doesn't specify that pnpm install must run from the repo root. In this workspace with workspace:* protocol dependencies, running from the example subdirectory will fail to resolve internal packages correctly.
✏️ Proposed fix
#### Running locally
```sh
+# From the repo root
pnpm install
-pnpm dev
+pnpm dev --filter sqlrooms-multi-room</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @examples/multi-room/README.md around lines 19 - 21, Update the README
instruction to specify that pnpm install must be run from the repository root
(so workspace:* dependencies resolve) and change the dev command to include the
workspace filter for the example (use pnpm dev --filter sqlrooms-multi-room);
ensure the README shows a comment like "From the repo root" before the install
command and replace the plain "pnpm dev" line with "pnpm dev --filter
sqlrooms-multi-room" so users run the example correctly in the monorepo context.
</details>
<!-- fingerprinting:phantom:triton:churro -->
<!-- This is an auto-generated comment by CodeRabbit -->
…Configs, localStorage - Replace IndexedDB with localStorage for room list + persistSliceConfigs per room - Use createRoomStoreCreator() with roomId parameter and storeKey for lifecycle - Use useRoomStore from factory in all components - Rely on sqlrooms built-in store init/destroy via storeKey mechanism - Clean up persist storage on room deletion - Fix StrictMode compatibility by not manually calling destroy
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
examples/multi-room/src/RoomPage.tsx (1)
123-126:⚠️ Potential issue | 🟡 MinorEscape table identifiers before composing SQL text.
selectedTableis interpolated directly into SQL. Table names containing"can break the query text.Proposed fix
function TableBrowser() { const initialized = useRoomStore((s) => s.room.initialized); const tables = useRoomStore((s) => s.db.tables); const [selectedTable, setSelectedTable] = useState<string | null>(null); + const quoteIdentifier = (name: string) => `"${name.replace(/"/g, '""')}"`; @@ {selectedTable ? ( <QueryDataTable key={selectedTable} - query={`SELECT * FROM "${selectedTable}"`} + query={`SELECT * FROM ${quoteIdentifier(selectedTable)}`} fontSize="text-xs" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/multi-room/src/RoomPage.tsx` around lines 123 - 126, The SQL is built by interpolating selectedTable directly into the query prop of QueryDataTable, which allows table names containing double quotes to break the SQL; before composing the query string in RoomPage (where selectedTable is used), escape the identifier by replacing any " characters with "" and then wrap the result in double quotes (i.e., produce a safe identifier) so the query prop receives a properly quoted table name; update the code that sets query={`SELECT * FROM "${selectedTable}"`} to use the escaped identifier logic for selectedTable before passing it into QueryDataTable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/multi-room/src/RoomPage.tsx`:
- Line 17: The useRef call for storeRef is invalid under React 19 because useRef
requires an initial value; update the declaration of storeRef (currently
useRef<StoreApi<RoomState>>()) to supply an initial value and include null in
the type to match nullable ref semantics (e.g., useRef<StoreApi<RoomState> |
null>(null)), so the ref is properly typed and initialized for RoomPage
components that reference StoreApi<RoomState>.
In `@examples/multi-room/src/rooms-list.ts`:
- Around line 28-30: addRoom currently appends a new RoomListEntry without
checking for duplicate id, causing getRoom to return a stale first match and
deleteRoom to remove all matches; update addRoom to guard against duplicates by
using getRoomsList() to check for an existing entry with the same id
(RoomListEntry.id) and either early-return if found or replace/deduplicate prior
to calling saveRoomsList([...]) so the saved list contains at most one entry per
id; reference addRoom, getRoomsList, saveRoomsList, getRoom, deleteRoom, and
RoomListEntry.id when making the change.
---
Duplicate comments:
In `@examples/multi-room/src/RoomPage.tsx`:
- Around line 123-126: The SQL is built by interpolating selectedTable directly
into the query prop of QueryDataTable, which allows table names containing
double quotes to break the SQL; before composing the query string in RoomPage
(where selectedTable is used), escape the identifier by replacing any "
characters with "" and then wrap the result in double quotes (i.e., produce a
safe identifier) so the query prop receives a properly quoted table name; update
the code that sets query={`SELECT * FROM "${selectedTable}"`} to use the escaped
identifier logic for selectedTable before passing it into QueryDataTable.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
docs/media/examples/multi-room.mp4is excluded by!**/*.mp4pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
examples/multi-room/package.jsonexamples/multi-room/src/RoomPage.tsxexamples/multi-room/src/RoomsListPage.tsxexamples/multi-room/src/main.tsxexamples/multi-room/src/room-store.tsexamples/multi-room/src/rooms-list.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- examples/multi-room/src/RoomsListPage.tsx
- examples/multi-room/package.json
- examples/multi-room/src/room-store.ts
- examples/multi-room/src/main.tsx
|
|
||
| function RoomLoader({roomId}: {roomId: string}) { | ||
| const roomConfig = getRoom(roomId); | ||
| const storeRef = useRef<StoreApi<RoomState>>(); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
React 19 @types/react useRef TypeScript signature overloads
💡 Result:
In React 19 (@types/[email protected]), the TypeScript overloads for useRef were changed to fix long-standing “mutable vs readonly ref” issues and to simplify inference:
What changed (vs React 18)
React 18 (common overload shape)
useRef had overloads that could return either MutableRefObject<T> or RefObject<T> depending on whether your initial value included null. This is what caused the classic surprise:
useRef<HTMLInputElement>(null)→ picked theT | nulloverload → returned a readonlyRefObject<HTMLInputElement>(soref.current = ...errored in TS)
(That overload-selection behavior is the root of many “why is current readonly?” questions.)
React 19 (new overload shape)
useRefnow requires an argument (no moreuseRef()), and there’s a codemod for it.- All refs are now mutable, and
MutableRef*is deprecated in favor of a singleRefObjecttype. - Convenience overloads exist for
nullandundefinedinitial values so you still get the rightT | null/T | undefinedincurrent. (react.dev)
Practical “signatures/overloads” to know in React 19
Conceptually, you should think of React 19 useRef as:
useRef<T>(initialValue: T) -> RefObject<T>(mutablecurrent)useRef<T>(null) -> RefObject<T | null>useRef<T>(undefined) -> RefObject<T | undefined>
And you must pass an argument, even if it’s undefined. (react.dev)
Migration / usage patterns
- DOM ref:
const elRef = useRef<HTMLDivElement | null>(null);(then check for null)
- “set later” value:
const idRef = useRef<number | undefined>(undefined);
- If you previously wrote
useRef<T>(), change touseRef<T | undefined>(undefined)or similar (codemod does this). (github.com)
Citations:
- 1: https://react.dev/blog/2024/04/25/react-19-upgrade-guide?utm_source=openai
- 2: https://react.dev/blog/2024/04/25/react-19-upgrade-guide?utm_source=openai
- 3: https://github.com/eps1lon/types-react-codemod?utm_source=openai
🏁 Script executed:
# Check the actual current state of RoomPage.tsx line 17
fd -t f "RoomPage.tsx" --exec cat -n {} \; | head -50Repository: sqlrooms/sqlrooms
Length of output: 1985
Fix invalid useRef initialization for React 19 (breaks build with TS2554).
In React 19, useRef requires an initial argument; calling useRef<T>() without one is invalid. Update line 17 to provide both the initial value and correct type:
- const storeRef = useRef<StoreApi<RoomState>>();
+ const storeRef = useRef<StoreApi<RoomState> | null>(null);🧰 Tools
🪛 GitHub Actions: Build examples
[error] 17-17: TS2554: TypeScript error: Expected 1 arguments, but got 0. (src/RoomPage.tsx:17,20)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/multi-room/src/RoomPage.tsx` at line 17, The useRef call for
storeRef is invalid under React 19 because useRef requires an initial value;
update the declaration of storeRef (currently useRef<StoreApi<RoomState>>()) to
supply an initial value and include null in the type to match nullable ref
semantics (e.g., useRef<StoreApi<RoomState> | null>(null)), so the ref is
properly typed and initialized for RoomPage components that reference
StoreApi<RoomState>.
| export function addRoom(room: RoomListEntry): void { | ||
| saveRoomsList([...getRoomsList(), room]); | ||
| } |
There was a problem hiding this comment.
addRoom does not guard against duplicate IDs.
If a room with the same id is added twice (e.g., after partial state corruption or a re-seed), getRoom silently returns the stale first entry while deleteRoom removes all matches. A simple early-exit or deduplication prevents this:
🛡️ Proposed fix
export function addRoom(room: RoomListEntry): void {
+ if (getRoomsList().some((r) => r.id === room.id)) return;
saveRoomsList([...getRoomsList(), room]);
}📝 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.
| export function addRoom(room: RoomListEntry): void { | |
| saveRoomsList([...getRoomsList(), room]); | |
| } | |
| export function addRoom(room: RoomListEntry): void { | |
| if (getRoomsList().some((r) => r.id === room.id)) return; | |
| saveRoomsList([...getRoomsList(), room]); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/multi-room/src/rooms-list.ts` around lines 28 - 30, addRoom
currently appends a new RoomListEntry without checking for duplicate id, causing
getRoom to return a stale first match and deleteRoom to remove all matches;
update addRoom to guard against duplicates by using getRoomsList() to check for
an existing entry with the same id (RoomListEntry.id) and either early-return if
found or replace/deduplicate prior to calling saveRoomsList([...]) so the saved
list contains at most one entry per id; reference addRoom, getRoomsList,
saveRoomsList, getRoom, deleteRoom, and RoomListEntry.id when making the change.
Signed-off-by: Ilya Boyandin <[email protected]>
Signed-off-by: Ilya Boyandin <[email protected]>
There was a problem hiding this comment.
♻️ Duplicate comments (6)
examples/multi-room/src/RoomPage.tsx (2)
17-17:useRefmissing-argument error addressed🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/multi-room/src/RoomPage.tsx` at line 17, The useRef declaration for storeRef currently uses useRef<StoreApi<RoomState>>(null) which causes a missing-argument/type error; update the generic to allow null (e.g. useRef<StoreApi<RoomState> | null>(null)) or initialize with undefined (useRef<StoreApi<RoomState> | undefined>(undefined)) so the type matches the initial value; adjust any downstream null/undefined checks where storeRef is accessed (storeRef.current) accordingly.
124-129: Table identifier not escaped in SQL query🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/multi-room/src/RoomPage.tsx` around lines 124 - 129, The SQL query built for QueryDataTable uses raw selectedTable which can lead to SQL injection; sanitize or validate the table identifier before constructing query. Ensure selectedTable (used in the QueryDataTable prop query) is either validated against a strict whitelist/regex (e.g. only letters, digits, underscores) or escaped by doubling any quote characters and then wrapped in the proper identifier quotes, and reject or refuse to run if it doesn't pass validation; update the RoomPage.tsx location that builds `query={`SELECT * FROM "${selectedTable}"`}` to perform this validation/escaping prior to passing the query prop.examples/multi-room/README.md (3)
5-5:altis not a valid attribute on<video>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/multi-room/README.md` at line 5, The README contains a <video> element using an invalid alt attribute; remove the alt attribute from the <video> tag in the README (examples/multi-room README's <video> element shown in diff) and, if descriptive text is required for accessibility, add an aria-label or aria-describedby on the <video> element or provide a nearby caption/figcaption or an <img> fallback with an alt attribute describing the video.
3-3:Github→GitHub🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/multi-room/README.md` at line 3, Change the link text from "Github repo" to "GitHub repo" in the README entry that currently reads "[Github repo](https://github.com/sqlrooms/sqlrooms/tree/main/examples/multi-room)"; update the capitalization only (GitHub) so the link becomes "[GitHub repo](https://github.com/sqlrooms/sqlrooms/tree/main/examples/multi-room)".
24-26:pnpm install/pnpm devneed repo-root context and a workspace filter🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/multi-room/README.md` around lines 24 - 26, Update the README entry for the multi-room example to note that the plain "pnpm install" and "pnpm dev" commands must be run from the repository root or invoked with a pnpm workspace filter; change the instructions to explicitly tell users to run dependency installation at the repo root and/or run the dev script targeted to the example workspace using pnpm's --filter workspace selector so the correct workspace context is used.examples/multi-room/src/RoomsListPage.tsx (1)
38-44: DuplicaterenameRoomwrite from blur + submit still unguarded🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/multi-room/src/RoomsListPage.tsx` around lines 38 - 44, confirmRename can be called twice (from blur and submit) causing duplicate renameRoom writes; introduce a guard (e.g., a useRef renameInProgress) checked at the top of confirmRename and set to true before calling renameRoom, then reset after the async operation (or after loadRooms) so subsequent blur/submit calls are no-ops; reference confirmRename, renameRoom, loadRooms, setEditingId, editingId, and editName when applying the guard.
🧹 Nitpick comments (1)
examples/multi-room/src/RoomsListPage.tsx (1)
90-98: Native<button>for room navigation is inconsistent with<Button>used elsewhereAll other interactive elements in the list use the
<Button>component from@sqlrooms/ui, but the room-name navigation control uses a raw<button>. This bypasses any shared styling, focus-ring, or variant tokens.♻️ Suggested fix
- <button - className="flex-1 text-left font-medium hover:underline" - onClick={() => - navigate({to: '/room/$id', params: {id: room.id}}) - } - > - {room.name} - </button> + <Button + variant="ghost" + size="sm" + className="flex-1 justify-start font-medium hover:underline" + onClick={() => + navigate({to: '/room/$id', params: {id: room.id}}) + } + > + {room.name} + </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/multi-room/src/RoomsListPage.tsx` around lines 90 - 98, Replace the raw HTML <button> in RoomsListPage (the onClick that calls navigate({to: '/room/$id', params: {id: room.id}}) and renders room.name) with the shared Button component from `@sqlrooms/ui` so it inherits styling, focus rings, and variants used elsewhere; keep the same onClick handler and className/props (e.g., "flex-1 text-left font-medium hover:underline") or map them to the Button's variant/size props, and ensure the Button renders room.name and calls navigate with room.id.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@examples/multi-room/README.md`:
- Line 5: The README contains a <video> element using an invalid alt attribute;
remove the alt attribute from the <video> tag in the README (examples/multi-room
README's <video> element shown in diff) and, if descriptive text is required for
accessibility, add an aria-label or aria-describedby on the <video> element or
provide a nearby caption/figcaption or an <img> fallback with an alt attribute
describing the video.
- Line 3: Change the link text from "Github repo" to "GitHub repo" in the README
entry that currently reads "[Github
repo](https://github.com/sqlrooms/sqlrooms/tree/main/examples/multi-room)";
update the capitalization only (GitHub) so the link becomes "[GitHub
repo](https://github.com/sqlrooms/sqlrooms/tree/main/examples/multi-room)".
- Around line 24-26: Update the README entry for the multi-room example to note
that the plain "pnpm install" and "pnpm dev" commands must be run from the
repository root or invoked with a pnpm workspace filter; change the instructions
to explicitly tell users to run dependency installation at the repo root and/or
run the dev script targeted to the example workspace using pnpm's --filter
workspace selector so the correct workspace context is used.
In `@examples/multi-room/src/RoomPage.tsx`:
- Line 17: The useRef declaration for storeRef currently uses
useRef<StoreApi<RoomState>>(null) which causes a missing-argument/type error;
update the generic to allow null (e.g. useRef<StoreApi<RoomState> | null>(null))
or initialize with undefined (useRef<StoreApi<RoomState> |
undefined>(undefined)) so the type matches the initial value; adjust any
downstream null/undefined checks where storeRef is accessed (storeRef.current)
accordingly.
- Around line 124-129: The SQL query built for QueryDataTable uses raw
selectedTable which can lead to SQL injection; sanitize or validate the table
identifier before constructing query. Ensure selectedTable (used in the
QueryDataTable prop query) is either validated against a strict whitelist/regex
(e.g. only letters, digits, underscores) or escaped by doubling any quote
characters and then wrapped in the proper identifier quotes, and reject or
refuse to run if it doesn't pass validation; update the RoomPage.tsx location
that builds `query={`SELECT * FROM "${selectedTable}"`}` to perform this
validation/escaping prior to passing the query prop.
In `@examples/multi-room/src/RoomsListPage.tsx`:
- Around line 38-44: confirmRename can be called twice (from blur and submit)
causing duplicate renameRoom writes; introduce a guard (e.g., a useRef
renameInProgress) checked at the top of confirmRename and set to true before
calling renameRoom, then reset after the async operation (or after loadRooms) so
subsequent blur/submit calls are no-ops; reference confirmRename, renameRoom,
loadRooms, setEditingId, editingId, and editName when applying the guard.
---
Nitpick comments:
In `@examples/multi-room/src/RoomsListPage.tsx`:
- Around line 90-98: Replace the raw HTML <button> in RoomsListPage (the onClick
that calls navigate({to: '/room/$id', params: {id: room.id}}) and renders
room.name) with the shared Button component from `@sqlrooms/ui` so it inherits
styling, focus rings, and variants used elsewhere; keep the same onClick handler
and className/props (e.g., "flex-1 text-left font-medium hover:underline") or
map them to the Button's variant/size props, and ensure the Button renders
room.name and calls navigate with room.id.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
examples/multi-room/README.mdexamples/multi-room/src/RoomPage.tsxexamples/multi-room/src/RoomsListPage.tsxexamples/multi-room/src/vite-env.d.ts
✅ Files skipped from review due to trivial changes (1)
- examples/multi-room/src/vite-env.d.ts
Add Cursor Cloud specific instructions to AGENTS.md to document the development environment setup and usage.
Summary by CodeRabbit
New Features
Documentation