fix(e2e): fix cache-components template build failures#8071
fix(e2e): fix cache-components template build failures#8071jacekradko wants to merge 11 commits intomainfrom
Conversation
Use connection() to prevent prerendering of the use-cache error trigger route, and wrap SignIn in Suspense for the sign-in catchall page.
🦋 Changeset detectedLatest commit: bdece64 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@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/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: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request introduces changes to a Next.js cache components template and test infrastructure. It adds a Suspense boundary wrapper to the SignIn component on the sign-in page, refactors the dynamic route page to use Suspense for asynchronous content rendering, and modifies the API route to call 🚥 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. 📝 Coding Plan
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)
integration/templates/next-cache-components/src/app/api/use-cache-error-trigger/route.ts (1)
21-23:⚠️ Potential issue | 🟠 MajorHarden catch-path error handling to prevent secondary runtime failures.
The catch block uses
catch (e: any)with directe.messageaccess, which throws a TypeError if a non-Error value is thrown. This breaks the error handler's own recovery path.Suggested fix
- } catch (e: any) { + } catch (e: unknown) { // Return the error message so we can verify it in tests - return Response.json({ error: e.message }, { status: 500 }); + const message = e instanceof Error ? e.message : 'Unknown error'; + return Response.json({ error: message }, { status: 500 }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integration/templates/next-cache-components/src/app/api/use-cache-error-trigger/route.ts` around lines 21 - 23, The catch block in the route handler currently assumes the thrown value has a message property (catch (e: any) { return Response.json({ error: e.message }, ...); }) which can raise a TypeError for non-Error throws; update the handler to safely extract the error text (e.g. const errorText = e instanceof Error ? e.message : String(e ?? 'Unknown error')) and return that in Response.json so the error path cannot itself throw.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@integration/templates/next-cache-components/src/app/api/use-cache-error-trigger/route.ts`:
- Around line 21-23: The catch block in the route handler currently assumes the
thrown value has a message property (catch (e: any) { return Response.json({
error: e.message }, ...); }) which can raise a TypeError for non-Error throws;
update the handler to safely extract the error text (e.g. const errorText = e
instanceof Error ? e.message : String(e ?? 'Unknown error')) and return that in
Response.json so the error path cannot itself throw.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: d04a1d1d-078d-4ea3-b682-6c25892048d1
📒 Files selected for processing (2)
integration/templates/next-cache-components/src/app/api/use-cache-error-trigger/route.tsintegration/templates/next-cache-components/src/app/sign-in/[[...catchall]]/page.tsx
The cache-components test was in the CI matrix but had no corresponding turbo task in turbo.json, so turbo --affected always returned 0 tasks and the test was silently skipped on every PR.
1 participant
Summary
Fixes the
cache-componentsintegration test suite so it builds and passes on CI.Build fixes
use-cache-error-triggerroute: Removed entirely — it tested Next.js"use cache"error behavior, not Clerk functionalityuse-cache-errorpage: Removed for the same reasonsign-inpage: Wrapped<SignIn />in<Suspense>— required by Next.js 16 withcacheComponents: truesince the component accessesheaders()internallydynamic-route/[id]page: Wrapped in<Suspense>—paramsis dynamic data that requires a Suspense boundary with cache componentsTurbo config
test:integration:cache-componentstask toturbo.json(was defined on an unmerged branch, causing--affectedto silently return 0 tasks)Skipped tests
currentUser()cache correct pattern (signed in):clerkClient()also callsheaders()internally viabuildRequestLike(), so it fails inside"use cache". Needs a fix in@clerk/nextjsto fall through to env-based config — tracked separately.toBeSignedOut()times out in CI with cache components enabled. Were previously hidden as "did not run" due to earlier test failure.Follow-ups needed
clerkClient()to not re-throwClerkUseCacheErrorso it works inside"use cache"(the error message already recommends this pattern)<SignIn />requiring<Suspense>withcacheComponents— ideally handled internally by@clerk/nextjs