Skip to content

Commit 123d4bd

Browse files
committed
Fix race condition with shouldRecycle=false during fast scroll
When shouldRecycle is false, the previous code path would call virtualComponents.removeObjects(_componentPool) without properly handling the DOM, which could cause issues during fast scrolling. This change simplifies the logic by moving the shouldRecycle check inside the loop, ensuring components are always properly cleaned up regardless of the shouldRecycle setting. Fixes #296
1 parent 224c4e5 commit 123d4bd

File tree

1 file changed

+26
-31
lines changed
  • vertical-collection/src/-private/data-view/radar

1 file changed

+26
-31
lines changed

vertical-collection/src/-private/data-view/radar/radar.js

Lines changed: 26 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -588,38 +588,33 @@ export default class Radar {
588588

589589
// If there are any items remaining in the pool, remove them
590590
if (_componentPool.length > 0) {
591-
if (shouldRecycle === true) {
592-
// Grab the DOM of the remaining components and move it to temporary node disconnected from
593-
// the body if the item can be reused later otherwise delete the component to avoid virtual re-rendering of the
594-
// deleted item. If we end up using these components again, we'll grab their DOM and put it back
595-
for (let i = _componentPool.length - 1; i >= 0; i--) {
596-
const component = _componentPool[i];
597-
const item = objectAt(items, component.index);
598-
if (item) {
599-
insertRangeBefore(
600-
this._domPool,
601-
null,
602-
component.realUpperBound,
603-
component.realLowerBound,
604-
);
605-
} else {
606-
// Insert the virtual component bound back to make sure Glimmer is
607-
// not confused about the state of the DOM.
608-
insertRangeBefore(
609-
this._itemContainer,
610-
null,
611-
component.realUpperBound,
612-
component.realLowerBound,
613-
);
614-
run(() => {
615-
virtualComponents.removeObject(component);
616-
});
617-
_componentPool.splice(i, 1);
618-
}
591+
// Grab the DOM of the remaining components and move it to temporary node disconnected from
592+
// the body if the item can be reused later otherwise delete the component to avoid virtual re-rendering of the
593+
// deleted item. If we end up using these components again, we'll grab their DOM and put it back
594+
for (let i = _componentPool.length - 1; i >= 0; i--) {
595+
const component = _componentPool[i];
596+
const item = objectAt(items, component.index);
597+
if (shouldRecycle === true && item) {
598+
insertRangeBefore(
599+
this._domPool,
600+
null,
601+
component.realUpperBound,
602+
component.realLowerBound,
603+
);
604+
} else {
605+
// Insert the virtual component bound back to make sure Glimmer is
606+
// not confused about the state of the DOM.
607+
insertRangeBefore(
608+
this._itemContainer,
609+
null,
610+
component.realUpperBound,
611+
component.realLowerBound,
612+
);
613+
run(() => {
614+
virtualComponents.removeObject(component);
615+
});
616+
_componentPool.splice(i, 1);
619617
}
620-
} else {
621-
virtualComponents.removeObjects(_componentPool);
622-
_componentPool.length = 0;
623618
}
624619
}
625620

0 commit comments

Comments
 (0)