fix(clerk-js): Remove beforeunload event listener from SafeLock#7775
Conversation
The beforeunload listener was redundant and degraded UX by disabling the browser's back-forward cache (bfcache). Lock cleanup is handled automatically: - Web Locks API releases locks natively on page unload - browser-tabs-lock has a built-in timeout mechanism that expires orphaned locks after ~5 seconds Fixes clerk#7714 https://claude.ai/code/session_01S2A4oTbEm3huTsR26xkU1E
🦋 Changeset detectedLatest commit: 9c9b70f The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@claude is attempting to deploy a commit to the Clerk Production Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughRemoved the 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
nikosdouvlis
left a comment
There was a problem hiding this comment.
Makes sense, I verified the claims against the library source, thank you
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.changeset/remove-beforeunload-safelock.md:
- Line 5: Update the changelog entry to remove the unsupported claim about
triggering "Leave site?" prompts and instead state that the removed window
"beforeunload" listener in SafeLock caused bfcache disablement; reference the
SafeLock component and the removed beforeunload handler (which called
lock.releaseLock(key)) so the description clearly attributes the change to
bfcache impact and not to prompt behavior, and optionally mention issue `#7714`
for context.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Nikos Douvlis <nikosdouvlis@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Nikos Douvlis <nikosdouvlis@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
… (#7818) Co-authored-by: Ben Singer <benjaminsinger@ftmedia.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Nikos Douvlis <nikosdouvlis@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Nikos Douvlis <nikosdouvlis@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Removes the beforeunload event listener from the SafeLock function. This listener was attempting to release locks when the page unloads, but it is both redundant and harmful to user experience.
Why this listener is harmful
The beforeunload event listener disables the browser's back-forward cache (bfcache), which degrades navigation performance. As noted in the codebase itself (beforeUnloadTracker.ts:6-7):
This is the root cause of the UX issues reported in #7714.
Why this listener is redundant
The SafeLock function uses a dual-locking strategy:
Trade-off analysis
In the fallback case, other tabs may need to wait up to 5 seconds to acquire an orphaned lock. This is acceptable because:
Additional benefit
This also removes a TypeScript linting issue (@typescript-eslint/no-misused-promises) where an async function was used as an event handler without proper promise handling.
🐛 Bug fix (fixes #7714)
Summary by CodeRabbit