Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@N2D4 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 59 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRefactors StackHandler to delegate routing/rendering to a new StackHandlerClient; updates init-stack generator to emit a no-arg Next.js Handler; adds an e2e test for listing users (including anonymous), changes dashboard tsconfig JSX runtime to "preserve", and adds a backend development env var. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App / Caller
participant Wrapper as StackHandler (wrapper)
participant Client as StackHandlerClient
participant RouteMap as Route Mapping & Aliases
participant Component as Resolved Component
rect rgb(240,245,255)
Note over App,Wrapper: New flow (post-change)
App->>Wrapper: render(BaseHandlerProps)
Wrapper->>Client: delegate props
Client->>Client: normalize path & search params
Client->>RouteMap: resolve route / check aliases
alt alias match
RouteMap-->>Client: canonical route
Client->>App: redirect (replace)
else matched route
Client->>Component: renderComponent with props
Component-->>Client: UI
Client-->>Wrapper: UI
Wrapper-->>App: UI
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
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 |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the StackHandler component to simplify its API by removing the need for passing app, routeProps, params, and searchParams props. The main implementation has been moved to a new client component (StackHandlerClient) that uses hooks to retrieve necessary data, while the original file becomes a thin wrapper that maintains backward compatibility by accepting deprecated parameters.
- Moved StackHandler implementation to a new
stack-handler-client.tsxfile marked with"use client" - Original
stack-handler.tsxnow serves as a compatibility wrapper that forwards props toStackHandlerClient - Updated code generation in
init-stackto use simplified API without app/routeProps parameters
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/template/src/components-page/stack-handler.tsx | Replaced with minimal wrapper that accepts deprecated props and delegates to StackHandlerClient |
| packages/template/src/components-page/stack-handler-client.tsx | New client component containing the refactored StackHandler logic using hooks |
| packages/init-stack/src/index.ts | Updated handler file generation to use simplified StackHandler API |
| apps/e2e/tests/js/list-users.test.ts | Added tests for listing users with/without anonymous users |
| apps/dashboard/tsconfig.json | Changed JSX compilation from react-jsx to preserve |
| apps/backend/.env.development | Added STACK_SEED_INTERNAL_PROJECT_USER_GITHUB_ID configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
Refactored StackHandler to be a client component by extracting logic into StackHandlerClient and using React hooks instead of server-side props.
- Extracted all routing logic from
stack-handler.tsxinto newstack-handler-client.tsxwith"use client"directive StackHandlernow acts as a thin wrapper that accepts deprecated props for backward compatibility but delegates toStackHandlerClient- Uses
useStackApp()hook to access app context instead of requiringappprop - Uses Next.js client hooks (
usePathname,useSearchParams) instead of server-sideparamsandsearchParamsprops - Updated handler code generation in
init-stackto generate cleaner code without server-side props - Changed dashboard tsconfig JSX mode from
react-jsxtopreservefor Next.js compatibility - Added E2E tests for anonymous user listing functionality
Confidence Score: 5/5
- This PR is safe to merge with minimal risk
- Clean architectural refactoring that improves code organization by separating client and server concerns. Maintains full backward compatibility through deprecated props. E2E tests added for new functionality. All changes are well-structured and follow Next.js best practices for client components.
- No files require special attention
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| packages/template/src/components-page/stack-handler.tsx | 5/5 | Converted to thin wrapper that delegates to StackHandlerClient, maintains backward compatibility with deprecated props |
| packages/template/src/components-page/stack-handler-client.tsx | 5/5 | New client component extracted from stack-handler.tsx, uses useStackApp hook and Next.js client hooks (usePathname, useSearchParams) |
| packages/init-stack/src/index.ts | 5/5 | Updated generated handler code to remove app and routeProps parameters, now generates simpler <StackHandler fullPage /> component |
Sequence Diagram
sequenceDiagram
participant User
participant NextJS as Next.js Route
participant SH as StackHandler
participant SHC as StackHandlerClient
participant Hooks as useStackApp/usePathname/useSearchParams
participant Provider as StackProvider (Context)
participant Component as Auth Component
User->>NextJS: Navigate to /handler/sign-in
NextJS->>SH: Render StackHandler (Server)
Note over SH: Accepts deprecated props (app, routeProps)<br/>but ignores them
SH->>SHC: Delegate to StackHandlerClient (Client)
activate SHC
SHC->>Hooks: useStackApp()
Hooks->>Provider: Access StackContext
Provider-->>Hooks: Return stackApp instance
Hooks-->>SHC: stackApp
SHC->>Hooks: usePathname() & useSearchParams()
Hooks-->>SHC: pathname & searchParams
SHC->>SHC: Parse path relative to handler URL
SHC->>SHC: Route to appropriate component
SHC->>Component: Render SignIn component
deactivate SHC
Component-->>User: Display sign-in page
6 files reviewed, no comments
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
packages/template/src/components-page/stack-handler-client.tsx (1)
212-222: useMemo dependencies are correct for cross-platform support.The past review comment flagged
originin the dependency array as misleading since it's a constant in the Next.js path. However, examining the platform-conditional code (lines 200-210),originis dynamic in the React platform (line 209:window.location.origin). The dependency array correctly includes all values used in the memoized computation for both platforms.packages/init-stack/src/index.ts (1)
948-951: LGTM! Simplified handler aligns with client-driven architecture.The handler generation is now streamlined to just render
<StackHandler fullPage />without passingapporrouteProps, which correctly delegates all routing logic to the newStackHandlerClientcomponent introduced in this PR. This simplification improves maintainability and aligns with the refactoring objectives.The spacing issue from the past review comment has also been resolved.
packages/template/src/components-page/stack-handler.tsx (1)
5-5: Fix the typo in the comment.The comment states "returns " but the implementation actually returns
<StackHandlerClient />.Apply this diff:
-// This file exists as a component that can be both client and server, ignores its parameters, and returns <StackHandler /> +// This file exists as a component that can be both client and server, ignores its parameters, and returns <StackHandlerClient />
🧹 Nitpick comments (2)
packages/template/src/components-page/stack-handler-client.tsx (1)
60-65: Clarify misleading comment about path alias handling.The comment on line 61 states "also includes the uppercase and non-dashed versions," but the
pathAliasesobject only contains lowercase, dashed versions. The actual case-insensitive and dash-removal logic is implemented later in therenderComponentfunction (line 186), not in this object definition.Consider updating the comment to accurately reflect that this object defines explicit aliases, while the default case in
renderComponenthandles case variations.apps/backend/.env.development (1)
9-9: Clarify variable naming; value should be a GitHub user ID, not an email address.The variable
STACK_SEED_INTERNAL_PROJECT_USER_GITHUB_IDis used asproviderAccountIdwhen creating GitHub OAuth accounts (seed.ts, lines 329–365). GitHub OAuth expects a numeric user ID, but the valueadmin@example.comis an email address, creating a semantic mismatch between the variable name and its actual value.For the development seed, either:
- Rename to
STACK_SEED_INTERNAL_PROJECT_USER_GITHUB_ACCOUNT_IDand update the value to a numeric GitHub user ID (e.g.,"123456"), or- If email is intentional as a placeholder, rename to
STACK_SEED_INTERNAL_PROJECT_USER_OAUTH_IDENTIFIERto reflect that it accepts flexible identifiersAdd an inline comment explaining the expected format.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/backend/.env.development(1 hunks)apps/dashboard/tsconfig.json(1 hunks)apps/e2e/tests/js/list-users.test.ts(1 hunks)packages/init-stack/src/index.ts(1 hunks)packages/template/src/components-page/stack-handler-client.tsx(1 hunks)packages/template/src/components-page/stack-handler.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-12T17:55:06.710Z
Learnt from: madster456
Repo: stack-auth/stack-auth PR: 769
File: apps/backend/.env:67-71
Timestamp: 2025-08-12T17:55:06.710Z
Learning: In the stack-auth project, .env files use inline comments after the = sign (e.g., KEY=# description) as a consistent pattern throughout the configuration file. The maintainers prefer this format over separate comment lines above each key.
Applied to files:
apps/backend/.env.development
🧬 Code graph analysis (3)
packages/template/src/components-page/stack-handler.tsx (1)
packages/template/src/components-page/stack-handler-client.tsx (2)
BaseHandlerProps(67-72)StackHandlerClient(196-279)
packages/template/src/components-page/stack-handler-client.tsx (7)
packages/template/src/components-page/sign-out.tsx (1)
SignOut(12-20)packages/template/src/components-page/oauth-callback.tsx (1)
OAuthCallback(12-61)packages/template/src/components-page/magic-link-callback.tsx (1)
MagicLinkCallback(16-87)packages/template/src/components-page/mfa.tsx (1)
MFA(181-214)packages/stack-shared/src/utils/errors.tsx (1)
StackAssertionError(69-85)packages/stack-shared/src/utils/urls.tsx (2)
url(278-280)getRelativePart(256-258)packages/stack-shared/src/utils/objects.tsx (2)
FilterUndefined(365-367)filterUndefined(373-375)
apps/e2e/tests/js/list-users.test.ts (2)
apps/e2e/tests/helpers.ts (1)
it(12-12)apps/e2e/tests/js/js-helpers.ts (1)
createApp(41-86)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Vercel Agent Review
- GitHub Check: check_prisma_migrations (22.x)
- GitHub Check: build (22.x)
- GitHub Check: restart-dev-and-test-with-custom-base-port
- GitHub Check: setup-tests
- GitHub Check: build (22.x)
- GitHub Check: lint_and_build (latest)
- GitHub Check: docker
- GitHub Check: restart-dev-and-test
- GitHub Check: build (22.x)
- GitHub Check: all-good
- GitHub Check: Security Check
🔇 Additional comments (6)
apps/e2e/tests/js/list-users.test.ts (2)
4-30: LGTM! Comprehensive test coverage for anonymous user filtering.The test correctly verifies that:
- Anonymous users are created with distinct IDs
includeAnonymous: falseexcludes anonymous users from resultsincludeAnonymous: trueincludes all users in sign-up order
32-50: LGTM! Validates default behavior correctly.This test confirms that anonymous users are excluded by default when the
includeAnonymousparameter is not specified, which is the expected behavior for backward compatibility.packages/template/src/components-page/stack-handler-client.tsx (2)
224-242: LGTM! Redirect logic correctly preserves state.The
redirectIfNotHandlerhelper properly:
- Avoids unnecessary redirects when already on the handler path
- Preserves query parameters during redirection
- Uses
RedirectType.replaceto avoid polluting browser history
244-278: LGTM! Clean delegation and cross-platform handling.The main render logic correctly:
- Delegates routing to
renderComponentwith all necessary context- Provides platform-appropriate not-found handling (Next.js
notFound()vs React custom UI)- Handles redirect responses uniformly across platforms
apps/dashboard/tsconfig.json (1)
17-17: JSX transformation change is correct for Next.js; full build verification unavailable in this environment.The
"jsx": "preserve"setting is a mandatory change Next.js makes, as it implements its own optimized JSX transform. This configuration aligns with the official@tsconfig/nextbase configuration. With preserve mode, JSX is kept as part of the output to be further consumed by another transform step (e.g. Next.js), and the output will have a .jsx file extension.The configuration is valid and standard. However, a full build verification was not possible in the sandbox environment due to missing monorepo dependencies, so please confirm that builds succeed in your local development environment or CI/CD pipeline.
packages/template/src/components-page/stack-handler.tsx (1)
7-31: LGTM! Clean backward-compatibility wrapper.The implementation correctly:
- Imports and delegates to
StackHandlerClientfor actual rendering logic- Accepts deprecated props (
app,routeProps,params,searchParams) with proper JSDoc deprecation notices- Silently ignores deprecated props while forwarding valid props
- Maintains type safety by requiring
BaseHandlerProps(which includesfullPage)This design elegantly prevents the "Classes or null prototypes are not supported" error while maintaining backward compatibility and guiding users toward the new API.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/template/src/components-page/stack-handler-client.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/template/src/components-page/stack-handler-client.tsx (7)
packages/template/src/components-page/sign-out.tsx (1)
SignOut(12-20)packages/template/src/components-page/oauth-callback.tsx (1)
OAuthCallback(12-61)packages/template/src/components-page/magic-link-callback.tsx (1)
MagicLinkCallback(16-87)packages/template/src/components-page/mfa.tsx (1)
MFA(181-214)packages/stack-shared/src/utils/errors.tsx (1)
StackAssertionError(69-85)packages/stack-shared/src/utils/urls.tsx (2)
url(278-280)getRelativePart(256-258)packages/stack-shared/src/utils/objects.tsx (2)
FilterUndefined(365-367)filterUndefined(373-375)
🪛 GitHub Actions: Run setup tests
packages/template/src/components-page/stack-handler-client.tsx
[error] 244-244: TS2304: Cannot find name 'MessageCard'.
🪛 GitHub Actions: Runs E2E API Tests
packages/template/src/components-page/stack-handler-client.tsx
[error] 244-244: TS2304: Cannot find name 'MessageCard'.
🪛 GitHub Actions: Runs E2E API Tests with custom port prefix
packages/template/src/components-page/stack-handler-client.tsx
[error] 244-244: Cannot find name 'MessageCard'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Vercel Agent Review
- GitHub Check: check_prisma_migrations (22.x)
- GitHub Check: docker
- GitHub Check: restart-dev-and-test
- GitHub Check: lint_and_build (latest)
- GitHub Check: build (22.x)
- GitHub Check: restart-dev-and-test-with-custom-base-port
- GitHub Check: all-good
1 participant
Summary by CodeRabbit
New Features
Tests
Refactor