fix: release repeat views when the host disconnects#7650
Open
AKnassa wants to merge 1 commit into
Open
Conversation
A repeat kept its views alive on unbind, so reconnecting the host reused the stale views instead of rebuilding them. Route swaps, tab switches and DOM moves produced binding errors against a nulled source, and every repeated element fired disconnectedCallback twice. The views' DOM is now parked when the controller is genuinely going away, and restored on rebind, so element instances survive a DOM move. A recycled view rebinding to new data is unaffected, keeping the nested-repeat path at parity. Stale notifications arriving while unbound are ignored, matching the guards already present on the other view behaviors. Fixes microsoft#7144
|
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
Fixes a list (
repeat) that comes back wrong after its component is taken out of the page and put back — which is what happens on a route change, a tab switch, or any time the DOM is moved.Why
When a component was removed from the page,
repeatlet go of its data but kept its rendered rows. So when the component came back, it reused those stale rows instead of rebuilding them.Two things went wrong as a result:
What changed
How to see it
Run the
fast-elementtest suite.templating/repeat-reconnect.pw.spec.tsis new: it removes a component containing a list, changes the data, puts it back, and asserts the rows are rebuilt rather than resurrected. Those tests fail onmain.Performance was measured, because this is a hot path. Every steady-state scenario — create, reuse, splice, and the nested-recycle case that a naive version of this fix would have wrecked — comes out at parity with
main. The added cost lands only on disconnect and reconnect (roughly 0.2 ms for a 500-row list per full cycle), which is not a per-frame path, and it is the price of actually taking the DOM out while detached. In the scenario this issue describes, the fixed version is likely faster, sincemaincurrently resurrects rows and then tears them down again.Note for reviewers
RepeatBehavior.unbind()now takes the controller, and views carry an internalisUnbindingflag. That flag is what distinguishes "this controller is going away" from "this view is being recycled with new data" — without it, teardown would rebuild every nested list's DOM on every recycled row, which would be a real regression.Fixes #7144