Skip to content

Conversation

@nhquyss
Copy link
Contributor

@nhquyss nhquyss commented Oct 26, 2025

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

Description

Follow-up improvements to #5110

For detailed, see this comment in PR

Changes

  • Move promise check to early return to prevent unnecessary logic execution

  • Add flush: 'post' to watch to ensure it runs after DOM updates

Additional context

@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Oct 26, 2025
@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 26, 2025

Open in StackBlitz

@vueuse/components

npm i https://pkg.pr.new/@vueuse/components@5122

@vueuse/core

npm i https://pkg.pr.new/@vueuse/core@5122

@vueuse/electron

npm i https://pkg.pr.new/@vueuse/electron@5122

@vueuse/firebase

npm i https://pkg.pr.new/@vueuse/firebase@5122

@vueuse/integrations

npm i https://pkg.pr.new/@vueuse/integrations@5122

@vueuse/math

npm i https://pkg.pr.new/@vueuse/math@5122

@vueuse/metadata

npm i https://pkg.pr.new/@vueuse/metadata@5122

@vueuse/nuxt

npm i https://pkg.pr.new/@vueuse/nuxt@5122

@vueuse/router

npm i https://pkg.pr.new/@vueuse/router@5122

@vueuse/rxjs

npm i https://pkg.pr.new/@vueuse/rxjs@5122

@vueuse/shared

npm i https://pkg.pr.new/@vueuse/shared@5122

commit: a5c1e16

@codecov
Copy link

codecov bot commented Oct 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.33%. Comparing base (1808c65) to head (a5c1e16).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5122      +/-   ##
==========================================
- Coverage   64.38%   64.33%   -0.05%     
==========================================
  Files         343      343              
  Lines        6587     6581       -6     
  Branches     2002     1996       -6     
==========================================
- Hits         4241     4234       -7     
- Misses       1945     1946       +1     
  Partials      401      401              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nhquyss nhquyss force-pushed the fix/useInfiniteScroll-watch-flush branch 3 times, most recently from 3d6c06f to 04fcda3 Compare November 5, 2025 15:45
@OrbisK
Copy link
Member

OrbisK commented Nov 6, 2025

Thank you 🙏🏽

Would you mind adding some tests to cover the new handling?

@maximepvrt
Copy link

@nhquyss

@nhquyss nhquyss force-pushed the fix/useInfiniteScroll-watch-flush branch from af98d59 to de3d12d Compare November 23, 2025 15:56
@nhquyss
Copy link
Contributor Author

nhquyss commented Nov 23, 2025

Thank you 🙏🏽

Would you mind adding some tests to cover the new handling?

Hi @OrbisK, sorry for the late reply.

I’ve tried adding tests for this change, but I still haven’t been able to write one that reproduces the issue.​

So I created a small live demo to show the behavior before and after this change.

Stackblitz live demo

Without flush: 'post', the API is called twice on the initial load even though the first call already returns enough items to fill the screen.

Hope this demo helps clarify the issue!!

@nhquyss
Copy link
Contributor Author

nhquyss commented Nov 23, 2025

@nhquyss

@maximepvrt, thanks for the reminder :)

I've added a demo above.

Let me know if you need anything else!!

@maximepvrt
Copy link

Hi @nhquyss, thanks for the update!

I’ve just tested with the newly released v14.1.0 (which includes #5110) as well as with the PR build from pkg.pr.new.

I’m running into an issue: it triggers infinite loading of my data.

Here is a reproduction of the issue:
https://stackblitz.com/edit/github-ostrvl2k

Do you have any idea what might be causing this?

…o watch

- Move promise check to early return to prevent unnecessary logic execution
- Add flush: 'post' to watch to ensure it runs after DOM updates
@nhquyss nhquyss force-pushed the fix/useInfiniteScroll-watch-flush branch from de3d12d to a5c1e16 Compare November 29, 2025 08:04
@nhquyss
Copy link
Contributor Author

nhquyss commented Nov 29, 2025

Hi @nhquyss, thanks for the update!

I’ve just tested with the newly released v14.1.0 (which includes #5110) as well as with the PR build from pkg.pr.new.

I’m running into an issue: it triggers infinite loading of my data.

Here is a reproduction of the issue: https://stackblitz.com/edit/github-ostrvl2k

Do you have any idea what might be causing this?

@maximepvrt

I think the scroll container in your Items.vue doesn't have a fixed height or max-height set.

This causes useInfiniteScroll to continuously trigger the load function because of this condition check in the composable:

const isNarrower =
  direction === "bottom" || direction === "top"
    ? scrollHeight <= clientHeight
    : scrollWidth <= clientWidth;

Without a defined height, scrollHeight always equals clientHeight, making the condition for loading the next page always true, which leads to infinite loading.

I've created a demo based on your reproduction with a max-height applied to the scroll container. This way, the next page will only be loaded when you actually scrolls to the bottom of the container.

Or if you prefer full-page scrolling, you can use useInfiniteScroll composable with document or window as the scroll element instead of a specific container:

useInfiniteScroll(
  document,
  async () => {
    await refresh();
  },
  {
    canLoadMore: () => canLoadMore.value && props.infiniteScroll,
    distance: 100,
  }
);

Hope this helps resolve your issue!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants