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

useInfiniteScroll - broken on v10.2.1 #3201

Open
7 tasks done
francoism90 opened this issue Jul 1, 2023 · 27 comments
Open
7 tasks done

useInfiniteScroll - broken on v10.2.1 #3201

francoism90 opened this issue Jul 1, 2023 · 27 comments

Comments

@francoism90
Copy link
Contributor

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: #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

@genu
Copy link
Contributor

genu commented Jul 2, 2023

I can confirm, its broken in latest version

@francoism90
Copy link
Contributor Author

francoism90 commented Jul 30, 2023

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.

@erikkkwu
Copy link
Contributor

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
Copy link
Contributor

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
})

@francoism90
Copy link
Contributor Author

@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
Copy link
Contributor Author

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.

@erikkkwu
Copy link
Contributor

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.

@francoism90
Copy link
Contributor Author

@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
Copy link
Contributor Author

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))

@erikkkwu
Copy link
Contributor

erikkkwu commented Aug 1, 2023

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 ?

@francoism90
Copy link
Contributor Author

francoism90 commented Aug 1, 2023

@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.

@erikkkwu
Copy link
Contributor

erikkkwu commented Aug 2, 2023

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

@genu
Copy link
Contributor

genu commented Aug 7, 2023

@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.

// Document and Window cannot be observed by IntersectionObserver
const observedElement = computed<HTMLElement | SVGElement | null | undefined>(() => {
const el = toValue(element)
if (el instanceof Window)
return window.document.documentElement
if (el instanceof Document)
return document.documentElement
return el
})
const isElementVisible = useElementVisibility(observedElement)

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

@erikkkwu
Copy link
Contributor

erikkkwu commented Aug 7, 2023

@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.

// Document and Window cannot be observed by IntersectionObserver
const observedElement = computed<HTMLElement | SVGElement | null | undefined>(() => {
const el = toValue(element)
if (el instanceof Window)
return window.document.documentElement
if (el instanceof Document)
return document.documentElement
return el
})
const isElementVisible = useElementVisibility(observedElement)

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

@francoism90
Copy link
Contributor Author

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

I will try to release some code today.

@genu
Copy link
Contributor

genu commented Aug 7, 2023

@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.

@erikkkwu
Copy link
Contributor

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

@julienreszka
Copy link

yup it's broken

@francoism90
Copy link
Contributor Author

@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.

@gazben
Copy link

gazben commented Aug 23, 2023

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

@shynline
Copy link

+1

1 similar comment
@grindpride
Copy link

+1

@erikkkwu
Copy link
Contributor

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

@grindpride
Copy link

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

@shynline
Copy link

shynline commented Oct 9, 2023

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

@DerZade
Copy link
Contributor

DerZade commented Nov 28, 2023

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

@innocenzi
Copy link
Contributor

innocenzi commented Jan 1, 2024

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 👍

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

No branches or pull requests

9 participants