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

Infinite loop in Vue adapter when estimated size doesn't match dynamic size #715

Open
2 tasks done
markjaniczak opened this issue Apr 25, 2024 · 4 comments
Open
2 tasks done

Comments

@markjaniczak
Copy link
Contributor

markjaniczak commented Apr 25, 2024

Describe the bug

I'm using @tanstack/vue-virtual to render a dynamic list of elements and on specific browsers at specific resolution scaling virtualizer will end up in an infinite loop. I've found these previous issues which seem somewhat related:

#619 (comment)
I put my measure function inside of a nextTick as to prevent that same issue.

#159 (comment)
I found the same issue on my windows machine at 125% scaling; what should be integer pixel sizes are read as decimal pixel sizes when calling getBoundingClientRect inside of my measure function.

In my actual app my list item elements will typically be one of two sizes as they conditionally render a line of text if it's available in the item. My codesandbox example is not exactly the same scenario as I've not been able to reproduce sub pixel sizes exactly but I've attached a couple of videos to help show what's happening.

Your minimal, reproducible example

https://codesandbox.io/p/devbox/runtime-river-jw2kt4

Steps to reproduce

  1. Load the page
  2. Observe that the page is frozen in an infinite loop

Expected behavior

As a user I was expecting the virtualizer to behave the same way on all screens regardless of pixel density.

How often does this bug happen?

Every time

Screenshots or Videos

git.issue.mp4

Platform

Windows 11 Pro 23H2
Edge 123.0.2420.97

tanstack-virtual version

3.4.0

TypeScript version

5.2.2

Additional context

This issue was first noticed on a chromebook screen that had resolution scaling set to 125%. When it was changed to 100% the problem no longer appeared. On the same machine, I could move the window to a connected monitor that was set to 1920x1080 at 100% scaling and couldn't reproduce the issue either.

Terms & Code of Conduct

  • I agree to follow this project's Code of Conduct
  • I understand that if my bug cannot be reliable reproduced in a debuggable environment, it will probably not be fixed and this issue may even be closed.
@piecyk
Copy link
Collaborator

piecyk commented Apr 25, 2024

hmm @markjaniczak do you use some custom measureElement method, default one already use Math.round for exactly this reason https://github.com/TanStack/virtual/blob/main/packages/virtual-core/src/index.ts#L197-L216

@markjaniczak
Copy link
Contributor Author

markjaniczak commented Apr 25, 2024

I don't, no. Although I did try supplying my own measureElement and used different methods of rounding and even just returned the decimal value from it.

It doesn't happen when I move measureElement out of the nextTick function so I think it's something to do with the order of how Vue renders after a state change. But I can't do that because then I face the same scrolling issue mentioned in #619 (comment)

@markjaniczak
Copy link
Contributor Author

I've added some logging around here

private notify = (force: boolean, sync: boolean) => {
const { startIndex, endIndex } = this.range ?? {
startIndex: undefined,
endIndex: undefined,
}
const range = this.calculateRange()
if (
force ||
startIndex !== range?.startIndex ||
endIndex !== range?.endIndex
) {
this.options.onChange?.(this, sync)
}
}

and noticing that the calculated range bounces back and forth:
image

@markjaniczak
Copy link
Contributor Author

markjaniczak commented Apr 26, 2024

I've done a little more digging and found that the issue actually occurs when the estimated sizes don't match the measured sizes and you've wrapped the measureElement function inside a nextTick. But using nextTick seems necessary in order to prevent the scroll issue mentioned in the other issue.

Perhaps the vue adapter also needs to defer state updates with nextTick?

watch(
() => unref(options),
(options) => {
virtualizer.setOptions({
...options,
onChange: (instance, sync) => {
triggerRef(state)
options.onChange?.(instance, sync)
},
})
virtualizer._willUpdate()
triggerRef(state)
},
{
immediate: true,
},
)
onScopeDispose(cleanup)
return state
}

@markjaniczak markjaniczak changed the title Infinite loop with sub pixel sizes Infinite loop in Vue adapter when estimated size doesn't match dynamic size Apr 26, 2024
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

2 participants