Skip to content

fix(input-number): stop the hold-to-repeat timers on unmount - #8252

Open
MindOfqc wants to merge 1 commit into
tusen-ai:mainfrom
MindOfqc:fix/input-number-hold-timers-on-unmount
Open

MindOfqc wants to merge 1 commit into
tusen-ai:mainfrom
MindOfqc:fix/input-number-hold-timers-on-unmount

Conversation

@MindOfqc

Copy link
Copy Markdown

Symptom

n-input-number supports press-and-hold on the add / minus buttons: after 800 ms the
value starts repeating every 100 ms. Nothing stops that repeat when the component is
destroyed, because the only way out is a mouseup listener on document and the
component has no unmount hook. So if the input number is unmounted while its button is
being held — it sits in a v-if block, in a row of a dynamic list that gets deleted, in a
modal / drawer / popover that is closed (destroy-on-close, Esc) — then:

  • update:value keeps firing every 100 ms into the still-mounted parent, so the bound
    model keeps being rewritten after the control has left the page;
  • the repeat only stops whenever some unrelated mouseup finally reaches document — if
    the pointer is released outside the browser window, or the user alt-tabs away while
    holding the button, it never stops and the destroyed instance is retained by the pending
    timer and by the document listener.

Reproduction as a user action:

  1. Render <n-input-number v-if="show" v-model:value="count" /> next to a
    <n-button @click="show = false">remove</n-button>.
  2. Press and hold the add button of the input number. While it is still held down, the
    input number is removed from the tree (the row is deleted, the modal closes, the tab is
    switched away — anything that unmounts it before the mouseup arrives).
  3. count keeps increasing by step every 100 ms, indefinitely, although the control is
    gone from the page.

Root cause

handleAddMousedown / handleMinusMousedown schedule a window.setTimeout that starts a
window.setInterval, and they rely solely on a document mouseup listener to cancel
them:

  • src/input-number/src/InputNumber.tsx:512-519 — firstMinusMousedownId = window.setTimeout(...), minusHoldStateIntervalId = window.setInterval(doMinus, HOLDING_CHANGE_INTERVAL), on('mouseup', document, clearMinusHoldTimeout, { once: true })
  • src/input-number/src/InputNumber.tsx:523-530 — the same pair for the add button

The component has no onBeforeUnmount / onUnmounted at all, so unmounting leaves both
timers running and the document listener attached. Every tick then calls doAdd /
doMinus, which reach emit('update:value', ...).

Fix

Release the hold timers and the document listeners when the component unmounts
(src/input-number/src/InputNumber.tsx):

onBeforeUnmount(() => {
  clearAddHoldTimeout()
  clearMinusHoldTimeout()
  off('mouseup', document, clearAddHoldTimeout)
  off('mouseup', document, clearMinusHoldTimeout)
})

off and onBeforeUnmount are added to the existing evtd / vue import statements.

Test

src/input-number/tests/InputNumber.spec.tsx — for each of the two buttons: hold the
button with fake timers, assert the repeat actually started, unmount, advance the timers
again and assert no further update:value is emitted.

Red (before the fix):

 FAIL  src/input-number/tests/InputNumber.spec.tsx > n-input-number > should stop holding the `add` button when the component is unmounted
 FAIL  src/input-number/tests/InputNumber.spec.tsx > n-input-number > should stop holding the `minus` button when the component is unmounted
AssertionError: expected "vi.fn()" to be called 2 times, but got 4 times
 ❯ src/input-number/tests/InputNumber.spec.tsx:309:29

 Test Files  1 failed (1)
      Tests  2 failed | 16 passed (18)

Green (after the fix), whole src/input-number suite:

 ✓ src/input-number/tests/InputNumber.spec.tsx (18 tests) 635ms
 ✓ src/input-number/tests/server.spec.tsx (1 test) 33ms

 Test Files  2 passed (2)
      Tests  19 passed (19)

Commands run:

node node_modules/vitest/vitest.mjs run --config vitest.local.config.mts src/input-number/
node node_modules/typescript/bin/tsc -p tsconfig.test.json --noEmit --pretty false   # 0 errors

Note: eslint could not be run in this clone — node_modules/.bin is missing and
node_modules/eslint resolves to an empty directory, so the toolchain is not installed.
The change is limited to one existing import line, one existing import list, and one new
hook placed next to the timers it cancels, so the repo's lint-staged step in CI should not
flag anything.

Prepared with an AI coding agent.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant