spurious hide() after destroy() related to addDocumentPress() without matching removeDocumentPress()

[quarl]

Bug description

Tippy calls addDocumentPress() to register mousedown & related handlers in conjunction with showing, and removeDocumentPress() to unregister, in conjunction with hiding.

The problem is that sometimes Tippy calls addDocumentPress() but then misses calling removeDocumentPress() by the time the tippy instance is destroy()ed, causing an unwanted warning in the console about hide() being called after destroy().

tippy.js

hide() was called on a destroyed instance. This is a no-op but indicates a potential memory leak.

This warning turned out to be a harmless bug in Tippy itself, but I spent time debugging it because I thought it might be related to performance degradation in my complex application.

I've identified two ways this can happen:

• Case1: Construct the tippy instance with onShow() returning false, then call destroy(), then click the document (to trigger onDocumentPress()).
• Case2: Trigger the tippy with a delay to cause scheduleShow() to be called, then call destroy() immediately (before show() gets called), then click the document (to trigger onDocumentPress()).

In Case1, observe that Tippy's scheduleShow() already called addDocumentPress(), but then since show() does not end up showing the tippy, then a subsequent hide() does not know it needs to call removeDocumentPress().
Case1 was the specific issue that I observed in my application. (My application has a hook that disables showing the tippy under certain situations.). In Case2, show()

The following would fix Case1 specifically:

Fix1: Make show() call removeDocumentPress() if it's not going to show.

  function show() {
    ...
    if (instance.props.onShow(instance) === false) {
      removeDocumentPress(); // <-- add this line
      return;
    }
    ...
  }

Either one of the following would fix both Case1 and Case2:

Fix2: Add removeDocumentPress() to destroy(). Since removing events is idempotent, it seems safe to call without bothering to keep track of whether we think we've already called removeDocumentPress().

  function destroy() {
    ...
    removeListeners();
    removeDocumentPress(); // <-- add this line
    ...
  }

Fix3: In onDocumentPress(), if the tippy instance is already destroyed, then ensure that we do remove mousedown handlers, and return early to ensure that we don't try to hide.

  function onDocumentPress(event) {
    if (instance.state.isDestroyed) { // <-- add this block
      removeDocumentPress();
      return;
    }
    ...
  }

Any of single one of these 3 fixes would work for me. I suggest doing all three, or else both Fix2 and Fix3. Fix1 and Fix2 clean up as early as possible (even if destroy() isn't called or the document isn't clicked), while Fix3 is defensive against any other potential circumstances that could cause onDocumentPress() to be called when it's already destroyed.

Reproduction

https://codepen.io/c83018918/pen/rNRLaqo

1. Use a non-production version of Tippy so that you get dev warnings, e.g. https://unpkg.com/tippy.js@6/dist/tippy.umd.js
2. Code:

let t = tippy(document.querySelector('button'), {
  content: 'hello',
  delay: 10,
  showOnCreate: true
});
t.destroy()

3. Click document.
4. Observe memory leak warning in console.