From f20da6d3def7b02f922062da771829c0d2085dfc Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 26 Jul 2021 22:05:35 +0100 Subject: [PATCH] Fix infinite pagination loop when offline I'm not really sure how this is meant to work - and I'm not sure how it was working before, but this is causing fairly bad infinite loops if I start element with no homeserver connection. This is a fairly crude fix, only thing I can think that would be better is some awareness of when network requests were failing and intentionally backing off. Fixes https://github.com/vector-im/element-web/issues/18242 --- src/components/structures/ScrollPanel.tsx | 27 ++++++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/src/components/structures/ScrollPanel.tsx b/src/components/structures/ScrollPanel.tsx index 1d167551069..f90cdca167f 100644 --- a/src/components/structures/ScrollPanel.tsx +++ b/src/components/structures/ScrollPanel.tsx @@ -183,8 +183,14 @@ export default class ScrollPanel extends React.Component { private readonly itemlist = createRef(); private unmounted = false; private scrollTimeout: Timer; + // Are we currently trying to backfill? private isFilling: boolean; + // Is the current fill request caused by a props update? + private isFillingDueToPropsUpdate = false; + // Did another request to check the fill state arrive while we were trying to backfill? private fillRequestWhileRunning: boolean; + // Is that next fill request scheduled because of a props update? + private pendingFillDueToPropsUpdate: boolean; private scrollState: IScrollState; private preventShrinkingState: IPreventShrinkingState; private unfillDebouncer: number; @@ -213,7 +219,7 @@ export default class ScrollPanel extends React.Component { // adding events to the top). // // This will also re-check the fill state, in case the paginate was inadequate - this.checkScroll(); + this.checkScroll(true); this.updatePreventShrinking(); } @@ -251,12 +257,12 @@ export default class ScrollPanel extends React.Component { // after an update to the contents of the panel, check that the scroll is // where it ought to be, and set off pagination requests if necessary. - public checkScroll = () => { + public checkScroll = (isFromPropsUpdate = false) => { if (this.unmounted) { return; } this.restoreSavedScrollState(); - this.checkFillState(); + this.checkFillState(0, isFromPropsUpdate); }; // return true if the content is fully scrolled down right now; else false. @@ -319,7 +325,7 @@ export default class ScrollPanel extends React.Component { } // check the scroll state and send out backfill requests if necessary. - public checkFillState = async (depth = 0): Promise => { + public checkFillState = async (depth = 0, isFromPropsUpdate = false): Promise => { if (this.unmounted) { return; } @@ -355,14 +361,20 @@ export default class ScrollPanel extends React.Component { // don't allow more than 1 chain of calls concurrently // do make a note when a new request comes in while already running one, // so we can trigger a new chain of calls once done. + // However, we make an exception for when we're already filling due to a + // props (or children) update, because very often the children include + // spinners to say whether we're pagainating or not, so this would cause + // infinite paginating. if (isFirstCall) { - if (this.isFilling) { + if (this.isFilling && !this.isFillingDueToPropsUpdate) { debuglog("isFilling: not entering while request is ongoing, marking for a subsequent request"); this.fillRequestWhileRunning = true; + this.pendingFillDueToPropsUpdate = isFromPropsUpdate; return; } debuglog("isFilling: setting"); this.isFilling = true; + this.isFillingDueToPropsUpdate = isFromPropsUpdate; } const itemlist = this.itemlist.current; @@ -393,11 +405,14 @@ export default class ScrollPanel extends React.Component { if (isFirstCall) { debuglog("isFilling: clearing"); this.isFilling = false; + this.isFillingDueToPropsUpdate = false; } if (this.fillRequestWhileRunning) { + const refillDueToPropsUpdate = this.pendingFillDueToPropsUpdate; this.fillRequestWhileRunning = false; - this.checkFillState(); + this.pendingFillDueToPropsUpdate = false; + this.checkFillState(0, refillDueToPropsUpdate); } };