fix(core): invoke structured Signal listeners with runtime args before bound args

[cptbtptpbcptdtptp]

Summary

• Flip structured-binding listener invocation order from method(...boundArgs, ...signalArgs) to method(...signalArgs, ...boundArgs), so the event object always sits at index 0.
• Aligns with DOM (event, customData) convention.

Background

Cherry-picked from b16d8b5d7 on fix/shaderlab.

Test plan

• 3 new Signal.test.ts cases: arg ordering, event index 0 invariant, once() parity
• All 29 Signal tests pass
• CI green

🤖 Generated with Claude Code

Summary by CodeRabbit

Summary by CodeRabbit

• Bug Fixes

  • Fixed Signal structured binding invocation order so runtime signal arguments are passed before pre-resolved bound arguments. The event object continues to be the first argument, restoring consistent handler behavior.

• Tests

  • Expanded Signal tests to verify runtime vs bound argument ordering for both persistent and single-use listeners.

• Chores

  • Updated related test handler signatures and formatting to align with structured binding argument expectations.

[coderabbitai]

Walkthrough

Signal's structured binding listener invocation now passes runtime signal arguments before pre-resolved bound arguments. The implementation is updated in the listener wrapper, documented in JSDoc for both on and once overloads, validated with new test cases that verify argument order preservation, and test handler signatures across multiple test files are realigned to accept the new parameter order.

Changes

Signal Argument Ordering for Structured Binding

Layer / File(s)	Summary
Core Implementation packages/core/src/Signal.ts	Listener wrapper for structured binding now calls target method as target[methodName](...signalArgs, ...args) instead of target[methodName](...args, ...signalArgs).
Documentation packages/core/src/Signal.ts	JSDoc for both on and once structured binding overloads now document that runtime signal arguments are passed first and bound arguments are appended.
Test Infrastructure tests/src/core/Signal.test.ts	TestHandler gains lastArgs field to record received arguments and handleWithArgs(...args) method to support argument ordering validation.
Test Validation tests/src/core/Signal.test.ts	New test cases verify runtime args precede bound args, event object stays at index 0 regardless of bound-arg count, and once preserves argument order for both invocation count and captured arguments.
Handler Signature Alignment tests/src/core/CloneUtils.test.ts, tests/src/ui/UIInteractive.test.ts	Test handlers are updated to accept leading runtime argument parameters (numeric or PointerEventData) before existing parameters, matching the new Signal structured binding invocation order. Import statements are also reformatted and minor punctuation is added.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Arguments waltz in perfect line,
Runtime first, then bound ones shine!
Signal speaks with ordered care,
Handlers listen everywhere! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the primary change: updating Signal listener invocation to pass runtime arguments before bound arguments, which is the core semantic change across all modified files.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[GuoLei1990]

已关闭问题清单

历史 23 轮 review 的问题均已关闭：

• 参数顺序正确性 — 已多次验证正确，关闭。
• call-site 影响分析（closure vs structured binding） — 已确认引擎内无 listener 依赖旧顺序，关闭。
• breaking change 需记入 CHANGELOG — 历轮反复提出（P2/P1 均出现过），最终结论为非阻塞 P2，状态确认仍未落入 CHANGELOG 但不阻塞合入。按既定结论不再重提。

总结

将 structured binding listener 的调用顺序从 method(...boundArgs, ...signalArgs) 翻转为 method(...signalArgs, ...boundArgs)，使 runtime signal 参数在前、bound 参数在后，event 对象固定在 index 0。

方向正确。这里刻意不遵循 Function.prototype.bind（bind 是前置 bound args），而是对齐 DOM addEventListener / Cocos (event, customData) 的事件回调约定——对 signal/event 系统而言这是本质正确的选择：回调第一个参数永远是事件本身。JSDoc 已显式写明 method(...signalArgs, ...args)，契约自洽。

第一性原理复核（基于 PR HEAD eb8ac58b）：

• 语义正确性 — 确认。runtime 在前、bound 在后，与 closure listener 语义一致，event 恒在 index 0。
• 向后兼容 — 4 个生产 Signal 实例（onClick/onStateChanged/onTrackedDeviceChanged/onChanged）全部走 closure 形式，不受影响；唯一 structured-binding 生产调用点 ReflectionParser.ts:181 是反序列化透传，无 handler 逻辑；两个既有 bound-args 测试都用零 runtime args invoke，翻转后结果不变。
• 回归测试反向证伪 — 有效。把 fix revert 回 (...args, ...signalArgs)：测试 "runtime args precede bound args" 期望 ["event", 42, "boundA", "boundB"] 会变成 ["boundA", "boundB", "event", 42] → fail；once 测试期望 [99, "bound"] 会变成 ["bound", 99] → fail。测试真实守住了 bug。链路方式（invoke() 触发 + 断言 lastArgs）符合规范。
• 注释合规 — 新增 JSDoc 多行块结尾带句号、@param 用破折号不带句号，均合规。

问题

无新问题。LGTM，可合入。

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

tests/src/ui/UIInteractive.test.ts (1)

16-28: ⚠️ Potential issue | 🔴 Critical

Update handleClick() to accept PointerEventData parameter.

The handleClick() method is missing the PointerEventData parameter that was added to handleClickWithPrefix(). Both methods are used as Signal handlers and should follow the same argument ordering. Update handleClick() to:

handleClick(event: PointerEventData) {
  this.callCount++;
}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/src/ui/UIInteractive.test.ts` around lines 16 - 28, The handleClick()
method in the ClickHandler class is missing the PointerEventData parameter that
is required for Signal handlers. Update the handleClick() method signature to
include the event parameter of type PointerEventData as the first parameter,
matching the signature pattern used in handleClickWithPrefix(). This ensures
both methods can be properly used as Signal handlers with consistent argument
ordering.

tests/src/core/CloneUtils.test.ts (1)

95-107: ⚠️ Potential issue | 🔴 Critical

Update handleClick() to accept the Signal data parameter.

The handleClick() method should receive the number argument emitted by the Signal, matching the structured binding convention where method(...signalArgs, ...args) places runtime signal arguments first. Change:

handleClick(): void {
  this.callCount++;
}

to:

handleClick(arg: number): void {
  this.callCount++;
}

The handleClickWithPrefix() method is already correct. No other handler methods exist in this test file.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/src/core/CloneUtils.test.ts` around lines 95 - 107, The handleClick()
method in the ClickHandler class does not accept the Signal data parameter that
is being emitted, while handleClickWithPrefix() correctly accepts parameters.
Update the handleClick() method signature to accept the arg: number parameter as
the first parameter, matching the signal binding convention where runtime signal
arguments are passed before custom arguments.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@tests/src/core/CloneUtils.test.ts`:
- Around line 95-107: The handleClick() method in the ClickHandler class does
not accept the Signal data parameter that is being emitted, while
handleClickWithPrefix() correctly accepts parameters. Update the handleClick()
method signature to accept the arg: number parameter as the first parameter,
matching the signal binding convention where runtime signal arguments are passed
before custom arguments.

In `@tests/src/ui/UIInteractive.test.ts`:
- Around line 16-28: The handleClick() method in the ClickHandler class is
missing the PointerEventData parameter that is required for Signal handlers.
Update the handleClick() method signature to include the event parameter of type
PointerEventData as the first parameter, matching the signature pattern used in
handleClickWithPrefix(). This ensures both methods can be properly used as
Signal handlers with consistent argument ordering.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: fb3b2753-084b-4402-a3a6-c19939f1c0b2

📥 Commits

Reviewing files that changed from the base of the PR and between eb8ac58 and 74800f2.

📒 Files selected for processing (2)

• tests/src/core/CloneUtils.test.ts
• tests/src/ui/UIInteractive.test.ts

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.89%. Comparing base (1bc2b10) to head (74800f2).
⚠️ Report is 28 commits behind head on dev/2.0.

Additional details and impacted files

@@             Coverage Diff             @@
##           dev/2.0    #2987      +/-   ##
===========================================
+ Coverage    78.25%   79.89%   +1.64%     
===========================================
  Files          900      919      +19     
  Lines        99234   102586    +3352     
  Branches     10172    11348    +1176     
===========================================
+ Hits         77657    81966    +4309     
+ Misses       21406    20438     -968     
- Partials       171      182      +11     

Flag	Coverage Δ
unittests	79.89% <100.00%> (+1.64%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[GuoLei1990]

总结

自上轮 review（HEAD eb8ac58b）以来，新增一个 commit 74800f282（test: align Signal structured-binding fixtures with new arg order）。生产代码 Signal.ts 零改动，本 commit 纯粹是把两个既有 fixture 补上前导 runtime 参数以修复 CI 红 + prettier 格式化。方向正确。

增量核对：

• 根因正确 — 原 fix commit 6728ba456 翻转了调用顺序为 method(...signalArgs, ...boundArgs)，但漏改了 CloneUtils.test.ts 与 UIInteractive.test.ts 里的 handleClickWithPrefix fixture（仍是单参 (prefix)）。翻转后这两处 fixture 的 prefix 会捕获到 runtime 信号参数而非绑定值 → CI 红。本 commit 把签名补成 (arg: number, prefix) / (event: PointerEventData, prefix)，prefix 重新解析到绑定值 "myPrefix"，是补全 6728ba456 应做而未做的工作，落点正确。
• 逐路径验证 — CloneUtils.test.ts:604 注册 ("myPrefix") + invoke(1) → handleClickWithPrefix(1, "myPrefix") → prefix="myPrefix" ✓；UIInteractive.test.ts:239 注册 ("myPrefix") + invoke(PointerEventData) → handleClickWithPrefix(pointerEvent, "myPrefix") → prefix="myPrefix" ✓。两处断言 lastPrefix==="myPrefix" 均成立。
• 反向证伪有效 — 把生产 fix revert 回 ...args, ...signalArgs：两处会变成 handleClickWithPrefix("myPrefix", runtimeArg) → prefix=runtimeArg（数字 1 / PointerEventData）→ expect(lastPrefix).toBe("myPrefix") fail。新签名下这两个 fixture 真实守住回归。
• 非对称性自洽 — Signal.test.ts 同名 fixture handleClickWithPrefix(prefix: string) 保持单参且无需改，因为它的两处调用（206、407 行）都是零 runtime arg invoke（invoke()），顺序翻转对零参是 no-op，断言不受影响。改与不改的边界划得准。
• CI — 全绿（build / lint / e2e / codecov 全 pass）。

问题

无新问题。本 commit 是干净的 CI 修复，生产逻辑未变，历史已关闭问题（参数顺序正确性、call-site 影响、breaking-change CHANGELOG 非阻塞结论）均不复现。LGTM，可合入。