LOW: Migrate the main chat list to FlashList

[roryabraham]

HOLD on:

• Comment linking to be 100% complete
• The new architecture to be enabled (this is not a hard requirement but I have a hunch that this should wait on that too)

Problem

The main chat list is one of the most, if not the most important component in our app. If you scroll far or fast on this list, you may see frames drop. Furthermore, there are some known performance issues with this list (example). We have already migrated almost every other virtualized list in our app from FlatList to FlashList, because its performance is much better.

Solution

Let's build support for bidirectional pagination in FlashList, then enable it on the main chat list in E/App.

[melvin-bot]

Triggered auto assignment to @garrettmknight (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.

[roryabraham]

I expect this to be on HOLD until sometime in February, if not later, so I'm moving this back to monthly

[roryabraham]

cc @muttmuure adding this to the New Expensify performance improvements project. I acknowledge that the problem statement is a bit of a reverse-solution but I think it's a good bet that this will provide substantial performance improvements, as FlashList is just much better for performance than FlatList. We've established that elsewhere where we've migrated to FlashList

[roryabraham]

Still on HOLD

[roryabraham]

on HOLD for comment linking

[roryabraham]

HOLD, but could come off HOLD soon. Unclear if we'll prioritize

[garrettmknight]

@roryabraham we good to take this off hold? If so, where do you think it might fit in a wave, if any? I'm not 100% on the significance of this migration beyond general, better performance.

[roryabraham]

Good question. As far as I know there's no significance beyond general, better performance of chat screens.

[perunt]

@roryabraham, you’re absolutely right about the performance perks we’d get with FlashList—better speed, faster rendering, the whole deal. But there's a bit of a twist when it comes to the bidirectional scrolling we need. Right now, adding that feature to FlashList can lead to some choppy experiences. It might slow us down a bit to get that smoothed out.

There is a PR with the implementation of a bidirectional list, but it is a bit stale (Shopify/flash-list#824)
also would be nice to double-check this approach (https://github.com/irisjae/recyclerlistview/blob/master/README.md). This guy added something called preserveVisiblePosition, which claims to fix those choppy scrolling issues. I haven't tried it yet, but worth to try

[garrettmknight]

Still on hold.

[garrettmknight]

@roryabraham any idea where this fits priority wise?

[muttmuure]

I think this can come off hold now?

[garrettmknight]

@roryabraham can this come of hold?

[muttmuure]

@janicduplessis is going to investigate this

[janicduplessis]

I will take this :)

[janicduplessis]

After first investigation of this there are 2 main things we need to implement:

1. Support onStartReached. This one is pretty simple, I already have a working prototype.
2. Support maintainVisibleContentPosition (or some alternative implementation that does the same thing). I found why maintainVisibleContentPosition does not work with FlashList, it has to do with the view hierarchy it creates. The maintainVisibleContentPosition implementation assumes ScrollView -> ContentView -> Cells, but FlashList creates ScrollView -> ContentView -> AutoLayout -> Cells.

For #2 I am currently investigating the different approaches we can use to fix this.

[roryabraham]

I'm headed on parental leave. @mountiny going to reassign to you as this is part of #newdot-quality.

[mountiny]

@janicduplessis How is this one looking? What are the next steps and ETA?

[janicduplessis]

Picking up work on this this week. Will provide another update and ETA soon.

[janicduplessis]

Update: I abandoned the maintainVisibleContentPosition route as it isn't really compatible with how recyclerlistview handles virtualization.

Instead I am currently looking at using a redesigned version of flash-list / recyclerlistview, the changes made are described in details here (really suggest going through it, very interesting). In summary it changes the virtualization algorithm to have first class support for keeping the visible content position.

I managed to get it working in expensify app, the main work needed was to get it working on new arch and add support for onStartReached. It seem to work really well except for a pretty big flicker when it adjusts the position of items in one case. I have some ideas on how to fix this. I am pretty optimistic that this is a great solution and will continue working in that direction.

Will post some demo and code later.