Skip to content

Commit bcd90d0

Browse files
committed
Fix race condition with shouldRecycle=false during fast scroll
Consolidates component removal, prepend, and append operations into a single layout callback to prevent concurrent DOM manipulation that caused "removeChild" and "Cannot read properties of null" errors. Fixes #296
1 parent db043b5 commit bcd90d0

File tree

1 file changed

+52
-41
lines changed
  • vertical-collection/addon/-private/data-view/radar

1 file changed

+52
-41
lines changed

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

Lines changed: 52 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ export default class Radar {
104104
this._componentPool = [];
105105
this._prependComponentPool = [];
106106
this._appendComponentPool = []; // https://github.com/html-next/vertical-collection/issues/296
107+
this._removeComponentPool = []; // Defer removal when shouldRecycle=false to avoid race condition
107108

108109
// Boundaries
109110
this._occludedContentBefore = new OccludedContent(occlusionTagName);
@@ -615,8 +616,12 @@ export default class Radar {
615616
}
616617
}
617618
} else {
619+
// When shouldRecycle=false, remove from virtualComponents immediately (so array
620+
// size is correct for insertions) but keep refs in pool for cleanup tracking
618621
virtualComponents.removeObjects(_componentPool);
622+
this._removeComponentPool.push(..._componentPool);
619623
_componentPool.length = 0;
624+
this._scheduleLayout();
620625
}
621626
}
622627

@@ -640,6 +645,51 @@ export default class Radar {
640645
: '';
641646
}
642647

648+
_scheduleLayout() {
649+
if (this._nextLayout === null) {
650+
this._nextLayout = this.schedule('layout', () => {
651+
this._nextLayout = null;
652+
653+
const {
654+
_removeComponentPool,
655+
_appendComponentPool,
656+
_prependComponentPool,
657+
_occludedContentBefore,
658+
_occludedContentAfter,
659+
_itemContainer,
660+
} = this;
661+
662+
// Clear remove pool (components already removed from virtualComponents)
663+
if (_removeComponentPool.length > 0) {
664+
_removeComponentPool.length = 0;
665+
}
666+
667+
while (_prependComponentPool.length > 0) {
668+
const component = _prependComponentPool.pop();
669+
const relativeNode =
670+
_occludedContentBefore.realLowerBound.nextSibling;
671+
insertRangeBefore(
672+
_itemContainer,
673+
relativeNode,
674+
component.realUpperBound,
675+
component.realLowerBound,
676+
);
677+
}
678+
679+
while (_appendComponentPool.length > 0) {
680+
const component = _appendComponentPool.pop();
681+
const relativeNode = _occludedContentAfter.realUpperBound;
682+
insertRangeBefore(
683+
_itemContainer,
684+
relativeNode,
685+
component.realUpperBound,
686+
component.realLowerBound,
687+
);
688+
}
689+
});
690+
}
691+
}
692+
643693
_appendComponent(component) {
644694
const {
645695
virtualComponents,
@@ -668,26 +718,7 @@ export default class Radar {
668718
// We have to move them _after_ they render, so we schedule that if they exist
669719
if (!shouldRecycle) {
670720
_appendComponentPool.unshift(component);
671-
672-
if (this._nextLayout === null) {
673-
this._nextLayout = this.schedule('layout', () => {
674-
this._nextLayout = null;
675-
676-
while (_appendComponentPool.length > 0) {
677-
const component = _appendComponentPool.pop();
678-
679-
// Changes with each inserted component
680-
const relativeNode = _occludedContentAfter.realUpperBound;
681-
682-
insertRangeBefore(
683-
this._itemContainer,
684-
relativeNode,
685-
component.realUpperBound,
686-
component.realLowerBound,
687-
);
688-
}
689-
});
690-
}
721+
this._scheduleLayout();
691722
}
692723
}
693724
}
@@ -716,27 +747,7 @@ export default class Radar {
716747
// Components that are both new and prepended still need to be rendered at the end because Glimmer.
717748
// We have to move them _after_ they render, so we schedule that if they exist
718749
_prependComponentPool.unshift(component);
719-
720-
if (this._nextLayout === null) {
721-
this._nextLayout = this.schedule('layout', () => {
722-
this._nextLayout = null;
723-
724-
while (_prependComponentPool.length > 0) {
725-
const component = _prependComponentPool.pop();
726-
727-
// Changes with each inserted component
728-
const relativeNode =
729-
_occludedContentBefore.realLowerBound.nextSibling;
730-
731-
insertRangeBefore(
732-
_itemContainer,
733-
relativeNode,
734-
component.realUpperBound,
735-
component.realLowerBound,
736-
);
737-
}
738-
});
739-
}
750+
this._scheduleLayout();
740751
}
741752
}
742753

0 commit comments

Comments
 (0)