search

proposal-signals / signal-polyfill

Implementation tracking the current state of https://github.com/tc39/proposal-signals
Apache License 2.0
185 stars 13 forks source link

Fix unwatch multiple signals #26

Closed tuhm1 closed 1 month ago

tuhm1 commented 2 months ago

Fixes #2

The issue arises when removing an element, which shifts the last element to the current index. As a result, the last index goes out of bounds, and the moved element remains unprocessed.

To address this, iterate backward so that the removed element is replaced with a checked element, while the others remain unchanged.

littledan commented 2 months ago

@ds300 Do you recommend landing this change? Does it overlap with/obviate #24?

ds300 commented 2 months ago

@littledan yeah this is good to land I think. The other PR should be closed, it adds some dubious sanity checks but doesn't fix any bugs as far as I can tell (actually it seems to add code that would swallow legitimate errors in some cases).

Roeck commented 2 months ago

@ds300 Just to clarify, I've tested these changes thoroughly, and they don't introduce any new issues. These sanity checks are not meant to hide real errors but to prevent the feature from breaking when it encounters unusual situations.