fix: detect array length drift so cleared arrays re-render#7651
Open
AKnassa wants to merge 1 commit into
Open
Conversation
`array.length = 0` is the common idiom for clearing an array, but the length property cannot be trapped without a Proxy, so it recorded no splice and `repeat` kept rendering the removed items. The array observer now tracks the length it expects after the queued changes and, when the real length disagrees, emits a reset instead of incremental splices that no longer describe reality. Fixes microsoft#7167
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Makes
myArray.length = 0— the everyday way to empty an array in JavaScript — actually clear the list on screen.Why
FAST notices array changes by wrapping the array's methods (
push,splice,pop, and so on). Settinglengthis not a method call, so nothing noticed it. The array really was emptied, butrepeatkept rendering the old items, andlengthOfkept reporting the old count.The person who reported this put it plainly: "I cannot use FAST until this is resolved."
What changed
This same drift check also covers the other silent cases —
delete arr[i], writing past the end of the array, and the array methods that were never wrapped.An honest limit
lengthcannot be intercepted directly (it is non-configurable, so it cannot be trapped without turning arrays into Proxies, which would be a much larger change). What this does is detect the drift and heal from it. Soarr.length = 0on its own, with no other tracked change afterwards, still does not notify — the array observer has nothing to wake it. In practice the clear is nearly always followed by tracked work, which is when the correction lands.If maintainers would rather solve this properly with a Proxy-based reactive array, this PR is a stopgap and should be judged that way.
How to see it
Run the
fast-elementtest suite. New tests inobservation/arrays.pw.spec.tssetlength = 0, push a new item, and assert a single reset arrives — wheremainproduces change records describing an array that no longer exists.The existing test that asserts
array.splice(0, array.length, ...array)produces no change records is the sharpest guard here: the net length change is zero, so the drift check must not fire. It still passes.Fixes #7167