Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
kianbazza
force-pushed
the
ui-651-register-menu-rows-and-items-once-per-commit
branch
from
October 10, 2026 23:33
a24dafe to
a5a1030
Compare
This branch was successfully deployed
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
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
Each row re-sorted every row by DOM position when it mounted, and each item re-scored every item. When hundreds of rows mount together (opening a long menu, or a Backspace that clears the search), that is a full
compareDocumentPositionsort and a fullcommandScorepass per row.Registrations now queue, and the store applies each batch at once: rows that mount in a commit are sorted once, and items that mount together are re-scored once.
Who calls
flushPendingRegistrations():List's layout effect. A parent's layout effects run after its children's, so it sees every row registered in the commit.registrationBatchcounter, whichListsubscribes to. The first registration of a batch bumps it, so a commitListwasn't part of (an item's passive effect, a row that toggles on its own) re-rendersListin the same update. Publishing stays inside React's update; jsdomact(...)warnings are unchanged.getVisibleItemIds,selectByShortcut, andvalidateHighlight, before they read registration state.Evidence
row-renders.test.tsxmounts 20 items in one commit and counts how often the store publishes the row order and the filter scores, right afterrender:expected { rowOrder: 20, scores: 20 } to deeply equal { rowOrder: 1, scores: 1 }After: passes. With
List's flush removed it fails with{ rowOrder: 0, scores: 0 }, so it checks the wiring, not the microtask.bun run bench:browser -- --rows=1000 --throttle=1,4 --loops=10 --per-key, p50 in ms:The "all rows match" phases don't change (no rows register on those keys); they move between ~13 and ~29 ms from run to run on either branch.
Merge Danger
Door: two-way
Blast Radius: medium
Every menu, select, and combobox registers through this store.
registerRoworregisterItemon an exported store directly and readsstate.orderedRows,filteredItems, orfilteredCountright after now sees the old value untilflushPendingRegistrations()(or the next microtask). Inside components,Listpublishes before paint.filter={false}, if several items register in the same commit in an order different fromorderedItems, auto-highlight picks the first inorderedItems. It used to pick whichever registered first. Items registering in separate commits behave as before.*StoreStatetypes gainregistrationBatch.Closes UI-651