Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix problems with feed loading #1623

Merged
merged 1 commit into from
Dec 6, 2024

Conversation

micahmo
Copy link
Member

@micahmo micahmo commented Dec 5, 2024

Pull Request Description

Apologies in advance for the wall of text, but I did a deep dive into this issue and I really wanted to explain the thought process. Hopefully it all makes sense!

Background

I decided to take a look at the issue I mentioned here, which is that, when scrolling down in the feed now, I always see "No more items to load" instead of the spinner. However, the feed is loading more content, so it's not true that there are no more items to load.

I wanted to understand why the original change was made in #1596; however, in researching the original issue (#1592) further, I found that with or without the fix, I still encountered the issue. Here's what I did to reproduce.

  • I created a new test community: https://lemmy.thunderapp.dev/c/1592
  • I created a post with the character b. Then I created 20 posts with the character a.
  • In Thunder, I added a filter on a. The idea here is that, when sorting by New, the first page worth of posts should be filtered, and Thunder should need to query the next page in order to find any valid posts to display.
  • What I found is that, with or without the fix from #1596, I always got an empty feed.
image

The issue seems to be caused by the fact that when the initial call to fetchFeedItems returns empty, we never attempt to fetch more. (Normally fetching more items would be triggered by a scroll action, but there's nothing to scroll when the feed is empty.) Ideally, we would want to keep fetching until we either hit the end or have enough posts for the user to scroll.

This got me thinking that the limit of 20 could be treated more like a "desired amount" of posts. While we don't want to go much over 20 (at least we don't want to load the whole feed), it would also be nice if we could either get at least 20 posts or hit the end of the feed. Therefore, I tweaked the logic to keep trying until we satisfy one of those conditions. I also removed limit as a parameter since no one was passing it in. And I also changed hasReachedPostsEnd so that it's not set to true until we really hit the end of the feed (to fix the spinner problem).

Now the small downside of this approach is that we might get over 20. In the worst case scenario, we can have 19 posts and then make another API call that returns 10, so we end up with 29. But in my opinion, that's not the end of the world. The main thing we're trying to prevent with the limit is hitting the API constantly until we load the whole feed, which would defeat the purpose of paging.

At the end of these fixes, both the original issue (the feed displaying nothing when the first page is all filtered) and the issue I found (the end of the feed always displaying "No more items" when there are more items) were fixed!

image

However, there's one more little wrinkle. What if you have a filter so aggressive that many, many API calls still return no results, so your feed is just spinning for a while. Well on the one hand, you might be willing to wait (if your filter is very important). On the other, you might not know what's going on (is my internet slow, is my instance down?). Here you can see when I visit lemmy.ca with a filter on the letter w, it takes about 6 seconds and 20 API calls to load enough posts.

load.feed.filter.w.mp4

To address this potential concern, once we've hit 20 API calls for a single fetchFeedItems, we'll show a snackbar to the user telling them that their feed is loading slowly due to filters, with a deep link to change filters if desired.

qemu-system-x86_64_KZI4Tr0hm0.mp4

Again I hope that all makes sense and is a reasonable solution to all of the problems mention!

Issue Being Fixed

Issue Number: #1596 (comment)

Screenshots / Recordings

See above.

Checklist

  • If a new package was added, did you ensure it uses an appropriate license and is actively maintained?
  • Did you use localized strings (and added appropriate descriptions) where applicable?
  • Did you add semanticLabels where applicable for accessibility?

Copy link
Member

@hjiangsu hjiangsu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for diving into this and writing up the detailed information!

Just had a couple of ideas that floated around while I was reading the description. These don't have to be implemented here but would be good to keep in mind:

  • If a filter is too generic and causes too many API calls, we should ideally have a delay between each API call so that it doesn't spam the instance since it just places additional stress on the instance (e.g., perhaps we could use exponential backoff here if it exceeds 5 API calls for fetching the feed)
  • I'm not entirely sure about this as I haven't checked, but Lemmy might provide back headers on its API response indicating the current rate limits for the instance. If this is the case, then we could possibly expose that in the Lemmy API and limit the number of API calls based on the header information

Aside from those thoughts, I think this is a nice fix! I'll go ahead and merge it in so that I can also do some testing on it

@hjiangsu hjiangsu merged commit 9fa125f into thunder-app:develop Dec 6, 2024
1 check passed
@micahmo micahmo deleted the fix/feed-bottom-spinner branch December 6, 2024 04:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants