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

fix(visual-editing): prevent snapshot overfetching #2615

Merged
merged 1 commit into from
Feb 5, 2025

Conversation

rdunk
Copy link
Member

@rdunk rdunk commented Feb 5, 2025

It looks like 74fdcdd introduced some changes in the DocumentReporter component which removed the unique document ID check.

This is causing documents to be unobserved/observed on every render—such as hovering on/off overlay—even if the IDs haven't changed, which in turn triggers a request to Presentation to fetch new document snapshots for every document on the page. This can result in hundreds of requests on each interaction. See video below.

This PR reverts that change, and should fix this issue.

Kapture.2025-02-05.at.01.49.06.mp4

@rdunk rdunk requested a review from a team as a code owner February 5, 2025 01:58
Copy link

vercel bot commented Feb 5, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
live-visual-editing-next ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 5, 2025 1:58am
visual-editing-astro ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 5, 2025 1:58am
visual-editing-next ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 5, 2025 1:58am
visual-editing-next-with-i18n ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 5, 2025 1:58am
visual-editing-nuxt ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 5, 2025 1:58am
visual-editing-page-builder-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 5, 2025 1:58am
visual-editing-remix ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 5, 2025 1:58am
visual-editing-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 5, 2025 1:58am
visual-editing-svelte ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 5, 2025 1:58am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
visual-editing-studio ⬜️ Skipped (Inspect) Feb 5, 2025 1:58am

@rdunk rdunk requested a review from stipsan February 5, 2025 01:59
Copy link
Member

@stipsan stipsan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch!! 😱

@@ -323,6 +330,7 @@ export const Overlays: FunctionComponent<{
}, [overlayEnabled])

const documentIds = useMemo(() => {
console.log('recalc documentIds')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
console.log('recalc documentIds')

@stipsan stipsan merged commit 5533a2b into main Feb 5, 2025
15 of 16 checks passed
@stipsan stipsan deleted the fix/snapshot-fetching branch February 5, 2025 07:38
@ecospark ecospark bot mentioned this pull request Feb 5, 2025
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