Repository navigation
Conversation
Two bugs in the post-rewrite hotkey migration (#3178): 1. The "already migrated" guard checked for the `hotkey-overrides` localStorage key, but Zustand's persist middleware doesn't write it until the store mutates. Users who never customized anything re-ran the migration (and re-hit the legacy tRPC endpoint) every launch. Replaced with a dedicated `hotkey-overrides-migrated` sentinel set in every terminal branch. 2. The new registry declares punctuation via code-based names (`bracketleft`, `comma`, `slash`, …) because react-hotkeys-hook matches on `KeyboardEvent.code`. The old store held the literal character ("["). The migration copied overrides verbatim, so anything bound to `[ ] , . / \ ; ' backtick - =` silently stopped working after upgrade. Added a token-level translation table. Adds migrate.test.ts covering both fixes plus the existing branches.
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Contributor
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
This branch was previously 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
Two bugs in the post-rewrite hotkey migration shipped in #3178:
hotkey-overrideslocalStorage key, but Zustand'spersistmiddleware doesn't write that key until the store mutates. Users who never customized anything kept hitting the legacy tRPC endpoint forever. Replaced with a dedicatedhotkey-overrides-migratedsentinel written in every terminal branch (success, no-op, error).bracketleft,bracketright,comma,slash, …) because react-hotkeys-hook matches onKeyboardEvent.code. The old store held the literal character ("["). The migration copied overrides verbatim, so any user-customized binding using[ ] , . / \ ; '- =` was migrated into the new store but never fired. Added a token-level translation table applied before write.Test plan
bun test src/renderer/hotkeys/migrate.test.ts— 6/6 pass, including a multi-call test that asserts the sentinel prevents repeat tRPC queries, and a translation test that assertsmeta+[→meta+bracketleft.bun run typecheckclean inapps/desktop.~/.superset/app-state.jsonwithhotkeysState.byPlatform.darwinoverrides that include punctuation, clearlocalStorage["hotkey-overrides"]andlocalStorage["hotkey-overrides-migrated"], reload, confirm bindings fire.Summary by cubic
Fixes the desktop hotkey migration so it runs once and preserves punctuation-based shortcuts by translating them to the new code-based names. Prevents repeated legacy tRPC calls and ensures custom bindings like meta+[ still work.
hotkey-overrides-migratedsentinel to stop re-running the migration on every launch.[→bracketleft).Written for commit 4f74b03. Summary will update on new commits.