feat(astro): Support Astro v7#8974
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: a72ad23 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 |
📝 WalkthroughWalkthroughAdds Astro v7 to ChangesAstro v7 Compatibility
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@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.
dont need tailwind in this...
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
There was a problem hiding this comment.
the change basically just removed the early return; in favor of wrapping the menu-item registration logic in the existing else branch
There was a problem hiding this comment.
Astro v6 compatibility smoke test
There was a problem hiding this comment.
🧹 Nitpick comments (1)
integration/tests/astro/middleware.test.ts (1)
183-186: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid a permanent unconditional skip for this regression case.
test.skip(true, ...)disables this check in every run, so routing/auth regressions here can silently reappear. Prefer a trackedtest.fixme(with issue ID) or a version-gated skip so coverage returns automatically when the matcher behavior is fixed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integration/tests/astro/middleware.test.ts` around lines 183 - 186, The test.skip(true, ...) call unconditionally disables this regression test in every run, which allows routing and auth regressions to silently reappear. Replace this with test.fixme(...) that includes a tracked issue ID reference, or implement version-gated conditional skip logic based on the Astro version so the test coverage automatically resumes when the routing normalization issue is resolved. The key is to make the skip temporary and tracked rather than permanent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@integration/tests/astro/middleware.test.ts`:
- Around line 183-186: The test.skip(true, ...) call unconditionally disables
this regression test in every run, which allows routing and auth regressions to
silently reappear. Replace this with test.fixme(...) that includes a tracked
issue ID reference, or implement version-gated conditional skip logic based on
the Astro version so the test coverage automatically resumes when the routing
normalization issue is resolved. The key is to make the skip temporary and
tracked rather than permanent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 1b4b2e9c-0535-4606-8b56-033870a8b624
📒 Files selected for processing (4)
integration/tests/astro/compatibility.test.tsintegration/tests/astro/middleware.test.tspackages/astro/package.jsonpackages/astro/src/astro-components/interactive/UserButton/MenuItemRenderer.astro
🚧 Files skipped from review as they are similar to previous changes (3)
- integration/tests/astro/compatibility.test.ts
- packages/astro/package.json
- packages/astro/src/astro-components/interactive/UserButton/MenuItemRenderer.astro
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@integration/testUtils/machineAuthHelpers.ts`:
- Around line 275-281: In the POST request handling blocks, the status assertion
must be moved before the JSON parsing. Currently, the code calls
postWithSessionRes.json() before checking postWithSessionRes.status(), which
means if the response is an error, the json() call may fail before the status
assertion runs, obscuring the actual failure. Reorder the statements so that
expect(postWithSessionRes.status()).toBe(200) is called immediately after the
post request completes, before calling await postWithSessionRes.json(). Apply
this same fix to both the postWithSessionRes block and the similar block
referenced in the "Also applies to" comment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: af4cda56-7137-4065-a8e4-43d80f4d8263
📒 Files selected for processing (1)
integration/testUtils/machineAuthHelpers.ts
| const postWithSessionRes = await u.page.request.post(url, { | ||
| headers: { Origin: origin }, | ||
| }); | ||
| const sessionData = await postWithSessionRes.json(); | ||
| expect(postWithSessionRes.status()).toBe(200); | ||
| expect(sessionData.userId).toBe(fakeBapiUser.id); | ||
| expect(sessionData.tokenType).toBe(TokenType.SessionToken); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Assert response status before parsing JSON bodies in POST branches.
json() can throw on non-JSON error responses before the status assertion runs, which obscures the real failure. Assert status first, then parse.
Suggested patch
const postWithSessionRes = await u.page.request.post(url, {
headers: { Origin: origin },
});
- const sessionData = await postWithSessionRes.json();
expect(postWithSessionRes.status()).toBe(200);
+ const sessionData = await postWithSessionRes.json();
expect(sessionData.userId).toBe(fakeBapiUser.id);
expect(sessionData.tokenType).toBe(TokenType.SessionToken);
const postWithApiKeyRes = await u.page.request.post(url, {
headers: { Authorization: `Bearer ${fakeAPIKey.secret}`, Origin: origin },
});
- const apiKeyData = await postWithApiKeyRes.json();
expect(postWithApiKeyRes.status()).toBe(200);
+ const apiKeyData = await postWithApiKeyRes.json();
expect(apiKeyData.userId).toBe(fakeBapiUser.id);
expect(apiKeyData.tokenType).toBe(TokenType.ApiKey);Also applies to: 283-285
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@integration/testUtils/machineAuthHelpers.ts` around lines 275 - 281, In the
POST request handling blocks, the status assertion must be moved before the JSON
parsing. Currently, the code calls postWithSessionRes.json() before checking
postWithSessionRes.status(), which means if the response is an error, the json()
call may fail before the status assertion runs, obscuring the actual failure.
Reorder the statements so that expect(postWithSessionRes.status()).toBe(200) is
called immediately after the post request completes, before calling await
postWithSessionRes.json(). Apply this same fix to both the postWithSessionRes
block and the similar block referenced in the "Also applies to" comment.
| // match %2561dmin to the admin/ directory, returning 404 | ||
| test.skip( | ||
| true, | ||
| 'Astro 7 production now routes this double-encoded path to the admin endpoint; createPathMatcher needs follow-up to align with Astro routing normalization.', |
There was a problem hiding this comment.
dont think it's worth it to update since we're removing it anyway
Description
Adds Astro 7 support for
@clerk/astro.Astro 7 now uses the Rust-based
.astrocompiler as the only compiler. The new compiler is stricter and surfaced an invalid top-levelreturn;(ref) inside the inline script emitted byUserButtoncustom menu item rendering.This PR updates the script flow to avoid that error while preserving the existing behavior.
Fixes #8963
Resolves USER-5610
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
UserButtonmenu items in Astro 7.