urbit/shrub
1
1
Fork 0
mirror of https://github.com/urbit/shrub.git synced 2024-12-19 16:51:42 +03:00
Code Issues Packages Projects Releases Wiki Activity

VirtualScroller: fix race condition in ref deletion

Browse Source 
A callback ref is called after the component is mounted, but before the
component is unmounted. However, we might still be adjusting scroll
position based on a component that is going to be remounted. Previously,
we delayed the deletion until the next tick with setTimeout. With the
faster ordered map implementation, the component may be remounted
before the next tick, leading to the deletion of a ref that is still
mounted. To work around this, we store a set of 'orphans' and clear the
map of orphans on an interval, and only clear the map if we are not
currently adjusting our scroll position. Also includes fixes for jumpy
scroll behaviour on initial mount.
This commit is contained in:
Liam Fitzgerald 2021-04-22 15:04:38 +10:00
parent f80ca5a3da
commit 6f7ed005ae
No known key found for this signature in database
GPG Key ID: D390E12C61D1CFFB
1 changed files with 51 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
pkg/interface/src/views/components/VirtualScroller.tsx
View File

@ -115,6 +115,10 @@ export default class VirtualScroller<T> extends Component<VirtualScrollerProps<T
* A map of child refs, used to calculate scroll position
*/
private childRefs = new BigIntOrderedMap<HTMLElement>();
/**
* A set of child refs which have been unmounted
*/
private orphans = new Set<string>();
/**
* If saving, the bottommost visible element that we pin our scroll to
*/
@ -140,6 +144,10 @@ export default class VirtualScroller<T> extends Component<VirtualScrollerProps<T
private scrollRef: HTMLElement | null = null;
private cleanupRefInterval: NodeJS.Timeout | null = null;
private initScroll: NodeJS.Timeout | null = null;
constructor(props: VirtualScrollerProps<T>) {
super(props);
this.state = {
@ -157,6 +165,7 @@ export default class VirtualScroller<T> extends Component<VirtualScrollerProps<T
this.onScroll = IS_IOS ? _.debounce(this.onScroll.bind(this), 400) : this.onScroll.bind(this);
this.scrollKeyMap = this.scrollKeyMap.bind(this);
this.setWindow = this.setWindow.bind(this);
this.restore = this.restore.bind(this);
}
componentDidMount() {
@ -164,8 +173,27 @@ export default class VirtualScroller<T> extends Component<VirtualScrollerProps<T
this.resetScroll();
this.loadTop();
this.loadBottom();
this.cleanupRefInterval = setInterval(this.cleanupRefs, 5000);
this.initScroll = setTimeout(() => {
log('scroll', 'initialised scroll');
this.restore();
this.initScroll = null;
}, 100);
}
cleanupRefs = () => {
if(this.saveDepth > 0) {
return;
}
[...this.orphans].forEach(o => {
const index = bigInt(o);
this.childRefs.delete(index);
});
this.orphans.clear();
};
// manipulate scrollbar manually, to dodge change detection
updateScroll = IS_IOS ? () => {} : _.throttle(() => {
if(!this.window || !this.scrollRef) {
@ -199,6 +227,12 @@ export default class VirtualScroller<T> extends Component<VirtualScrollerProps<T
componentWillUnmount() {
window.removeEventListener('keydown', this.invertedKeyHandler);
if(this.cleanupRefInterval) {
clearInterval(this.cleanupRefInterval);
}
if(this.initScroll) {
clearTimeout(this.initScroll);
}
}
startOffset() {
@ -237,9 +271,6 @@ export default class VirtualScroller<T> extends Component<VirtualScrollerProps<T
}, () => {
requestAnimationFrame(() => {
this.restore();
requestAnimationFrame(() => {
});
});
});
}
@ -339,6 +370,10 @@ export default class VirtualScroller<T> extends Component<VirtualScrollerProps<T
// bail if we're going to adjust scroll anyway
return;
}
if(this.initScroll) {
clearTimeout(this.initScroll);
this.initScroll = null;
}
if(this.saveDepth > 0) {
log('bail', 'deep scroll queue');
return;
@ -394,8 +429,15 @@ export default class VirtualScroller<T> extends Component<VirtualScrollerProps<T
log('bail', 'Deep restore');
return;
}
if(this.initScroll) {
log('bail', 'still initialising scroll');
return;
}
const ref = this.childRefs.get(this.savedIndex)!;
let ref = this.childRefs.get(this.savedIndex)
if(!ref) {
return;
}
const newScrollTop = this.window.scrollHeight - ref.offsetTop - this.savedDistance;
this.window.scrollTo(0, newScrollTop);
@ -435,11 +477,12 @@ export default class VirtualScroller<T> extends Component<VirtualScrollerProps<T
if(!this.window || this.savedIndex) {
return;
}
this.saveDepth++;
if(this.saveDepth !== 1) {
if(this.saveDepth !== 0) {
console.log('bail', 'deep save');
return;
}
this.saveDepth++;
let bottomIndex: BigInteger | null = null;
const { scrollTop, scrollHeight } = this.window;
@ -472,10 +515,9 @@ export default class VirtualScroller<T> extends Component<VirtualScrollerProps<T
setRef = (element: HTMLElement | null, index: BigInteger) => {
if(element) {
this.childRefs.set(index, element);
this.orphans.delete(index.toString());
} else {
setTimeout(() => {
this.childRefs.delete(index);
});
this.orphans.add(index.toString());
}
}