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

Run watcher post DOM update in Vue adapter #716

Merged
merged 2 commits into from
Apr 26, 2024
Merged

Run watcher post DOM update in Vue adapter #716

merged 2 commits into from
Apr 26, 2024

Conversation

markjaniczak
Copy link
Contributor

This solves #715

According to the Vue docs; watchers will run their effects after state updates but before DOM updates. The ref attribute performs the same way which is why some users discovered they need to wrap their measureElement calls with nextTick in order to measure elements in a post update state (#619).

Currently the vue adapter will update state pre DOM update which causes an infinite loop if the estimated size of an element doesn't exactly match the measured size. Simply running the effect post DOM update will ensure that the elements are measured and taken into account before virtualizer updates its state.

@markjaniczak markjaniczak changed the title Run watcher p Run watcher post DOM update in Vue adapter Apr 26, 2024
Copy link

nx-cloud bot commented Apr 26, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 70f8dec. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

@piecyk
Copy link
Collaborator

piecyk commented Apr 26, 2024

@markjaniczak great thanks 👏 btw not that familiar with vue, why we need the first watch in adapter, this part

watch(
() => unref(options).getScrollElement(),
(el) => {
if (el) {
virtualizer._willUpdate()
}
},
{
immediate: true,
},
)

isn't it redundant as second one is watch whole options 🤔

@piecyk
Copy link
Collaborator

piecyk commented Apr 26, 2024

@markjaniczak can you run formatting with pnpm prettier:write

@markjaniczak
Copy link
Contributor Author

@markjaniczak great thanks 👏 btw not that familiar with vue, why we need the first watch in adapter, this part

watch(
() => unref(options).getScrollElement(),
(el) => {
if (el) {
virtualizer._willUpdate()
}
},
{
immediate: true,
},
)

isn't it redundant as second one is watch whole options 🤔

I'm not very familiar with Vue either; I'm usually a React guy.

That said, I believe both are still necessary. The first watcher will react to changes of what getScrollElement returns and the second watcher will react to changes of the function's signature (if the function was saved as a vue ref).

@piecyk piecyk merged commit 9871f58 into TanStack:main Apr 26, 2024
2 checks passed
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