From 5d96ea59a03fa62351ece848285a21e3507a5cf1 Mon Sep 17 00:00:00 2001 From: PRASSamin Date: Tue, 15 Sep 2026 15:40:22 +0600 Subject: [PATCH] fix(react): make disabled a registration gate and warn on duplicate shortcuts --- packages/core/CHANGELOG.md | 7 + packages/core/package.json | 2 +- packages/core/src/ShortcutManager.test.ts | 58 +++++- packages/core/src/ShortcutManager.ts | 36 ++++ packages/react/CHANGELOG.md | 7 + packages/react/README.md | 214 +++++++++++++------- packages/react/package.json | 2 +- packages/react/src/Keybindy.test.tsx | 44 ++-- packages/react/src/disabled-gating.test.tsx | 185 +++++++++++++++++ packages/react/src/useShortcuts.ts | 118 ++++++++--- tests/sample-react/src/App.jsx | 80 +++++++- 11 files changed, 625 insertions(+), 128 deletions(-) create mode 100644 packages/react/src/disabled-gating.test.tsx diff --git a/packages/core/CHANGELOG.md b/packages/core/CHANGELOG.md index 9221dbf..6d9a2f1 100644 --- a/packages/core/CHANGELOG.md +++ b/packages/core/CHANGELOG.md @@ -1,5 +1,12 @@ # @keybindy/core +## [2.0.2] - 2026-09-15 + +### Improvements & Fixes + +- `register()` now logs a development warning when a registration overwrites an existing binding for the same keys and scope, instead of replacing it silently. + + ## [2.0.1] - 2026-08-30 ### Improvements & Fixes diff --git a/packages/core/package.json b/packages/core/package.json index 4cf894d..9a4928f 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -1,6 +1,6 @@ { "name": "@keybindy/core", - "version": "2.0.1", + "version": "2.0.2", "description": "A lightweight and framework-agnostic keyboard shortcut manager for web apps. Define, register, and handle keybindings with ease.", "author": { "name": "PRASSamin", diff --git a/packages/core/src/ShortcutManager.test.ts b/packages/core/src/ShortcutManager.test.ts index 89a5b46..5e7b219 100644 --- a/packages/core/src/ShortcutManager.test.ts +++ b/packages/core/src/ShortcutManager.test.ts @@ -632,14 +632,20 @@ describe('ShortcutManager', () => { manager.register(['B'], globalHandler, { scope: 'global' }); // Scoped hook only for canvas - manager.beforeEach(() => { - canvasOrder.push('canvas-before'); - }, { scope: 'canvas' }); + manager.beforeEach( + () => { + canvasOrder.push('canvas-before'); + }, + { scope: 'canvas' } + ); // Key-filtered hook only for 'B' - manager.beforeEach(() => { - globalOrder.push('b-before'); - }, { keys: ['B'] }); + manager.beforeEach( + () => { + globalOrder.push('b-before'); + }, + { keys: ['B'] } + ); // Trigger 'B' (global) manager.setActiveScope('global'); @@ -697,12 +703,50 @@ describe('ShortcutManager', () => { expect(dgBeforeHook).not.toHaveBeenCalled(); }); }); -}); + describe('duplicate registration', () => { + it('should warn when a binding is overwritten in the same scope', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + manager.register(['A'], vi.fn()); + manager.register(['A'], vi.fn()); + + const warnings = warnSpy.mock.calls.filter(([msg]) => + String(msg).includes('was overwritten') + ); + expect(warnings).toHaveLength(1); + + warnSpy.mockRestore(); + }); + + it('should not warn when the same combo is registered in different scopes', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + manager.register(['A'], vi.fn(), { scope: 'global' }); + manager.register(['A'], vi.fn(), { scope: 'editor' }); + const warnings = warnSpy.mock.calls.filter(([msg]) => + String(msg).includes('was overwritten') + ); + expect(warnings).toHaveLength(0); + warnSpy.mockRestore(); + }); + it('should only fire the most recently registered handler', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const first = vi.fn(); + const second = vi.fn(); + manager.register(['A'], first); + manager.register(['A'], second); + dispatchKeyEvent('keydown', 'KeyA'); + expect(first).not.toHaveBeenCalled(); + expect(second).toHaveBeenCalledOnce(); + warnSpy.mockRestore(); + }); + }); +}); diff --git a/packages/core/src/ShortcutManager.ts b/packages/core/src/ShortcutManager.ts index f36dda2..3db8461 100644 --- a/packages/core/src/ShortcutManager.ts +++ b/packages/core/src/ShortcutManager.ts @@ -17,6 +17,19 @@ import { ScopeManager } from './ScopeManager'; import { EventEmitter } from './utils/eventemitter'; import { Logger } from './utils/log'; +/** + * Whether the current build is a production bundle. + * Written so bundlers (Next.js, Vite, webpack) can statically replace `process.env.NODE_ENV`, + * while plain browser environments without a `process` shim simply fall back to `false`. + */ +const isProductionBuild = (() => { + try { + return process.env.NODE_ENV === 'production'; + } catch { + return false; + } +})(); + /** * Manages keyboard shortcuts with support for scopes, enabling/disabling, * dynamic registration, and cheat sheet generation. @@ -25,6 +38,7 @@ export class ShortcutManager extends ScopeManager { private shortcuts: Shortcut[] = []; private pressedKeys = new Set(); private activeHoldShortcuts = new Set(); + private warnedDuplicates = new Set(); private typingEmitter = new EventEmitter<{ key: string; event: KeyboardEvent; @@ -513,6 +527,27 @@ export class ShortcutManager extends ScopeManager { for (const combo of expandedCombos) { const normalized = combo.map(k => k.toLowerCase() as Keys); + const replaced = this.shortcuts.filter( + s => + JSON.stringify(s.keys) === JSON.stringify(normalized) && + (s.options?.scope || 'global') === targetScope && + s.id !== id + ); + + const duplicateSignature = `${targetScope}::${JSON.stringify(normalized)}`; + if ( + replaced.length > 0 && + !this.warnedDuplicates.has(duplicateSignature) && + !isProductionBuild + ) { + this.warnedDuplicates.add(duplicateSignature); + console.warn( + `[Keybindy] Duplicate binding ${JSON.stringify(normalized)} in scope "${targetScope}" was overwritten. ` + + `The previous registration has been replaced and will never fire. ` + + `If this is a repeated component instance, give it its own scope or avoid registering while inactive.` + ); + } + this.shortcuts = this.shortcuts.filter( s => JSON.stringify(s.keys) !== JSON.stringify(normalized) || @@ -683,6 +718,7 @@ export class ShortcutManager extends ScopeManager { this.resetScope(); this.activeSequences = []; this.activeHoldShortcuts.clear(); + this.warnedDuplicates.clear(); this.logger.log('Instance destroyed'); } diff --git a/packages/react/CHANGELOG.md b/packages/react/CHANGELOG.md index d904093..c2af06f 100644 --- a/packages/react/CHANGELOG.md +++ b/packages/react/CHANGELOG.md @@ -1,5 +1,12 @@ # @keybindy/react +## [2.0.2] - 2026-09-15 + +### Improvements & Fixes + +- **`disabled` no longer disables sibling shortcuts.** When a hook is disabled it now registers nothing and never claims the active scope, so `{ disabled: !isOpen }` is the recommended way to gate dialogs, pickers, and other parallel component instances — an inactive copy can no longer steal keys from the active one, and unmounting it leaves other bindings intact. +- **Development warning for duplicate registrations** — when two hooks register the same keys in the same scope, a single actionable warning is printed instead of failing silently. The warning re-arms once all colliding registrations unmount. + ## [2.0.1] - 2026-08-30 ### Improvements & Fixes diff --git a/packages/react/README.md b/packages/react/README.md index 43d4d4a..50bf6f3 100644 --- a/packages/react/README.md +++ b/packages/react/README.md @@ -15,6 +15,7 @@ --- Most React keyboard shortcut hooks fail in subtle, frustrating ways: + 1. **The "Blinking" Problem**: Every state change re-renders the component, causing the hook to unregister and re-register the hotkey. Rapid typing or animations cause micro-gaps where shortcuts are dropped. 2. **Stale Closures**: Forgetting to update dependency arrays traps shortcuts with initial state values. 3. **Modal Leaks**: Closing a dialog forgets to re-enable background shortcuts or corrupts the active scope. @@ -62,17 +63,28 @@ function DocumentEditor() { const [content, setContent] = useState(''); // ⚡️ Always accesses the latest `content` state without re-registering! - useShortcut(['Ctrl', 'S'], (event) => { - saveDocument(content); - }, { - preventDefault: true, - ignoreInputs: true, // Won't trigger if user is typing in a textarea - }); + useShortcut( + ['Ctrl', 'S'], + event => { + saveDocument(content); + }, + { + preventDefault: true, + ignoreInputs: true, // Won't trigger if user is typing in a textarea + } + ); // Cross-platform Command/Ctrl + K - useShortcut([['Meta', 'K'], ['Ctrl', 'K']], () => { - openSearchPalette(); - }, { preventDefault: true }); + useShortcut( + [ + ['Meta', 'K'], + ['Ctrl', 'K'], + ], + () => { + openSearchPalette(); + }, + { preventDefault: true } + ); return + +
+

Instance mode: duplicate modals + Enter

+

+ Increment a counter, press Enter. The log must show the counter of + the modal you interacted with. +

+ + +
+ +
+

Duplicate warning: non-instance hooks

+

+ Two mounted global hooks bind Q without instance mode. Watch the + console for one "Duplicate shortcut detected" warning. +

+ + +
); @@ -130,6 +150,64 @@ function App() { export default App; +const InstanceModal = ({ label }) => { + const [isOpen, setIsOpen] = useState(false); + const [counter, setCounter] = useState(0); + + useShortcuts( + [ + { + keys: ['Enter'], + handler: () => { + console.log(`${label} fired — counter: ${counter}`); + }, + options: { + preventDefault: true, + }, + }, + ], + { + // disabled: !isOpen, + } + ); + + return ( +
+ + {isOpen && ( +
+

+ {label} counter: {counter} +

+ + +
+ )} +
+ ); +}; + +const NonInstanceDuplicate = () => { + useShortcuts([ + { + keys: ['Q'], + handler: () => console.log('Q pressed'), + }, + ]); + + return null; +}; + const Modal = ({ setIsModalOpen }) => { return ( { console.log("x pressed from Modal") } }]}>