-
-
Notifications
You must be signed in to change notification settings - Fork 232
fix(ui): view transitions hangs browser instance #1278
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThe plugin app/plugins/view-transitions.client.ts now wraps document.startViewTransition in a try/catch. On success it starts the transition, awaits the page:view-transition:start hook, returns the transition promise and wires transition.finished to a finally handler to reset state. On failure it logs the error, invokes changeRoute(), completes finishTransition, and calls resetTransitionState. changeRoute is initialised to ensure a defined function before use. The page:finish hook no longer auto-resets state and relies on transition.finished for reset. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
|
can you reproduce the bug? |
it is hard, since it happens from time to time when navigating to some package page, on local dev or npmx.dev or pr preview (maybe a page refresh, maybe when searching (a lot of |
|
Could this also solve this, I could be wrong? #1234 I looked at it briefly and I locally got out of memory errors when trying to fetch the docs since some package could not be found on esm. And expected to see the no docs found page. This did happen after a refresh sometimes. |
|
no, this isn't #1234 |
|
@userquin i actually disabled view transitions for the site earlier today except for index <> search pages so it's unlikely to be related unless you're on an outdated branch I don't think this pr is correct let me know if you experience any issues from now on - it might have been fixed by my change already |
|
The main branch on my local contains the same view transition code at main here, maybe the issue when navigating npmx.dev, just add on-hold label to this PR, will try to reproduce the issue with npmx.dev (should contain your fix). |
From time to time my chrome browser hangs, disabling view transitions seems to fix the issue and so I asked Gemini PRO, the issue is gone on my local applying the suggestion, here the reasoning:
Analysis of the View Transitions Deadlock
This PR fixes a critical browser freeze issue caused by a promise deadlock within the View Transitions plugin.
The Flow & The Deadlock
router.beforeResolveis triggered. It creates areadypromise and returns it. Vue Router pauses navigation until this promise resolves.document.startViewTransition(callback)is called. The browser captures the current state (screenshot) and then executes thecallback.changeRoute()is called, which resolves thereadypromise.promisethat waits forfinishTransition(triggered bypage:finish).ready.readyresolves inside thestartViewTransitioncallback.document.startViewTransitionthrows an error synchronously (e.g., if a transition is already active or invalid) or if the browser delays the callback execution unexpectedly while the main thread is blocked waiting for the router,changeRoute()is never called.beforeResolvehangs indefinitely, freezing the application navigation.The Fix
We wrapped the transition logic in a
try/catchblock and improved state management:document.startViewTransitionfails, we immediately callchangeRoute()andfinishTransition()in thecatchblock to ensure the router is unblocked.resetTransitionStatetotransition.finished.finally(...). This ensures cleanup happens reliably whether the transition completes, is skipped, or fails, avoiding race conditions where state might be cleared prematurely.