Feat conflict detection #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open

solssak wants to merge 3 commits into TanStack:main from solssak:feat-conflict-detection

.changeset/conflict-detection.md

-Original file line number
+Diff line change
@@ -0,0 +1,14 @@
+    ---
+    '@tanstack/keys': minor
+    ---
+    Add hotkey conflict detection with configurable behavior
+    Implements conflict detection when registering hotkeys with the same combination on the same target. Adds a new `conflictBehavior` option to `HotkeyOptions`:
+    - `'warn'` (default) - Log a warning to console but allow both registrations
+    - `'error'` - Throw an error and prevent the new registration
+    - `'replace'` - Unregister the existing hotkey and register the new one
+    - `'allow'` - Allow multiple registrations without warning
+    This addresses the "Warn/error on conflicting shortcuts (TBD)" item from the README.

docs/framework/react/reference/functions/useHotkeyRecorder.md

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -9,7 +9,7 @@ title: useHotkeyRecorder
  
    function useHotkeyRecorder(options): ReactHotkeyRecorder;

    ```

    Defined in: [useHotkeyRecorder.ts:60](https://github.com/TanStack/keys/blob/main/packages/react-keys/src/useHotkeyRecorder.ts#L60)

    Defined in: [useHotkeyRecorder.ts:57](https://github.com/TanStack/keys/blob/main/packages/react-keys/src/useHotkeyRecorder.ts#L57)

    React hook for recording keyboard shortcuts.

docs/framework/react/reference/interfaces/ReactHotkeyRecorder.md

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -5,7 +5,7 @@ title: ReactHotkeyRecorder
  
    # Interface: ReactHotkeyRecorder

    Defined in: [useHotkeyRecorder.ts:9](https://github.com/TanStack/keys/blob/main/packages/react-keys/src/useHotkeyRecorder.ts#L9)

    Defined in: [useHotkeyRecorder.ts:6](https://github.com/TanStack/keys/blob/main/packages/react-keys/src/useHotkeyRecorder.ts#L6)

    ## Properties

    @@ -15,7 +15,7 @@ Defined in: [useHotkeyRecorder.ts:9](https://github.com/TanStack/keys/blob/main/
  
    cancelRecording: () => void;

    ```

    Defined in: [useHotkeyRecorder.ts:19](https://github.com/TanStack/keys/blob/main/packages/react-keys/src/useHotkeyRecorder.ts#L19)

    Defined in: [useHotkeyRecorder.ts:16](https://github.com/TanStack/keys/blob/main/packages/react-keys/src/useHotkeyRecorder.ts#L16)

    Cancel recording without saving

    @@ -31,7 +31,7 @@ Cancel recording without saving
  
    isRecording: boolean;

    ```

    Defined in: [useHotkeyRecorder.ts:11](https://github.com/TanStack/keys/blob/main/packages/react-keys/src/useHotkeyRecorder.ts#L11)

    Defined in: [useHotkeyRecorder.ts:8](https://github.com/TanStack/keys/blob/main/packages/react-keys/src/useHotkeyRecorder.ts#L8)

    Whether recording is currently active

    @@ -43,7 +43,7 @@ Whether recording is currently active
  
    recordedHotkey: Hotkey | null;

    ```

    Defined in: [useHotkeyRecorder.ts:13](https://github.com/TanStack/keys/blob/main/packages/react-keys/src/useHotkeyRecorder.ts#L13)

    Defined in: [useHotkeyRecorder.ts:10](https://github.com/TanStack/keys/blob/main/packages/react-keys/src/useHotkeyRecorder.ts#L10)

    The currently recorded hotkey (for live preview)

    @@ -55,7 +55,7 @@ The currently recorded hotkey (for live preview)
  
    startRecording: () => void;

    ```

    Defined in: [useHotkeyRecorder.ts:15](https://github.com/TanStack/keys/blob/main/packages/react-keys/src/useHotkeyRecorder.ts#L15)

    Defined in: [useHotkeyRecorder.ts:12](https://github.com/TanStack/keys/blob/main/packages/react-keys/src/useHotkeyRecorder.ts#L12)

    Start recording a new hotkey

    @@ -71,7 +71,7 @@ Start recording a new hotkey
  
    stopRecording: () => void;

    ```

    Defined in: [useHotkeyRecorder.ts:17](https://github.com/TanStack/keys/blob/main/packages/react-keys/src/useHotkeyRecorder.ts#L17)

    Defined in: [useHotkeyRecorder.ts:14](https://github.com/TanStack/keys/blob/main/packages/react-keys/src/useHotkeyRecorder.ts#L14)

    Stop recording (same as cancel)

docs/reference/classes/HotkeyManager.md

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -34,7 +34,7 @@ unregister()
  
    destroy(): void;

    ```

    Defined in: [manager.ts:526](https://github.com/TanStack/keys/blob/main/packages/keys/src/manager.ts#L526)

    Defined in: [manager.ts:520](https://github.com/TanStack/keys/blob/main/packages/keys/src/manager.ts#L520)

    Destroys the manager and removes all listeners.

    @@ -50,7 +50,7 @@ Destroys the manager and removes all listeners.
  
    getRegistrationCount(): number;

    ```

    Defined in: [manager.ts:497](https://github.com/TanStack/keys/blob/main/packages/keys/src/manager.ts#L497)

    Defined in: [manager.ts:491](https://github.com/TanStack/keys/blob/main/packages/keys/src/manager.ts#L491)

    Gets the number of registered hotkeys.

    @@ -66,7 +66,7 @@ Gets the number of registered hotkeys.
  
    isRegistered(hotkey, target?): boolean;

    ```

    Defined in: [manager.ts:508](https://github.com/TanStack/keys/blob/main/packages/keys/src/manager.ts#L508)

    Defined in: [manager.ts:502](https://github.com/TanStack/keys/blob/main/packages/keys/src/manager.ts#L502)

    Checks if a specific hotkey is registered.

    @@ -101,7 +101,7 @@ register(
  
       options): HotkeyRegistrationHandle;

    ```

    Defined in: [manager.ts:122](https://github.com/TanStack/keys/blob/main/packages/keys/src/manager.ts#L122)

    Defined in: [manager.ts:120](https://github.com/TanStack/keys/blob/main/packages/keys/src/manager.ts#L120)

    Registers a hotkey handler and returns a handle for updating the registration.

    @@ -157,7 +157,7 @@ handle.unregister()
  
    static getInstance(): HotkeyManager;

    ```

    Defined in: [manager.ts:80](https://github.com/TanStack/keys/blob/main/packages/keys/src/manager.ts#L80)

    Defined in: [manager.ts:78](https://github.com/TanStack/keys/blob/main/packages/keys/src/manager.ts#L78)

    Gets the singleton instance of HotkeyManager.

    @@ -173,7 +173,7 @@ Gets the singleton instance of HotkeyManager.
  
    static resetInstance(): void;

    ```

    Defined in: [manager.ts:90](https://github.com/TanStack/keys/blob/main/packages/keys/src/manager.ts#L90)

    Defined in: [manager.ts:88](https://github.com/TanStack/keys/blob/main/packages/keys/src/manager.ts#L88)

    Resets the singleton instance. Useful for testing.

docs/reference/classes/HotkeyRecorder.md

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -87,7 +87,7 @@ Use this to subscribe to state changes or access current state.
  
    cancel(): void;

    ```

    Defined in: [recorder.ts:212](https://github.com/TanStack/keys/blob/main/packages/keys/src/recorder.ts#L212)

    Defined in: [recorder.ts:214](https://github.com/TanStack/keys/blob/main/packages/keys/src/recorder.ts#L214)

    Cancel recording without saving.

    @@ -106,7 +106,7 @@ the onCancel callback if provided.
  
    destroy(): void;

    ```

    Defined in: [recorder.ts:257](https://github.com/TanStack/keys/blob/main/packages/keys/src/recorder.ts#L257)

    Defined in: [recorder.ts:259](https://github.com/TanStack/keys/blob/main/packages/keys/src/recorder.ts#L259)

    Clean up event listeners and reset state.

    @@ -168,7 +168,7 @@ a valid hotkey is recorded, Escape is pressed, or stop/cancel is called.
  
    stop(): void;

    ```

    Defined in: [recorder.ts:192](https://github.com/TanStack/keys/blob/main/packages/keys/src/recorder.ts#L192)

    Defined in: [recorder.ts:194](https://github.com/TanStack/keys/blob/main/packages/keys/src/recorder.ts#L194)

    Stop recording (same as cancel, but doesn't call onCancel).

docs/reference/functions/getHotkeyManager.md

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -9,7 +9,7 @@ title: getHotkeyManager
  
    function getHotkeyManager(): HotkeyManager;

    ```

    Defined in: [manager.ts:542](https://github.com/TanStack/keys/blob/main/packages/keys/src/manager.ts#L542)

    Defined in: [manager.ts:536](https://github.com/TanStack/keys/blob/main/packages/keys/src/manager.ts#L536)

    Gets the singleton HotkeyManager instance.

    Convenience function for accessing the manager.

packages/keys/src/index.ts

-Original file line number
+Diff line change
@@ Expand Up / @@ -22,6 +22,7 @@ export type { @@
       HotkeyCallback,
       HotkeyCallbackContext,
       // Option types
+      ConflictBehavior,
       HotkeyOptions,
       FormatDisplayOptions,
       ValidationResult,
@@ Expand Down @@

packages/keys/src/manager.ts

-Original file line number
+Diff line change
@@ Expand Up / @@ -23,6 +23,7 @@ const defaultHotkeyOptions: Omit< @@
       requireReset: false,
       enabled: true,
       ignoreInputs: true,
+      conflictBehavior: 'warn',
     }
     let registrationIdCounter = 0
@@ Expand Down Expand Up / @@ -131,6 +132,19 @@ export class HotkeyManager { @@
           options.target ??
           (typeof document !== 'undefined' ? document : ({} as Document))
+        // Resolve conflict behavior
+        const conflictBehavior = options.conflictBehavior ?? 'warn'
+        // Check for existing registrations with the same hotkey and target
+        const conflictingRegistration = this.#findConflictingRegistration(
+          hotkey,
+          target,
+        )
+        if (conflictingRegistration) {
+          this.#handleConflict(conflictingRegistration, hotkey, conflictBehavior)
+        }
         const registration: HotkeyRegistration = {
           id,
           hotkey,
@@ Expand Down Expand Up / @@ -419,6 +433,54 @@ export class HotkeyManager { @@
         return false
       }
+      /**
+       * Finds an existing registration with the same hotkey and target.
+       */
+      #findConflictingRegistration(
+        hotkey: Hotkey,
+        target: HTMLElement | Document | Window,
+      ): HotkeyRegistration | null {
+        for (const registration of this.#registrations.values()) {
+          if (registration.hotkey === hotkey && registration.target === target) {
+            return registration
+          }
+        }
+        return null
+      }
+      /**
+       * Handles conflicts between hotkey registrations based on conflict behavior.
+       */
+      #handleConflict(
+        conflictingRegistration: HotkeyRegistration,
+        hotkey: Hotkey,
+        conflictBehavior: 'warn' | 'error' | 'replace' | 'allow',
+      ): void {
+        if (conflictBehavior === 'allow') {
+          return
+        }
+        if (conflictBehavior === 'warn') {
+          console.warn(
+            `Hotkey '${hotkey}' is already registered. Multiple handlers will be triggered. ` +
+              `Use conflictBehavior: 'replace' to replace the existing handler, ` +
+              `or conflictBehavior: 'allow' to suppress this warning.`,
+          )
+          return
+        }
+        if (conflictBehavior === 'error') {
+          throw new Error(
+            `Hotkey '${hotkey}' is already registered. ` +
+              `Use conflictBehavior: 'replace' to replace the existing handler, ` +
+              `or conflictBehavior: 'allow' to allow multiple registrations.`,
+          )
+        }
+        // At this point, conflictBehavior must be 'replace'
+        this.#unregister(conflictingRegistration.id)
+      }
       /**
        * Checks if an element is an input-like element that should be ignored.
        *
@@ Expand Down @@

packages/keys/src/recorder.ts

-Original file line number
+Diff line change
@@ Expand Up / @@ -163,7 +163,9 @@ export class HotkeyRecorder { @@
           // Validate: must have at least one non-modifier key
           if (hasNonModifierKey(finalHotkey, this.#platform)) {
             // Remove listener FIRST to prevent any additional events
-            const handlerToRemove = this.#keydownHandler as ((event: KeyboardEvent) => void) | null
+            const handlerToRemove = this.#keydownHandler as
+              | ((event: KeyboardEvent) => void)
+              | null
             if (handlerToRemove) {
               this.#removeListener(handlerToRemove)
               this.#keydownHandler = null
@@ Expand Down @@

packages/keys/src/types.ts

-Original file line number
+Diff line change
@@ Expand Up / @@ -393,6 +393,16 @@ export type HotkeyCallback = ( @@
     // Options Types
     // =============================================================================
+    /**
+     * Behavior when registering a hotkey that conflicts with an existing registration.
+     *
+     * - `'warn'` - Log a warning to the console but allow both registrations (default)
+     * - `'error'` - Throw an error and prevent the new registration
+     * - `'replace'` - Unregister the existing hotkey and register the new one
+     * - `'allow'` - Allow multiple registrations of the same hotkey without warning
+     */
+    export type ConflictBehavior = 'warn' | 'error' | 'replace' | 'allow'
     /**
      * Options for registering a hotkey.
      */
@@ Expand All / @@ -413,6 +423,8 @@ export interface HotkeyOptions { @@
       ignoreInputs?: boolean
       /** The DOM element to attach the event listener to. Defaults to document. */
       target?: HTMLElement | Document | Window | null
+      /** Behavior when this hotkey conflicts with an existing registration on the same target. Defaults to 'warn' */
+      conflictBehavior?: ConflictBehavior
     }
     // =============================================================================
@@ Expand Down @@

packages/keys/tests/manager.test.ts

-Original file line number
+Diff line change
@@ Expand Up / @@ -717,4 +717,101 @@ describe('HotkeyManager', () => { @@
           }
         })
       })
+      describe('conflict detection', () => {
+        it('should warn by default when registering a conflicting hotkey', () => {
+          const manager = HotkeyManager.getInstance()
+          const callback1 = vi.fn()
+          const callback2 = vi.fn()
+          const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
+          manager.register('Mod+S', callback1)
+          manager.register('Mod+S', callback2)
+          expect(warnSpy).toHaveBeenCalled()
+          expect(warnSpy.mock.calls[0]?.[0]).toContain('already registered')
+          expect(manager.getRegistrationCount()).toBe(2)
+          warnSpy.mockRestore()
+        })
+        it('should throw error when conflictBehavior is "error"', () => {
+          const manager = HotkeyManager.getInstance()
+          const callback1 = vi.fn()
+          const callback2 = vi.fn()
+          manager.register('Mod+S', callback1)
+          expect(() => {
+            manager.register('Mod+S', callback2, { conflictBehavior: 'error' })
+          }).toThrow('already registered')
+          expect(manager.getRegistrationCount()).toBe(1)
+        })
+        it('should replace existing registration when conflictBehavior is "replace"', () => {
+          const manager = HotkeyManager.getInstance()
+          const callback1 = vi.fn()
+          const callback2 = vi.fn()
+          manager.register('Mod+S', callback1, { platform: 'mac' })
+          expect(manager.getRegistrationCount()).toBe(1)
+          manager.register('Mod+S', callback2, {
+            conflictBehavior: 'replace',
+            platform: 'mac',
+          })
+          expect(manager.getRegistrationCount()).toBe(1)
+          document.dispatchEvent(
+            createKeyboardEvent('keydown', 's', { metaKey: true }),
+          )
+          expect(callback1).not.toHaveBeenCalled()
+          expect(callback2).toHaveBeenCalledOnce()
+        })
+        it('should allow multiple registrations when conflictBehavior is "allow"', () => {
+          const manager = HotkeyManager.getInstance()
+          const callback1 = vi.fn()
+          const callback2 = vi.fn()
+          const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
+          manager.register('Mod+S', callback1, { platform: 'mac' })
+          manager.register('Mod+S', callback2, {
+            conflictBehavior: 'allow',
+            platform: 'mac',
+          })
+          expect(warnSpy).not.toHaveBeenCalled()
+          expect(manager.getRegistrationCount()).toBe(2)
+          document.dispatchEvent(
+            createKeyboardEvent('keydown', 's', { metaKey: true }),
+          )
+          expect(callback1).toHaveBeenCalledOnce()
+          expect(callback2).toHaveBeenCalledOnce()
+          warnSpy.mockRestore()
+        })
+        it('should not conflict when same hotkey is registered on different targets', () => {
+          const manager = HotkeyManager.getInstance()
+          const callback1 = vi.fn()
+          const callback2 = vi.fn()
+          const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
+          const div1 = document.createElement('div')
+          const div2 = document.createElement('div')
+          manager.register('Mod+S', callback1, { target: div1 })
+          manager.register('Mod+S', callback2, { target: div2 })
+          expect(warnSpy).not.toHaveBeenCalled()
+          expect(manager.getRegistrationCount()).toBe(2)
+          warnSpy.mockRestore()
+        })
+      })
     })

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Feat conflict detection #4

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!

Feat conflict detection #4

Are you sure you want to change the base?

Uh oh!

Feat conflict detection #4

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!