Repository navigation
Virtualize bookmarks list with react-virtualized - #172
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
764c32a to
8826a94
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 764c32a09e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const userId = (await getUserId()) ?? crypto.randomUUID(); | ||
|
|
||
| try { | ||
| await setUserTheme(userId, theme); |
There was a problem hiding this comment.
A non-browser client can forge the allowed Origin header and omit cookies on every request, causing each public POST /theme call to generate a fresh UUID and insert another users row. An attacker can therefore grow the production table without bound; apply rate limiting or another server-side abuse control before creating anonymous users.
Useful? React with 👍 / 👎.
| if (isTheme(serverTheme) && serverTheme !== window.__theme) { | ||
| dispatch(updateCurrentTheme(serverTheme)); | ||
| window.__setPreferredTheme(serverTheme); |
There was a problem hiding this comment.
When a user with an existing uid changes themes while the POST fails transiently, __setPreferredTheme has already updated the readable cookie but persistTheme ignores the failure. On the next load, a recovered GET returns the old database value and this block overwrites Redux, the window state, and the cookie, losing the user's selection instead of degrading to the cookie as intended. Avoid treating the database value as authoritative when a valid local choice may be newer, or track and retry failed writes.
AGENTS.md reference: AGENTS.md:L79-L82
Useful? React with 👍 / 👎.
| dispatch(updateCurrentTheme(newTheme)); | ||
| window.__setPreferredTheme(newTheme); | ||
| persistTheme(newTheme); |
There was a problem hiding this comment.
If a new visitor toggles twice before the first POST response installs the uid cookie, these fire-and-forget calls both arrive without an identity and create different users. Response order, rather than click order, then determines the final uid and theme cookies, so a reload can show the wrong theme and an orphan row remains; queue writes or prevent another toggle until the initial identity-setting request completes.
AGENTS.md reference: AGENTS.md:L78-L81
Useful? React with 👍 / 👎.
| - "${POSTGRES_PORT:-5432}:5432" | ||
| volumes: | ||
| - pgdata:/var/lib/postgresql/data | ||
| - ./db/init:/docker-entrypoint-initdb.d:ro |
There was a problem hiding this comment.
When DATABASE_URL points to a fresh managed or production database, configuring the URL alone does not create the users table: the only execution path for this SQL is the local Compose initialization mount, and neither the build/start scripts nor a migration command applies it elsewhere. Consequently every theme query fails with a missing-relation error and the route returns 503 until the schema is manually installed; add a deployment migration or explicit production provisioning step.
AGENTS.md reference: AGENTS.md:L28-L28
Useful? React with 👍 / 👎.
Summary
react-virtualized(9.22.6, React 19 compatible) and@types/react-virtualizedapp/bookmarks/BookmarksList.tsx, which renders it withWindowScroller+AutoSizer+Listso it scrolls with the page and only keeps the visible rows (plus overscan) in the DOMCellMeasurermeasures them and the cache is cleared whenever the width changesuseSyncExternalStore) to avoid a hydration mismatch. The server and pre-hydration render show the first 50 bookmarks with the same markupNotes
/bookmarks/atomrole="list"/role="listitem"divs instead oful/li, since react-virtualized renders wrapper divsTest plan
eslintandtsc --noEmitpassmake build(not run locally)🤖 Generated with Claude Code