fix: isLoading regression and getEffectiveBindingsMap keystroke caching#708
Draft
leoisadev1 wants to merge 1 commit intocursor/code-standards-compliance-d243from
Draft
Conversation
Issue 1: Derive isLoading from cache state so it is true when models haven't loaded yet and no error exists, fixing the brief false flash before the mount effect starts fetching. Issue 2: Memoize getEffectiveBindingsMap via useMemo and expose through a ref so the keydown handler reads a cached map instead of recomputing on every keystroke. Co-authored-by: Leo <leoisadev1@users.noreply.github.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
🚀 Preview Deployment ReadyVercel is rebuilding the frontend with the new Convex backend URL. Vercel will post the preview URL automatically. Convex Preview Backend
🤖 Deployed automatically by GitHub Actions |
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.
Fixes two regressions introduced in PR #707 (
cursor/code-standards-compliance-d243).Issue 1 —
isLoadingregression inuseModelshookProblem: After the module-cache refactor,
cache.loadinginitializes asfalse. On the first render ofuseModels(),isLoadingreadsfalseeven though no models have been fetched yet. There's a brief frame whereisLoadingis incorrectlyfalsebefore the mount effect starts fetching and flips it totrue.Fix: Derive
isLoadingfrom the full cache state:This ensures
isLoadingistruewhenever models haven't loaded yet and no error has occurred, covering the initial render beforefetchAllModels()kicks in.Issue 2 —
getEffectiveBindingsMaprecomputed on every keystrokeProblem: The
onKeyDownhandler insideuseMountEffectcalledgetEffectiveBindingsMap(bindingsRef.current, isMacRef.current)on every keystroke, creating a newMapeach time. While not expensive per-call, it's unnecessary work since bindings only change when the user modifies shortcut settings.Fix: Memoize the binding map with
useMemoand expose it through a ref:The
onKeyDownhandler readsbindingMapRef.currentinstead of recomputing. The map is only rebuilt whenbindingsorisMacchanges. Removed the now-unnecessarybindingsRefandisMacRef.Verification
bun check— passes (no new lint warnings)bun run test— all tests passbun check-types— no type errors in changed files (pre-existing errors in unrelated files)Note
Fix
isLoadingregression and memoizegetEffectiveBindingsMapin keydown handlerisLoadinginuseModelsto returntruewhen bothcache.modelsandcache.errorarenull, covering the initial indeterminate state that was previously not handled.useGlobalShortcutswith auseMemo-derived map stored in a ref, so the map is only recomputed whenbindingsorisMacchanges.Macroscope summarized 9b1cb06.