vueuse icon indicating copy to clipboard operation
vueuse copied to clipboard

useInfiniteScroll - broken on v10.2.1

Open francoism90 opened this issue 1 year ago • 27 comments

Describe the bug

I use this to scroll on the body:

useInfiniteScroll(document, fetch, {
  distance: 10
})

However upgrading to v10.2.1, it doesn't seem to work anymore. Downgrading to v10.2.0 fixes the issue.

I think this is the cause: https://github.com/vueuse/vueuse/pull/3143

Reproduction

https://stackblitz.com/edit/vitejs-vite-zf9drc

System Info

System:
    OS: Linux 6.3 Alpine Linux
    CPU: (16) x64 AMD Ryzen 7 5700G with Radeon Graphics
    Memory: 19.14 GB / 30.70 GB
    Container: Yes
    Shell: 1.36.0 - /bin/ash
  Binaries:
    Node: 18.16.0 - /usr/bin/node
    npm: 9.6.6 - /usr/bin/npm
  npmPackages:
    @vueuse/components: ^10.2.1 => 10.2.1 
    @vueuse/core: ^10.2.1 => 10.2.1
    @vueuse/head: ^1.1.26 => 1.1.26 
    vue: ^3.3.4 => 3.3.4

Used Package Manager

npm

Validations

  • [X] Follow our Code of Conduct
  • [X] Read the Contributing Guidelines.
  • [X] Read the docs.
  • [X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
  • [X] Make sure this is a VueUse issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to https://github.com/vuejs/core instead.
  • [X] Check that this is a concrete bug. For Q&A open a GitHub Discussion.
  • [X] The provided reproduction is a minimal reproducible example of the bug.

francoism90 avatar Jul 01 '23 13:07 francoism90

I can confirm, its broken in latest version

genu avatar Jul 02 '23 13:07 genu

I'm very sorry, but https://github.com/vueuse/vueuse/pull/3212 didn't seem to fix the issue. :/

useInfiniteScroll(document, fetch, {
  distance: 10
})

As you see, I'm using document, so it scrolls on the body.

Edit: I'm not seeing any error, fetch isn't being called at all.

francoism90 avatar Jul 30 '23 16:07 francoism90

image

I've used your reproduction code and updated the package version, which seems to be fixed. I'm considering the possibility that your browser version might not be supported with IntersectionObserver, could that be the case?

erikkkwu avatar Jul 31 '23 06:07 erikkkwu

I'm very sorry, but #3212 didn't seem to fix the issue. :/

useInfiniteScroll(document, fetch, {
  distance: 10
})

As you see, I'm using document, so it scrolls on the body.

Edit: I'm not seeing any error, fetch isn't being called at all.

if you use the following code, you'll actually find that the console doesn't print out any log. However, you can see that the request has been made if you look at the network tab. image

useInfiniteScroll(document, fetch, {
  distance: 10
})

erikkkwu avatar Jul 31 '23 06:07 erikkkwu

@erikkkwu It simply doesn't call anything. I tried Firefox and Brave, they don't call 'fetch' (or anything else) when reaching the button of the page.

It does work fine on [v10.2.0](https://github.com/vueuse/vueuse/releases/tag/v10.2.0) (and previous versions). so I don't think it's the observer.

francoism90 avatar Jul 31 '23 07:07 francoism90

So I did some more testing, and I believe it's completely broken now:

image

useInfiniteScroll(document, () => console.log('fetch'), {
  distance: 10
})

This happens when I just scroll to the end of the page, it keeps calling this over-and-over.. and never stops.

Weird thing, is when I call a method (useInfiniteScroll(document, fetch, ..)), it does call the method, but only one time after, and never again later. It also does call fetch 2x/3x on start, instead of the usual only when reaching the bottom of the page.

francoism90 avatar Jul 31 '23 07:07 francoism90

This happens when I just scroll to the end of the page, it keeps calling this over-and-over.. and never stops.

I've found that this is the original behavior. Because when onLoadMore is called, if your callback data can't stretch the scroll height, it will execute again, leading to an infinite loop. this behavior is consistent in both versions 10.2.0 and 10.3.0 when using the same test case.

Weird thing, is when I call a method (useInfiniteScroll(document, fetch, ..)), it does call the method, but only one time after, and never again later. It also does call fetch 2x/3x on start, instead of the usual only when reaching the bottom of the page.

As mentioned above, if there's no height initially, it will immediately trigger onLoadMore. However, if you can stretch the scroll height afterward, it will stop.

infinite loop caused by the inability to extend the height could be another point that needs to fix.

It's also possible that there are cases I haven't considered. If it's convenient for you, could you provide a more detailed case for me to test? thanks.

erikkkwu avatar Jul 31 '23 13:07 erikkkwu

@erikkkwu After more debugging, I think the problem is this line:

const set = (obj: Response) => {
    state.data = state.data.concat(obj.data) // <-- this one, array of objects
    state.links = obj.links
  }

If I use push instead, it also doesn't work:

const set = (obj: Response) => {
    state.data.push(...obj.data)
    state.links = obj.links
  }

This does work:

state.data.push(<BookModel>{id: 'test', name: 'test' })

I don't know why this has been changed? I think I need to use concat, as I already have data.

francoism90 avatar Jul 31 '23 15:07 francoism90

So I've moved to useScroll instead, I don't know what happend to useInfiniteScroll, but it just doesn't work anymore.

It's a bit ugly, but it also brings some new features:

const { arrivedState } = useScroll(document, {
  throttle: 100,
  behavior: 'smooth'
})

onMounted(() => initialize())
watch(arrivedState, (state) => (state.bottom ? fetch() : null))

francoism90 avatar Aug 01 '23 10:08 francoism90

hi @francoism90 you're saying that useInfiniteScroll is executed, but the state inside can no longer be updated using push after version 10.2.0?

Which state management are you using ?

erikkkwu avatar Aug 01 '23 10:08 erikkkwu

@erikkkwu I'm using Vue3 composables, like this:

const state = reactive(<BookState>{
  data: new Array(),
})

export function useBooks() {
  // initialize
  // fetch
  // reset
  // ..
}

It works fine when using useScroll, and it worked fine on 10.2.0.

It's really weird, it does call fetch (method used to merge next page results), but only once. Not only that, but it also seems to call it random, like before the actual component render and sometimes after. I'm using async components. I would expect it only calls fetch when actually scrolling to the bottom of the page.

francoism90 avatar Aug 01 '23 11:08 francoism90

hi @francoism90 I've tried to reproduce your issue, but I couldn't. However, your suggestion to only trigger the fetch when reaching the bottom instead of at the start seems like a new requirement that could be explored. https://stackblitz.com/edit/vitejs-vite-xumn4q?file=src%2FApp.vue

erikkkwu avatar Aug 02 '23 10:08 erikkkwu

@erikkkwu I think there is still an issue when working with Window and Document

Here, we state that Document and Window cannot be observed by IntersectionObserver, but it seems that useElementVisibility is using IntersectionObserver under the hood.

In my testing I noticed that isElementVisible will always be false when el is a Document instance.

https://github.com/vueuse/vueuse/blob/028a732333ea6792a984f39c18f1fdad8b0587bd/packages/core/useInfiniteScroll/index.ts#L60-L72

Can we bypass the whole visibility check for Document and Window? Are there any cases when visibility for those matter?

genu avatar Aug 07 '23 05:08 genu

@erikkkwu I think there is still an issue when working with Window and Document

Here, we state that Document and Window cannot be observed by IntersectionObserver, but it seems that useElementVisibility is using IntersectionObserver under the hood.

In my testing I noticed that isElementVisible will always be false when el is a Document instance.

https://github.com/vueuse/vueuse/blob/028a732333ea6792a984f39c18f1fdad8b0587bd/packages/core/useInfiniteScroll/index.ts#L60-L72

Can we bypass the whole visibility check for Document and Window? Are there any cases when visibility for those matter?

hi @genu my logic here involves converting the Window or Document into a root element so that it can be used by the IntersectionObserver.

I've previously created a bypass version, but it ended up being relatively verbose and harder to read. because the element is a MaybeRefOrGetter, I need to monitor changes to the element to determine whether it should use the IntersectionObserver.

But I can't reproduce a situation where isElementVisible for Window or Document is always false. Could you please create a reproduction so I can understand the case under which it is always false? thanks

erikkkwu avatar Aug 07 '23 07:08 erikkkwu

@erikkkwu Isn't this possible when using async components?

I will try to release some code today.

francoism90 avatar Aug 07 '23 07:08 francoism90

@erikkkwu I've had a hard time creating a minimal reproduction, as I can only see the problem in a private project.

After some further debugging, I'm starting to think that in fact, there isn't a bug in the useInifiniteScroll but rather issues can arise when css or certain styles cause the height of the elements being observed to be wrong.

For example, in my case, the rendered height of the <html> and <body> were not the full height of the scroll area.

Notice in the screenshot below: Upon scrolling down, the html height is not longer the full height.

CleanShot 2023-08-07 at 14 35 11

@francoism90 Maybe check the styles of the elements involved.

genu avatar Aug 07 '23 18:08 genu

hi @genu i think always false is related to this #3305

erikkkwu avatar Aug 11 '23 05:08 erikkkwu

yup it's broken

julienreszka avatar Aug 11 '23 14:08 julienreszka

@genu It's not the style, I tried multiple things, like applying overflow, removing overflow, change heights, use it on different elements.. but it's the window/document that is broken.

francoism90 avatar Aug 19 '23 09:08 francoism90

I've tried the latest version with window/document it's broken for me also. When I downgrade to 10.2.0 it works.

gazben avatar Aug 23 '23 14:08 gazben

+1

shynline avatar Sep 21 '23 13:09 shynline

+1

grindpride avatar Sep 22 '23 13:09 grindpride

hello, @shynline @grindpride could you provide a minimal reproduction for me? thanks

erikkkwu avatar Sep 22 '23 14:09 erikkkwu

@erikkkwu I checked my case - it no longer reproduces on the current version 🥳

grindpride avatar Oct 07 '23 13:10 grindpride

@erikkkwu I tested the latest version and it's working. Maybe it got fixed and it's safe to close this issue ?

shynline avatar Oct 09 '23 16:10 shynline

Can confirm. Updating to the latest version seems to have fixed the issue.

DerZade avatar Nov 28 '23 09:11 DerZade

10.7.1 does not fix the original issue for me. Still works on 10.2.0. I have an open-source project with the issue, but it requires PHP to run.

Fixing my CSS did work though, as pointed out by @genu it was a height issue on the <html> element 👍

innocenzi avatar Jan 01 '24 06:01 innocenzi