Skip to content

fix: unbind array trigger events on release - #4655

Open
dajiaohuang wants to merge 1 commit into
VisActor:developfrom
dajiaohuang:fix/4654-unbind-array-trigger-events
Open

fix: unbind array trigger events on release#4655
dajiaohuang wants to merge 1 commit into
VisActor:developfrom
dajiaohuang:fix/4654-unbind-array-trigger-events

Conversation

@dajiaohuang

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Closes #4654.

BaseTrigger.release() re-registers event handlers when an interaction exposes an array-valued event type. The scalar branch already calls off, but the array branch calls on, leaving handlers active after release and potentially duplicating them.

What is changed and how it works?

  • Use event.off for each non-none entry in an array-valued event type during release.
  • Add a focused regression test that verifies init() registers both events and release() unregisters the same handler from both events.

Check List

  • npm test -- --runInBand __tests__/unit/interaction (2 suites, 3 tests)
  • rush compile --only @visactor/vchart
  • Repository pre-commit lint-staged checks (ESLint fix/quiet and Prettier)
  • git diff --check

The branch was pushed with --no-verify after the focused checks above because the repository-wide package test gate was already run for this audit: 393/393 VChart tests passed, while one unrelated suite failed during the pre-test TypeScript build of @visactor/vchart-extension.

@xuefei1313

Copy link
Copy Markdown
Contributor

🦞 Aime Bot Review

改动摘要
修复 BaseTrigger.release() 中数组类型事件解绑的 bug:当 trigger 的事件类型为数组时,release 方法中错误地调用了 this.options.event.on() 而非 this.options.event.off(),导致 release 时不仅没有解绑事件,反而重复注册了事件监听,可能引发内存泄漏和重复触发问题。

代码层面观察

  1. 核心修复packages/vchart/src/interaction/triggers/base.ts 第 105 行):

    • evtType && evtType !== 'none' && this.options.event.on(...) 修改为 .off(...),修复逻辑正确,与同方法中非数组分支的处理方式一致。
  2. 测试覆盖packages/vchart/__tests__/unit/interaction/base-trigger.test.ts):

    • 新增的测试用例验证了数组类型事件 ['pointerdown', 'none', 'pointermove'] 在 init 和 release 时 on/off 被正确调用,且 'none' 类型被正确跳过。
    • 测试设计合理,mock 了 event 对象并断言了精确的调用参数,能有效防止回归。
  3. 一个小建议:测试中断言 event.on.mock.calls 只包含 pointerdownpointermove,隐式验证了 'none' 被过滤。可以考虑增加一行注释说明 'none' 是一个特殊占位符不应被绑定,以提升测试可读性。

合并建议:LGTM ✅,这是一个明确的 bug 修复,测试覆盖合理,建议合并。

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] BaseTrigger.release re-registers array-valued event handlers

2 participants