useHotkeys: does unbind all handlers attached to a key instead of just the one that the controller is disconnecting

[marcelolx]

For example, let's suppose we have to forms on the same page, each one attached to a controller that registers the same hotkey for both, when one of those forms is submitted the controller disconnects and unbinds the key, but it is unbinding both handlers for that given key instead of just that one of the form that was submitted.

Ex:

<form data-controller="example" id="new_name">
  <label for="name">Name:</label><br>
  <input type="text" id="name" name="name"><br>
  <input type="submit" name="commit" value="Submit" id="submit_new_name">
</form>
<!-- some other HTML here -->
<form data-controller="example" id="new_message">
  <label for="message">Message:</label><br>
  <input type="text" id="message" name="message"><br>
  <input type="submit" name="commit" value="Submit" id: "submit_new_message">
</form>

import { Controller } from "@hotwired/stimulus"
import { useHotkeys } from "stimulus-use"

// Connects to data-controller="example"
export default class extends Controller {
  connect() {
    useHotkeys(this, {
      hotkeys: {
        "ctrl+enter": {
          handler: this.submitForm,
          options: {
            element: this.element
          }
        },
        "cmd+enter": {
          handler: this.submitForm,
          options: {
            element: this.element
          }
        }
      },
      filter: this.filterForInput
    })
  }

  submitForm (evt) {
    const formToSubmit = evt.target.closest("form")
    const submitButton = formToSubmit.querySelector(`#submit_${formToSubmit.id}`)
    if (submitButton.disabled) return;

    submitButton.focus();
    submitButton.click();
    submitButton.disabled = true;
  }

  filterForInput(evt) {
    return evt.target.tagName === "INPUT"
  }
}

In this example above each form can be submitted when pressing Ctrl+Enter, if I for example submit one of those first, I can go to the next and provide a value to the input and press Ctrl+Enter and it won't submit, since the first submit did unbind the key for all handlers and not only for the handlers attached to that form.

From what I can see when Hotkeys.js unbinds a key it accepts as args the scope and the handler https://github.com/jaywcjlove/hotkeys/blob/master/src/index.js#L112 and here https://github.com/jaywcjlove/hotkeys/blob/master/src/index.js#L142 it checks if the method of a given handler is the same as the one that is being unbound, if use-hotkeys pass the handler to the unbind method then It wouldn't unbind handlers that it shouldn't.

I try it here and had to make the following changes to make it work as expected. I'm also not sure why in the bind method the handler is passed down as (e: KeyboardEvent) => handler(e, e as unknown as HotkeysEvent) instead of just handler (this would be a new function and never would match the handler passed down in the unbind method)

https://github.com/stimulus-use/stimulus-use/blob/main/src/use-hotkeys/use-hotkeys.ts#L42

bind = () => {
    for (const [hotkey, definition] of Object.entries(this.hotkeysOptions.hotkeys as any)) {
      const handler = (definition as HotkeyDefinition).handler.bind(this.controller)
-       hotkeys(hotkey, (definition as HotkeyDefinition).options, (e: KeyboardEvent) =>
-          handler(e, e as unknown as HotkeysEvent)
-        )
+      hotkeys(hotkey, (definition as HotkeyDefinition).options, handler)
    }
  }

https://github.com/stimulus-use/stimulus-use/blob/main/src/use-hotkeys/use-hotkeys.ts#L51

unbind = () => {
-    for (const hotkey in this.hotkeysOptions.hotkeys as any) {
-      hotkeys.unbind(hotkey)
+   for (const [hotkey, definition] of Object.entries(this.hotkeysOptions.hotkeys as any)) {
+     const handler = (definition as HotkeyDefinition).handler.bind(this.controller)
+     hotkeys.unbind(hotkey, (definition as HotkeyDefinition).scope, handler)
    }
  }

[adrienpoly]

@marcelolx thanks for the extensive research and clear explanations. Are you willing to do a PR on this ?

[marcelolx]

@adrienpoly Sure, I can work on a PR when I have some time free!

[marcelolx]

Turns out that what I suggested does not work as I expect, changing it to what I did suggest above makes hotkey.js not unbind the key 😢

I did solve this by binding the handler to the element the controller is connecting, this way it only disconnects the handler of the submitted form which also requires a small change in stimulus-use

import { Controller } from "@hotwired/stimulus"
import { useHotkeys } from "stimulus-use"

// Connects to data-controller="example"
export default class extends Controller {
  connect() {
+  this.submitFormBinded = this.submitForm.bind(this.element)
+
    useHotkeys(this, {
      hotkeys: {
        "ctrl+enter": {
-          handler: this.submitForm,
+         handler: this.submitFormBinded,
          options: {
            element: this.element
          }
        },
        "cmd+enter": {
-          handler: this.submitForm,
+         handler: this.submitFormBinded,
          options: {
            element: this.element
          }
        }
      },
      filter: this.filterForInput
    })
  }

  submitForm (evt) {
    const formToSubmit = evt.target.closest("form")
    const submitButton = formToSubmit.querySelector(`#submit_${formToSubmit.id}`)
    if (submitButton.disabled) return;

    submitButton.focus();
    submitButton.click();
    submitButton.disabled = true;
  }

  filterForInput(evt) {
    return evt.target.tagName === "INPUT"
  }
}

https://github.com/stimulus-use/stimulus-use/blob/main/src/use-hotkeys/use-hotkeys.ts#L42

bind = () => {
    for (const [hotkey, definition] of Object.entries(this.hotkeysOptions.hotkeys as any)) {
-      const handler = (definition as HotkeyDefinition).handler.bind(this.controller)
-       hotkeys(hotkey, (definition as HotkeyDefinition).options, (e: KeyboardEvent) =>
-          handler(e, e as unknown as HotkeysEvent)
-        )
+      hotkeys(hotkey, (definition as HotkeyDefinition).options, (definition as HotkeyDefinition).handler)
    }
  }

https://github.com/stimulus-use/stimulus-use/blob/main/src/use-hotkeys/use-hotkeys.ts#L51

unbind = () => {
-    for (const hotkey in this.hotkeysOptions.hotkeys as any) {
-      hotkeys.unbind(hotkey)
+   for (const [hotkey, definition] of Object.entries(this.hotkeysOptions.hotkeys as any)) {
+     hotkeys.unbind(hotkey, (definition as HotkeyDefinition).scope, (definition as HotkeyDefinition).handler)
    }
  }

I'm not yet sure if binding a function to an HTML element this way is the correct way, but it works as we expect. Gonna ask around to see people's opinion and maybe we can add this info to the docs

[pySilver]

@marcelolx do you plan any PR on that matter? I think your findings are totally correct.

[marcelolx]

@pySilver Unfortunately at the moment I don't have time to work on a PR (I tried at the time but got stuck with some TS errors and decided not to proceed)

If you want to send a PR please go ahead, I don't know when I will have time to invest in this, but at some point I will I just don't know yet when 😅

Currently, the patched version of it in vanilla JS ⬇️
File: app/javascript/packs/patched_stimulus_use.js

// MIT License

// Copyright (c) 2020 Adrien POLY

// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:

// The above copyright notice and this permission notice shall be included in all
// copies or substantial portions of the Software.

// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
// SOFTWARE.

const defaultOptions = {
  debug: false,
  logger: console,
  dispatchEvent: true,
  eventPrefix: true
}

export class PatchedStimulusUse {
  constructor(controller, options = {}) {
    this.debug = options?.debug || controller.application.stimulusUseDebug || defaultOptions.debug
    this.logger = options?.logger || defaultOptions.logger
    this.controller = controller
    this.controllerId = controller.element.id || controller.element.dataset.id
    this.targetElement = options?.element || controller.element

    const { dispatchEvent, eventPrefix } = Object.assign({}, defaultOptions, options)
    Object.assign(this, { dispatchEvent, eventPrefix })

    // make copies of lifecycle functions
    this.controllerInitialize = controller.initialize.bind(controller)
    this.controllerConnect = controller.connect.bind(controller)
    this.controllerDisconnect = controller.disconnect.bind(controller)
  }

  log = (functionName, args) => {
    if (!this.debug) return

    this.logger.groupCollapsed(`%c${this.controller.identifier} %c#${functionName}`, 'color: #3B82F6', 'color: unset')
    this.logger.log({
      controllerId: this.controllerId,
      ...args
    })
    this.logger.groupEnd()
  }

  dispatch = (eventName, details = {}) => {
    if (this.dispatchEvent) {
      const { event, ...eventDetails } = details
      const customEvent = this.extendedEvent(eventName, event || null, eventDetails)
      this.targetElement.dispatchEvent(customEvent)
      this.log('dispatchEvent', { eventName: customEvent.type, ...eventDetails })
    }
  }

  call = (methodName, args = {}) => {
    const method = this.controller[methodName]
    if (typeof method == 'function') {
      return method.call(this.controller, args)
    }
  }

  extendedEvent = (name, event, detail) => {
    const { bubbles, cancelable, composed } = event || { bubbles: true, cancelable: true, composed: true }

    if (event) {
      Object.assign(detail, { originalEvent: event })
    }

    const customEvent = new CustomEvent(this.composeEventName(name), {
      bubbles,
      cancelable,
      composed,
      detail
    })
    return customEvent
  }

  composeEventName = (name) => {
    let composedName = name
    if (this.eventPrefix === true) {
      composedName = `${this.controller.identifier}:${name}`
    } else if (typeof this.eventPrefix === 'string') {
      composedName = `${this.eventPrefix}:${name}`
    }
    return composedName
  }
}

app/javascript/packs/patched_use_hotkeys.js

// MIT License

// Copyright (c) 2020 Adrien POLY

// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:

// The above copyright notice and this permission notice shall be included in all
// copies or substantial portions of the Software.

// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
// SOFTWARE.

import hotkeys from "hotkeys-js"
import { PatchedStimulusUse } from "./patched_stimulus_use"

export class PatchedUseHotkeys extends PatchedStimulusUse {
  constructor(controller, hotkeysOptions) {
    super(controller, hotkeysOptions)
    this.controller = controller
    this.hotkeysOptions = hotkeysOptions
    this.enhanceController()
    this.bind()
  }

  bind = () => {
    for (const [hotkey, definition] of Object.entries(this.hotkeysOptions.hotkeys)) {
      hotkeys(hotkey, definition.options, definition.handler)
    }
  }

  unbind = () => {
    for (const [hotkey, definition] of Object.entries(this.hotkeysOptions.hotkeys)) {
      hotkeys.unbind(hotkey, definition.options?.scope, definition.handler)
    }
  }

  enhanceController() {
    if (this.hotkeysOptions.filter) {
      hotkeys.filter = this.hotkeysOptions.filter
    }

    const controllerDisconnect = this.controller.disconnect.bind(this.controller)

    const disconnect = () => {
      this.unbind()
      controllerDisconnect()
    }

    Object.assign(this.controller, { disconnect })
  }
}

const convertSimpleHotkeyDefinition = (definition) => {
  return {
    handler: definition[0],
    options: {
      element: definition[1]
    }
  }
}

const coerceOptions = (options) => {
  if (!options.hotkeys) {
    const hotkeys = {}

    Object.entries(options).forEach(([hotkey, definition]) => {
      Object.defineProperty(hotkeys, hotkey, {
        value: convertSimpleHotkeyDefinition(definition),
        writable: false,
        enumerable: true
      })
    })

    options = {
      hotkeys
    }
  }

  return options
}

export const patchedUseHotkeys = (controller, options) => {
  return new PatchedUseHotkeys(controller, coerceOptions(options))
}

FILE: app/javascript/controllers/my_controller.js

import { Controller } from "@hotwired/stimulus"
import { patchedUseHotkeys } from "../packs/patched_use_hotkeys"

export default class extends Controller {
  connect() {
    patchedUseHotkeys(this, {
      hotkeys: {
        "ctrl+enter": {
          handler: this.submitForm.bind(this.element),
          options: {
            element: this.element
          }
        },
        "cmd+enter": {
          handler: this.submitForm.bind(this.element),
          options: {
            element: this.element
          }
        }
      },
      filter: this.filterForElement
    })
  }

  submitForm (evt) {
    /// ....
  }

  filterForElement (evt) {
    /// ....
  }
}