fix(astro): Ensure runInjectionScript runs on initial page loads with view transitions enabled#7801
Conversation
…ds with view transitions enabled
|
@lino-levan is attempting to deploy a commit to the Clerk Production Team on Vercel. A member of the Team first needs to authorize it. |
🦋 Changeset detectedLatest commit: eaa1bc4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
📝 WalkthroughWalkthroughThis change moves dynamic imports of 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/astro/src/integration/create-integration.ts (1)
123-134:⚠️ Potential issue | 🔴 CriticalAsync
astro:before-swaphandler will silently break the custom swap logic.According to Astro's documentation,
astro:before-swaplisteners run synchronously and Astro does not await async handlers. When you overrideevent.swap = () => { ... }, Astro calls it synchronously — it is not awaited. (Astro View Transitions docs)In the current code (lines 123–134), the handler is
asyncwith anawait import(...)at line 124. This causes the function to return aPromiseimmediately, deferring lines 126–133 (includinge.swap = ...) to a microtask. When Astro'sdispatchEvent()returns,e.swaphas not yet been set, so Astro calls the default swap function instead of your custom one. The Clerk component preservation silently fails.Move the import outside the listener to keep the handler synchronous:
Proposed fix
if (transitionEnabledOnThisPage()) { - // We must do the dynamic imports within the event listeners because otherwise we may race and miss initial astro:page-load - document.addEventListener('astro:before-swap', async (e) => { - const { swapFunctions } = await import('astro:transitions/client'); + const swapFunctionsPromise = import('astro:transitions/client'); + + document.addEventListener('astro:before-swap', (e) => { + const { swapFunctions } = getSwapFunctionsSync(swapFunctionsPromise); const clerkComponents = document.querySelector('#clerk-components'); // Keep the div element added by Clerk if (clerkComponents) { const clonedEl = clerkComponents.cloneNode(true); e.newDocument.body.appendChild(clonedEl); } e.swap = () => swapDocument(swapFunctions, e.newDocument); });Alternatively, wrap the
import()in a WeakMap cache to avoid re-importing:+ let swapFunctionsCached; + import('astro:transitions/client').then(mod => { + swapFunctionsCached = mod.swapFunctions; + }); + - document.addEventListener('astro:before-swap', async (e) => { - const { swapFunctions } = await import('astro:transitions/client'); + document.addEventListener('astro:before-swap', (e) => { + if (!swapFunctionsCached) return; // Fallback if import hasn't resolved const clerkComponents = document.querySelector('#clerk-components'); // Keep the div element added by Clerk if (clerkComponents) { const clonedEl = clerkComponents.cloneNode(true); e.newDocument.body.appendChild(clonedEl); } - e.swap = () => swapDocument(swapFunctions, e.newDocument); + e.swap = () => swapDocument(swapFunctionsCached, e.newDocument); });The module will be cached by the time the first
before-swapfires, sinceastro:transitions/clientis loaded by Astro's runtime.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.changeset/ready-cats-tease.md:
- Line 5: Change the changelog line "Fixed issue when using ClientRouter where
clerk components don't load until navigation is performed" to a polished
sentence with proper capitalization and punctuation, e.g. "Fixed an issue where
Clerk components didn’t load on initial page load when using ClientRouter."
Ensure "Clerk" is capitalized and use the improved wording that mentions
"initial page load" instead of "until navigation is performed."
.changeset/ready-cats-tease.md
Outdated
| '@clerk/astro': patch | ||
| --- | ||
|
|
||
| Fixed issue when using ClientRouter where clerk components don't load until navigation is performed |
There was a problem hiding this comment.
Polish changelog wording/capitalization.
Consider “Clerk” capitalization and a slightly clearer sentence, e.g., “Fixed an issue where Clerk components didn’t load on initial page load when using ClientRouter.”
🤖 Prompt for AI Agents
In @.changeset/ready-cats-tease.md at line 5, Change the changelog line "Fixed
issue when using ClientRouter where clerk components don't load until navigation
is performed" to a polished sentence with proper capitalization and punctuation,
e.g. "Fixed an issue where Clerk components didn’t load on initial page load
when using ClientRouter." Ensure "Clerk" is capitalized and use the improved
wording that mentions "initial page load" instead of "until navigation is
performed."
@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: |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/astro/src/integration/create-integration.ts (1)
129-140:⚠️ Potential issue | 🔴 CriticalThe
asyncbefore-swap handler will not work —e.swapande.newDocumentmodifications are assigned in a microtask after the swap is already dispatched.
dispatchEvent()is synchronous and returns immediately after calling all listeners. Anyasynclistener runs synchronously only up to its firstawait, then the rest continues in a microtask. This means:
- Line 130:
await import(...)suspends the listener- Astro's router continues and calls
e.swap()synchronously during the dispatch- Lines 136–139: Execute later in the microtask queue — after
e.swap()has already been invokedThe result: the Clerk component clone and custom swap function are never used.
Per Astro's guidance,
astro:before-swapwork must remain strictly synchronous and fast. Moving the import inside the listener doesn't solve the original race condition mentioned in the comment; it worsens timing precision.The
astro:page-loadhandler (lines 142–150) is safe to be async since that event is informational.Fix: Use module-scoped state to resolve the imports during
page-load(which is safe to be async) and read them synchronously inbefore-swap:Proposed fix
let swapFunctions; let navigateFn; document.addEventListener('astro:before-swap', (e) => { + if (!swapFunctions) return; const clerkComponents = document.querySelector('#clerk-components'); if (clerkComponents) { const clonedEl = clerkComponents.cloneNode(true); e.newDocument.body.appendChild(clonedEl); } e.swap = () => swapDocument(swapFunctions, e.newDocument); }); document.addEventListener('astro:page-load', async (e) => { - const { navigate } = await import('astro:transitions/client'); + const client = await import('astro:transitions/client'); + swapFunctions = client.swapFunctions; + navigateFn = client.navigate; await runInjectionScript({ ...${JSON.stringify(internalParams)}, - routerPush: navigate, - routerReplace: (url) => navigate(url, { history: 'replace' }), + routerPush: navigateFn, + routerReplace: (url) => navigateFn(url, { history: 'replace' }), }); })
alexcarpenter
changed the title
3 participants
Description
At work we have an Astro site using Clerk and we had this issue in dev where Clerk's components never appeared until a page navigation happened. It took a while to track down where the issue was, and eventually we saw that the dynamic import of the transitions was racing against the astro:page-load event. This PR is from the patch that is currently in our production codebase.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit