Infinite Loading FlexTable: Dataset Of One Causes State Problem

[SpencerCarstens]

Hey there!

I think I found a bug, or... I am just missing something obvious.

Thanks for your work on RV!

Issue
Loading a single item list into an Infinite Loading FlexTable prevents any future changes, even after a clearing of state.

My guess would be how the caching is being done, but I wasn't able to make any headway on that.

Steps

• Take the Gist below and replace InfiniteLoader.example.js in the RV source with it.
• Start server, go to InfiniteLoader, etc.
• Click 'Only Two'.
  • This should give you only two results.
• Click 'Multi-Page'.
  • This should give you a standard Infinite list.
• Click 'Clear Data'.
  • This should... clear the data.
• Try any order combination of those 3 buttons.
  • This should not break anything...
• Click 'Only One'.
  • This should give you only one result.
• Click 'Only Two' or 'Multi-Page'.
  • This should hang at loading.
• Click 'Clear Data'.
  • This should clear the table, but no further button clicks will load results.

tl;dr - Once a result of a single row is loaded you can't un-stick it.

// Line 39: If you change this...
const rowCount = hasNextPage ? list.size + 1 : list.size;
// Into this...
const rowCount = hasNextPage ? list.size + 2 : list.size;
// But you are left with an extra loading row showing.

Example
Gist

❤️

[bvaughn]

This is a fantastic bug report. Thanks so much for taking the time to write a clear description and reproduction steps. 😄 I'll take a look soon.

[bvaughn]

Hi again @SpencerCarstens!

Here's an updated Gist that should hopefully be helpful to you. I added inline comments at the tricky parts, most notably here and here.

I think InfiniteLoader is working correctly but can be a bit confusing since, as you've found, there are ways to change the rowCount that won't trigger a new call to the loadMoreRows callback.

I think this belongs to a class of things that are sometimes not intuitive but are done in the name of performance (eg "pure" shouldComponentUpdate, memoized callbacks, etc). In this particular case, memoization was explicitly requested (see #345).

I'm always interested in feedback from the community regarding ways to make things less confusing (eg better docs, better default behaviors, etc) so if you have any suggestions in this case- please let me know. I will be updating the InfiniteLoader docs right now for what it's worth to call out this behavior more explicitly.

Thanks again for the fantastic bug report and repro. 😄

[bvaughn]

Here's a new section in the InfiniteLoader docs that will hopefully help you and others: https://github.com/bvaughn/react-virtualized/blob/master/docs/InfiniteLoader.md#edge-cases-and-considerations

[SpencerCarstens]

Thanks for the gist and the information!!

I'm looking it all over now.

One thing you may want to look at is this example as it falls into the some of the pitfalls that were surfaced here.

What I really liked about that example is that it only showed one loading row per data pull, so I'm going to see if I can find an elegant way to incorporate that with the guidance you have given me.

It's particularly useful in my case, because the dataset that I'm working with does not give you result counts before you have them. So I would like to avoid the possibility of having 100 loading rows and then only getting 1 api result back... if that makes sense.

[bvaughn]

One thing you may want to look at is this example as it falls into the some of the pitfalls that were surfaced here.

Not sure I understand. Are you saying that I should update the example to make it easier to avoid some of this? (I think that's what you're saying but I'm not sure.) 😁

What I really liked about that example is that it only showed one loading row per data pull, so I'm going to see if I can find an elegant way to incorporate that with the guidance you have given me.

Ah. You could achieve the same thing with the gists we've been sharing here. I didn't know it was desired. 😄 All you would need to change is the _getRowCount function:

  _getRowCount () {
    const { listSize, loadedRowCount, numLoadingRows } = this.state

    // If we have more rows to load, or are currently loading rows, render a placeholder (unloaded row) to trigger the next batch
    if (loadedRowCount < listSize) {
      return loadedRowCount + 1
    // If we've loaded all rows, render the full list
    } else {
      return listSize
    }
  }

[SpencerCarstens]

Yep! I ended up doing something similar. 😄

[SpencerCarstens]

I was thinking, how would you feel about a public function on InfiniteLoader to clear the memoized data? The reason I ask is that I've ran into another snag, but this time with a result set of zero. While I can get around it by forcing this._loadMoreRows using componentDidUpdate, managing when ( much more importantly when not! ) to do that is hairy ( in my situation at least ). Being able to clear the memoized cache manually when I call my this._clearData() function would be ideal ( for me at least ).

[bvaughn]

Hm. I'm kind of reluctant to add it because it feels to me like something that belongs in user-land. (After all, the data is being invalidated there- so it seems reasonable to me for loadMoreRows to be manually called there as well.)

I'd need to know more info about your particular situation I guess. I don't know why using componentDidUpdate to look for a changed prop (or props) wouldn't work. 😄

[SpencerCarstens]

I'd need to know more info about your particular situation I guess. I don't know why using componentDidUpdate to look for a changed prop (or props) wouldn't work. 😄

Well, it does work, but not ideally. 😛 _loadMoreRows gets called twice, and therefore does two API calls ( your gist does this as well ). Granted, this can be worked around by making a check for it being an initial call or something like that. I was just hoping to have to avoid that kind of thing. 😄

[bvaughn]

_loadMoreRows gets called twice, and therefore does two API calls

Why? The updated listSize (in my gist, which is kind of a silly example) only changes once. So what triggers the 2nd call?

[SpencerCarstens]

Fixed my issue. I had an embarrassing math typo. 😅

[bvaughn]

Hah! 😁 No worries. Been there, done that.