perf: stop re-rendering every sidebar row on each cache update (F1) - #2348
Open
SawyerHood wants to merge 3 commits into
Open
perf: stop re-rendering every sidebar row on each cache update (F1)#2348SawyerHood wants to merge 3 commits into
SawyerHood wants to merge 3 commits into
Conversation
A single sidebar payload change rebuilt every prop the project rows and thread rows are memoized on: stripProjectThreads spread each project, ProjectModeSections regrouped threads into fresh per-project arrays and list states, areProjectRowPropsEqual compared those by identity, renderSectionDisplayOptions created a new headerActions element per call, and usePromptDraftInputThreadIds rebuilt its presence store (re-reading one localStorage key per thread) for every new threads array. Each layer is now stabilized independently: stripProjectThreads memoizes per payload object (WeakMap), retainProjectRows and the personal-threads memo reuse previous rows whose entries are identical, the ProjectRow comparator compares projects shallowly and thread lists element-wise (valid because React Query structurally shares the payload), useSectionDisplayOptionsRenderer caches the menu element per section until its open state changes, and the draft-presence subscription list keeps identity while its keys are unchanged. Verified: new/extended unit tests in ProjectRow.test.ts, ProjectList.rows.test.ts, project-queries.test.tsx, usePromptDraftStorage.test.tsx and ProjectList.modes.test.tsx (the stripProjectThreads identity and draft-presence retention tests fail against HEAD sources); ProjectRow.interactions, ThreadRow, ProjectListSectionHeader and the other tests importing these modules pass; turbo typecheck for @bb/app passes. Co-Authored-By: Claude <noreply@anthropic.com>
…ef caches Review of 980d1eec6 found three gaps. (1) Every ProjectRow still re-rendered on a sidebar cache patch: useSidebarReorderDnd passed inline option objects to dnd-kit's useSensor, which memoizes on the options, so useSensors produced a new sensors array per render, DndContext rebuilt its activators, every useSortable handed out fresh listeners, and each row's projectDragBindings failed the memo comparator. (2) The render-time ref caches inside ProjectModeSections' projectRows/personalThreads useMemos made the React Compiler bail out of the whole component; useSectionDisplayOptionsRenderer also bailed on the ??= operator. (3) No test exercised the comparator's nested-identity branch (sources) and nothing asserted an end-to-end render count. The sensor options are now module constants and SidebarSectionOrderList memoizes its items copy so the SortableContext value stops churning. The ref caches moved into useRetainedProjectRows and useRetainedThreadList (the useSidebarThreadTitleMentionResources pattern) and the ??= became an explicit null check, so the compiler bailout is confined to the two small hooks. ProjectList.render-counts.test.tsx mounts the real ProjectList on a seeded query cache with ProjectRow wrapped in a counting memo that uses the real areProjectRowPropsEqual, patches one thread through applyToCachedSidebarNavigationThreads, and asserts only that project's row re-rendered (plus: a deep-equal refetch renders nothing, a B-only patch re-renders B only, and the hook keeps its sensors across re-renders). ProjectRow.test.ts gains the nested sources-replaced case. Verified: the render-count test fails with the sensor hoist reverted (rows a:3 b:3 c:3 instead of a:3 b:2 c:2, sensors identity changes) and passes with it; a throwaway babel-plugin-react-compiler transform logs CompileSuccess for ProjectModeSections and useSectionDisplayOptionsRenderer with only useRetainedProjectRows/useRetainedThreadList bailing; turbo typecheck for @bb/app passes; ProjectRow, ProjectList.rows, ProjectList.modes, useSidebarReorderDnd, ProjectRow.interactions, PinnedThreadTree, useSectionThreadDnd and PluginNavSidebarItems suites pass. Co-Authored-By: Claude <noreply@anthropic.com>
…suite
Round-2 review of c31911738 found the unit guard for the dnd-kit sensors regression (useSensor memoizes on its options object, so inline literals hand DndContext a fresh sensors array per render) sitting in a describe("useSidebarReorderDnd") block inside ProjectList.render-counts.test.tsx. Anyone editing useSidebarReorderDnd.ts runs the co-located useSidebarReorderDnd.test.tsx, which stayed green against a re-inlined hook; the regression only surfaced from a ProjectList-named file when the full @bb/app suite ran.
The case now lives as a third it() in the existing describe("useSidebarReorderDnd") block of useSidebarReorderDnd.test.tsx, with its comment naming ProjectList.render-counts.test.tsx as the end-to-end layer that catches the same thing. The render-counts file keeps only its end-to-end assertion and drops the renderHook and useSidebarReorderDnd imports it no longer uses.
Verified: with the pre-fix hook (c31911738~1, inline useSensor options) swapped in, useSidebarReorderDnd.test.tsx now fails only on the moved case (expected [...] to be [...] // Object.is equality on dndContextProps.sensors; 1 failed, 4 passed) and passes with the hook restored byte-identically; both test files pass (2 files, 6 tests); turbo typecheck for @bb/app passes; oxlint and oxfmt are clean.
Co-Authored-By: Claude <noreply@anthropic.com>
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.
What was wrong
Every sidebar cache update re-rendered all project and thread rows (an internal performance sweep, finding F1): ~1,500–1,650 component renders per realtime status change, 215–263 ms of main thread at 4× CPU. Five layers defeated memoization:
stripProjectThreadsspread every project into a new object per payload,threadsByProject/getProjectThreadListStatereturned fresh literals,areProjectRowPropsEqualcompared by identity,headerActionswas a new element per render, andusePromptDraftInputThreadIdsrebuilt its localStorage subscriptions on every newthreadsarray. A sixth, pre-existing one surfaced in review:useSidebarReorderDndpassed inline option objects to dnd-kit'suseSensor, so every row's drag bindings changed identity on each render.What changed
apps/app/src/hooks/queries/project-queries.ts,components/sidebar/ProjectList.tsx,ProjectRow.tsx,hooks/usePromptDraftStorage.ts,components/sidebar/useSidebarReorderDnd.ts,SidebarSectionOrderList.tsx:stripProjectThreadsis memoized per payload object (moduleWeakMap), so React Query's structural sharing carries through.retainProjectRowskeeps a project's row model, thread-list state andthreadsarray when its entries are unchanged; the personal-project list gets the same element-wise retention. The render-time ref caches live in small hooks (useRetainedProjectRows,useRetainedThreadList) so the React Compiler still compilesProjectModeSections.ProjectRowcomparesprojectshallowly andthreadListStateby value (areProjectThreadListStatesEqual,areThreadListsEqual); this relies on React Query structural sharing and on no cache owner mutating entries in place (documented in the tests).useSensors, activators and every row'sdragBindingskeep identity.No server, daemon, CLI or plugin API changes.
How you verified
New/extended tests, each failing on the pre-fix code:
ProjectRow.test.ts(comparators incl. nestedsourcesreplacement),ProjectList.rows.test.ts(retainProjectRows),project-queries.test.tsx,usePromptDraftStorage.test.tsx(no re-read on a same-content array),ProjectList.modes.test.tsx(menu element cache),useSidebarReorderDnd.test.tsx(sensor identity), and an end-to-endProjectList.render-counts.test.tsxthat mounts the realProjectListon a seededQueryClient, patches one thread throughapplyToCachedSidebarNavigationThreads, and asserts only that project'sProjectRowre-renders (it fails when the sensor hoist is reverted). A throwawaybabel-plugin-react-compilertransform confirmedProjectModeSectionscompiles again.On this branch:
pnpm exec turbo run typecheck(76/76),pnpm exec turbo run lint,pnpm exec turbo run test(all packages green). Suggested manual check before merge: browse the sidebar with running threads, pin/unpin, collapse, drag-reorder.Fixes: no issue — source is an internal performance sweep (finding F1).