ENG-373 draft: Add routes and components for auth #238
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThis change introduces a comprehensive authentication system using Supabase across both the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UIComponent
participant SupabaseClient
participant SupabaseServer
User->>UIComponent: Open login/signup/forgot password page
UIComponent->>SupabaseClient: submit credentials or email
SupabaseClient->>SupabaseServer: Authenticate or send reset email
SupabaseServer-->>SupabaseClient: Response (success/error)
SupabaseClient-->>UIComponent: Update UI state
UIComponent-->>User: Show result (logged in, error, instructions)
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
maparent
force-pushed
the
ENG-373-auth-in-roam-and-web
branch
from
4c2c494 to
63d509f
Compare
maparent
force-pushed
the
ENG-373-auth-in-roam-and-web
branch
from
63d509f to
9672554
Compare
maparent
force-pushed
the
ENG-373-auth-in-roam-and-web
branch
from
9672554 to
4d1ce04
Compare
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Nitpick comments (26)
apps/website/app/components/auth/LogoutButton.tsx (1)
10-14: Consider adding error handling for robustness.The logout function works correctly but could benefit from error handling in case the sign-out operation fails.
Consider this enhancement:
const logout = async () => { - const supabase = createClient(); - await supabase.auth.signOut(); - router.push("/auth/login"); + try { + const supabase = createClient(); + await supabase.auth.signOut(); + router.push("/auth/login"); + } catch (error) { + console.error("Logout failed:", error); + // Optionally show user feedback or still redirect + router.push("/auth/login"); + } };apps/website/app/components/auth/UpdatePasswordForm.tsx (2)
57-57: Use the Label component for better accessibility.Consider using the imported Label component instead of a basic HTML label for consistency with the design system:
- <label htmlFor="password">New password</label> + <Label htmlFor="password">New password</Label>You'll need to import it:
+import { Label } from "@/components/ui/label";
35-36: Address the TODO comment for user flow completion.The TODO comment indicates incomplete routing logic. Consider implementing proper post-update navigation or user feedback.
Would you like me to help design the user flow after successful password update?
packages/ui/src/components/ui/auth/UpdatePasswordForm.tsx (1)
21-23: Consider adding password validation.The component lacks password strength validation and confirmation field, which are important for security and user experience.
Consider adding:
- Password strength validation
- Password confirmation field
- Minimum length requirements
apps/roam/src/components/auth/ForgotPasswordForm.tsx (2)
1-4: Consider UI library consistency.The component mixes utilities from
@repo/uiwith components from@blueprintjs/core, which could lead to styling inconsistencies.Consider using components from a single UI library for consistency:
- Either use Blueprint.js components throughout
- Or use the custom UI components from
@repo/ui
39-66: Avoid passing className to multiple nested elements.The
classNameprop is being passed to multiple nested elements, which could cause unintended styling conflicts.Consider creating specific className props or using more targeted styling:
- className={cn( - "text-2xl font-semibold leading-none tracking-tight", - className, - )} + className="text-2xl font-semibold leading-none tracking-tight"apps/roam/src/components/auth/UpdatePasswordForm.tsx (2)
23-25: Complete the implementation by adding redirect logic.The TODO comment indicates missing redirect functionality after successful password update, which leaves users without feedback on success.
The component needs to handle successful password updates appropriately. Would you like me to suggest an implementation for the redirect logic, or should this be tracked as a separate issue?
- // Original: Update this route to redirect to an authenticated route. The user already has an active session. - // TODO: Replacement action - // location.href = "/protected"; + // Redirect to appropriate route after successful password update + window.location.href = "/dashboard"; // or appropriate route
1-4: Consider UI library consistency.Similar to the ForgotPasswordForm, this component mixes utilities from
@repo/uiwith components from@blueprintjs/core.For consistency across the roam app, consider using components from a single UI library.
apps/website/app/utils/supabase/middleware.ts (2)
47-47: Make login redirect path configurable.The hard-coded redirect path
/auth/loginreduces flexibility for different routing configurations.Consider making the login path configurable:
+const LOGIN_PATH = process.env.LOGIN_PATH || '/auth/login'; - url.pathname = '/auth/login' + url.pathname = LOGIN_PATH
40-49: Consider handling authenticated users on auth pages.The middleware doesn't redirect authenticated users away from auth pages (like login/signup), which could improve user experience.
Consider adding logic to redirect authenticated users away from auth pages:
if ( !user && !request.nextUrl.pathname.startsWith('/login') && !request.nextUrl.pathname.startsWith('/auth') ) { // no user, potentially respond by redirecting the user to the login page const url = request.nextUrl.clone() url.pathname = '/auth/login' return NextResponse.redirect(url) } + + // Redirect authenticated users away from auth pages + if ( + user && + (request.nextUrl.pathname.startsWith('/auth/login') || + request.nextUrl.pathname.startsWith('/auth/signup')) + ) { + const url = request.nextUrl.clone() + url.pathname = '/' + return NextResponse.redirect(url) + }apps/roam/src/components/auth/SignUpForm.tsx (1)
17-19: Consider moving Supabase client creation to component level for consistency.The Supabase client is created inside the event handler, but in
apps/roam/src/components/auth/LoginForm.tsx(line 11), it's created at the component level. This inconsistency could lead to confusion and makes the codebase harder to maintain.Apply this diff to make it consistent:
export function SignUpForm({ className, ...props }: React.ComponentPropsWithoutRef<"div">) { const [email, setEmail] = useState(""); const [password, setPassword] = useState(""); const [repeatPassword, setRepeatPassword] = useState(""); const [error, setError] = useState<string | null>(null); const [isLoading, setIsLoading] = useState(false); const [success, setSuccess] = useState(false); + const supabase = createClient(); const handleSignUp = async (e: React.FormEvent) => { - const supabase = createClient(); e.preventDefault();packages/ui/src/components/ui/input.tsx (1)
10-12: Consider extracting default styles to improve maintainability.The long className string contains many utility classes that could be extracted to a constant or CSS variable for better maintainability and readability.
Apply this diff to improve readability:
+const inputStyles = [ + "border-input bg-background ring-offset-background", + "file:text-foreground placeholder:text-muted-foreground focus-visible:ring-ring", + "flex h-10 w-full rounded-md border px-3 py-2 text-base", + "file:border-0 file:bg-transparent file:text-sm file:font-medium", + "focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-offset-2", + "disabled:cursor-not-allowed disabled:opacity-50 md:text-sm" +].join(" "); const Input = React.forwardRef<HTMLInputElement, React.ComponentProps<"input">>( ({ className, type, ...props }, ref) => { return ( <input type={type} className={cn( - "border-input bg-background ring-offset-background file:text-foreground placeholder:text-muted-foreground focus-visible:ring-ring flex h-10 w-full rounded-md border px-3 py-2 text-base file:border-0 file:bg-transparent file:text-sm file:font-medium focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50 md:text-sm", + inputStyles, className, )}apps/website/app/components/auth/LoginForm.tsx (3)
28-32: Inconsistent Supabase client creation pattern.This component creates the Supabase client inside the event handler, which is inconsistent with the pattern used in
apps/roam/src/components/auth/LoginForm.tsxwhere it's created at the component level.For consistency, consider moving the client creation to the component level:
export function LoginForm({ className, ...props }: React.ComponentPropsWithoutRef<"div">) { const [email, setEmail] = useState(""); const [password, setPassword] = useState(""); const [error, setError] = useState<string | null>(null); const [isLoading, setIsLoading] = useState(false); const router = useRouter(); + const supabase = createClient(); const handleLogin = async (e: React.FormEvent) => { e.preventDefault(); - const supabase = createClient(); setIsLoading(true);
40-41: Address the TODO comment for proper user flow.The redirect to "/" after login may not provide the best user experience. Consider implementing a proper user dashboard or authenticated landing page.
Would you like me to suggest a better post-login user flow or help implement a user dashboard component?
62-71: Missing accessibility labels.The form uses plain HTML
<label>elements instead of a proper Label component, which may not provide consistent styling and accessibility features compared to other forms in the codebase.Consider using a Label component for consistency:
+ import { Label } from "@/components/ui/label"; <div className="grid gap-2"> - <label htmlFor="email">Email</label> + <Label htmlFor="email">Email</Label> <Inputapps/roam/src/components/auth/LoginForm.tsx (1)
74-76: Use proper routing for better user experience.The sign-up link uses a plain anchor tag with
href, which causes a full page reload. Consider using proper routing if available in the roam application.packages/ui/src/components/ui/auth/ForgotPasswordForm.tsx (2)
26-27: Inconsistent Supabase client creation pattern.Like other auth components, this creates the Supabase client inside the event handler. For consistency and potential performance benefits, consider creating it at the component level.
export function ForgotPasswordForm({ className, ...props }: React.ComponentPropsWithoutRef<"div">) { const [email, setEmail] = useState(""); const [error, setError] = useState<string | null>(null); const [success, setSuccess] = useState(false); const [isLoading, setIsLoading] = useState(false); + const supabase = createClient(); const handleForgotPassword = async (e: React.FormEvent) => { - const supabase = createClient(); e.preventDefault();
91-93: Use proper routing component for navigation links.The login link uses a plain anchor tag which may not integrate well with the application's routing system.
Consider using a proper Link component if available in the context where this component is used.
packages/ui/src/components/ui/auth/SignUpForm.tsx (1)
121-123: Consider using proper routing componentsUsing a plain anchor tag for navigation may not be optimal in a React application context, especially if this component is used in Next.js or other routing environments.
Consider making the link configurable or using a more generic approach:
- <a href="/login" className="underline underline-offset-4"> + <a href="/login" className="underline underline-offset-4">Or accept a
loginHrefprop to make it more flexible across different routing systems.apps/website/app/components/auth/SignupForm.tsx (1)
46-48: Address TODO and verify email redirect configurationThe TODO comment indicates incomplete implementation, and the email redirect URL should be properly configured in Supabase dashboard.
The current redirect goes to the home page (
/), but the TODO suggests it should go to a user admin screen. This needs to be resolved before production use.Do you want me to help design the proper post-signup user flow or create an issue to track this requirement?
packages/ui/src/components/ui/auth/LoginForm.tsx (1)
73-78: Consider making navigation links configurableHard-coded anchor tags with fixed URLs reduce the component's reusability across different routing systems or URL structures.
Consider accepting href props for these navigation links:
export function LoginForm({ className, + forgotPasswordHref = "/forgot-password", + signUpHref = "/sign-up", ...props }: React.ComponentPropsWithoutRef<"div"> & { + forgotPasswordHref?: string; + signUpHref?: string; }) {Also applies to: 94-97
packages/ui/src/components/ui/auth/Account.tsx (2)
86-86: Capitalize button text for consistency.The button text should be capitalized to match other buttons in the UI.
- login + Login
65-65: Use self-closing tags for components without children.When components don't have children, use self-closing tags for cleaner code.
- <LoginForm></LoginForm> + <LoginForm />- <SignUpForm></SignUpForm> + <SignUpForm />- <ForgotPasswordForm></ForgotPasswordForm> + <ForgotPasswordForm />- <UpdatePasswordForm></UpdatePasswordForm> + <UpdatePasswordForm />Also applies to: 79-79, 93-93, 99-99
apps/roam/src/components/auth/Account.tsx (3)
62-65: Add missing semicolon.Add a semicolon at the end of the promise chain for consistency.
supabase.auth.signOut().then(() => { setAction(AuthAction.login); - }) + });
115-115: Capitalize button text for consistency.The button text should be capitalized to match other buttons in the UI.
- login + Login
42-131: Add default case to switch statement.Consider adding a default case to handle unexpected states, improving robustness.
case AuthAction.updatePassword: return ( <div> <UpdatePasswordForm /> </div> ); + default: + return ( + <div> + <p>Unexpected state. Please refresh the page.</p> + </div> + ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (39)
.github/workflows/roam-pr.yaml(1 hunks)apps/roam/package.json(1 hunks)apps/roam/scripts/compile.ts(1 hunks)apps/roam/src/components/auth/Account.tsx(1 hunks)apps/roam/src/components/auth/ForgotPasswordForm.tsx(1 hunks)apps/roam/src/components/auth/LoginForm.tsx(1 hunks)apps/roam/src/components/auth/SignUpForm.tsx(1 hunks)apps/roam/src/components/auth/UpdatePasswordForm.tsx(1 hunks)apps/roam/src/components/settings/Settings.tsx(3 hunks)apps/roam/tsconfig.json(1 hunks)apps/website/app/(home)/auth/confirm/route.ts(1 hunks)apps/website/app/(home)/auth/error/page.tsx(1 hunks)apps/website/app/(home)/auth/forgot-password/page.tsx(1 hunks)apps/website/app/(home)/auth/login/page.tsx(1 hunks)apps/website/app/(home)/auth/sign-up-success/page.tsx(1 hunks)apps/website/app/(home)/auth/sign-up/page.tsx(1 hunks)apps/website/app/(home)/auth/update-password/page.tsx(1 hunks)apps/website/app/components/DocsLayout.tsx(0 hunks)apps/website/app/components/Navigation.tsx(1 hunks)apps/website/app/components/TeamPerson.tsx(0 hunks)apps/website/app/components/auth/ForgotPasswordForm.tsx(1 hunks)apps/website/app/components/auth/LoginForm.tsx(1 hunks)apps/website/app/components/auth/LogoutButton.tsx(1 hunks)apps/website/app/components/auth/SignupForm.tsx(1 hunks)apps/website/app/components/auth/UpdatePasswordForm.tsx(1 hunks)apps/website/app/utils/supabase/client.ts(1 hunks)apps/website/app/utils/supabase/middleware.ts(1 hunks)apps/website/tsconfig.json(1 hunks)packages/ui/package.json(1 hunks)packages/ui/src/components/ui/auth/Account.tsx(1 hunks)packages/ui/src/components/ui/auth/ForgotPasswordForm.tsx(1 hunks)packages/ui/src/components/ui/auth/LoginForm.tsx(1 hunks)packages/ui/src/components/ui/auth/SignUpForm.tsx(1 hunks)packages/ui/src/components/ui/auth/UpdatePasswordForm.tsx(1 hunks)packages/ui/src/components/ui/button.tsx(1 hunks)packages/ui/src/components/ui/input.tsx(1 hunks)packages/ui/src/components/ui/label.tsx(1 hunks)packages/ui/src/lib/supabase/client.ts(1 hunks)turbo.json(4 hunks)
💤 Files with no reviewable changes (2)
- apps/website/app/components/TeamPerson.tsx
- apps/website/app/components/DocsLayout.tsx
🧰 Additional context used
🧬 Code Graph Analysis (17)
apps/website/app/(home)/auth/forgot-password/page.tsx (4)
apps/website/app/(home)/auth/login/page.tsx (1)
Page(3-11)apps/website/app/(home)/auth/sign-up/page.tsx (1)
Page(3-11)apps/website/app/(home)/auth/update-password/page.tsx (1)
Page(3-11)apps/website/app/components/auth/ForgotPasswordForm.tsx (1)
ForgotPasswordForm(17-104)
apps/website/app/(home)/auth/update-password/page.tsx (4)
apps/website/app/(home)/auth/login/page.tsx (1)
Page(3-11)apps/website/app/(home)/auth/forgot-password/page.tsx (1)
Page(3-11)apps/website/app/(home)/auth/sign-up/page.tsx (1)
Page(3-11)apps/website/app/components/auth/UpdatePasswordForm.tsx (1)
UpdatePasswordForm(17-77)
apps/website/app/utils/supabase/client.ts (2)
packages/ui/src/lib/supabase/client.ts (1)
createClient(7-12)packages/database/types.gen.ts (1)
Database(9-814)
apps/website/app/(home)/auth/login/page.tsx (4)
apps/website/app/(home)/auth/forgot-password/page.tsx (1)
Page(3-11)apps/website/app/(home)/auth/sign-up/page.tsx (1)
Page(3-11)apps/website/app/(home)/auth/update-password/page.tsx (1)
Page(3-11)apps/website/app/components/auth/LoginForm.tsx (1)
LoginForm(18-109)
packages/ui/src/components/ui/label.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/website/app/components/auth/LogoutButton.tsx (1)
apps/website/app/utils/supabase/client.ts (1)
createClient(4-9)
apps/website/app/components/auth/UpdatePasswordForm.tsx (5)
apps/website/app/utils/supabase/client.ts (1)
createClient(4-9)packages/ui/src/lib/utils.ts (1)
cn(4-6)packages/ui/src/components/ui/card.tsx (5)
Card(78-78)CardHeader(79-79)CardTitle(81-81)CardDescription(82-82)CardContent(83-83)packages/ui/src/components/ui/input.tsx (1)
Input(22-22)packages/ui/src/components/ui/button.tsx (1)
Button(56-56)
packages/ui/src/lib/supabase/client.ts (2)
apps/website/app/utils/supabase/client.ts (1)
createClient(4-9)packages/database/types.gen.ts (1)
Database(9-814)
packages/ui/src/components/ui/input.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/website/app/(home)/auth/confirm/route.ts (1)
apps/website/app/utils/supabase/client.ts (1)
createClient(4-9)
apps/roam/src/components/auth/ForgotPasswordForm.tsx (5)
packages/ui/src/lib/supabase/client.ts (1)
createClient(7-12)packages/ui/src/lib/utils.ts (1)
cn(4-6)packages/ui/src/components/ui/card.tsx (1)
Card(78-78)packages/ui/src/components/ui/label.tsx (1)
Label(26-26)packages/ui/src/components/ui/button.tsx (1)
Button(56-56)
apps/website/app/components/auth/LoginForm.tsx (7)
apps/roam/src/components/auth/LoginForm.tsx (1)
LoginForm(6-83)packages/ui/src/components/ui/auth/LoginForm.tsx (1)
LoginForm(17-104)apps/website/app/utils/supabase/client.ts (1)
createClient(4-9)packages/ui/src/lib/utils.ts (1)
cn(4-6)packages/ui/src/components/ui/card.tsx (5)
Card(78-78)CardHeader(79-79)CardTitle(81-81)CardDescription(82-82)CardContent(83-83)packages/ui/src/components/ui/input.tsx (1)
Input(22-22)packages/ui/src/components/ui/button.tsx (1)
Button(56-56)
apps/website/app/components/auth/SignupForm.tsx (6)
packages/ui/src/components/ui/auth/SignUpForm.tsx (1)
SignUpForm(17-131)apps/website/app/utils/supabase/client.ts (1)
createClient(4-9)packages/ui/src/lib/utils.ts (1)
cn(4-6)packages/ui/src/components/ui/card.tsx (5)
Card(78-78)CardHeader(79-79)CardTitle(81-81)CardDescription(82-82)CardContent(83-83)packages/ui/src/components/ui/input.tsx (1)
Input(22-22)packages/ui/src/components/ui/button.tsx (1)
Button(56-56)
packages/ui/src/components/ui/button.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/website/app/components/auth/ForgotPasswordForm.tsx (5)
apps/website/app/utils/supabase/client.ts (1)
createClient(4-9)packages/ui/src/lib/utils.ts (1)
cn(4-6)packages/ui/src/components/ui/card.tsx (5)
Card(78-78)CardHeader(79-79)CardTitle(81-81)CardDescription(82-82)CardContent(83-83)packages/ui/src/components/ui/input.tsx (1)
Input(22-22)packages/ui/src/components/ui/button.tsx (1)
Button(56-56)
packages/ui/src/components/ui/auth/ForgotPasswordForm.tsx (6)
packages/ui/src/lib/supabase/client.ts (1)
createClient(7-12)packages/ui/src/lib/utils.ts (1)
cn(4-6)packages/ui/src/components/ui/card.tsx (5)
Card(78-78)CardHeader(79-79)CardTitle(81-81)CardDescription(82-82)CardContent(83-83)packages/ui/src/components/ui/label.tsx (1)
Label(26-26)packages/ui/src/components/ui/input.tsx (1)
Input(22-22)packages/ui/src/components/ui/button.tsx (1)
Button(56-56)
packages/ui/src/components/ui/auth/LoginForm.tsx (6)
packages/ui/src/lib/supabase/client.ts (1)
createClient(7-12)packages/ui/src/lib/utils.ts (1)
cn(4-6)packages/ui/src/components/ui/card.tsx (5)
Card(78-78)CardHeader(79-79)CardTitle(81-81)CardDescription(82-82)CardContent(83-83)packages/ui/src/components/ui/label.tsx (1)
Label(26-26)packages/ui/src/components/ui/input.tsx (1)
Input(22-22)packages/ui/src/components/ui/button.tsx (1)
Button(56-56)
🔇 Additional comments (37)
.github/workflows/roam-pr.yaml (1)
15-16: LGTM! Proper environment variable configuration.The Supabase environment variables are correctly sourced from GitHub secrets, following security best practices for CI/CD workflows.
apps/roam/package.json (1)
31-33: Verify version specification consistency.There's an inconsistency in version specifications:
@supabase/auth-ui-reactis pinned to an exact version0.4.7while the other Supabase packages use flexible versioning with the^prefix. Ensure this exact version pinning is intentional.- "@supabase/auth-ui-react":"0.4.7", + "@supabase/auth-ui-react": "^0.4.7",If the exact version is required for compatibility reasons, consider adding a comment explaining why.
apps/website/app/components/Navigation.tsx (1)
1-2: LGTM! Correct Next.js client component pattern.The
"use client"directive is necessary since this component uses theusePathnamehook, which requires client-side execution in Next.js App Router.apps/roam/tsconfig.json (1)
15-18: LGTM! Well-structured path aliases.The path aliases follow best practices:
~/*provides clean internal imports fromsrc/@repo/*enables consistent monorepo package imports- Configuration syntax is correct
apps/website/tsconfig.json (1)
5-6: LGTM! Appropriate path aliases for Next.js app.The path aliases are well-configured:
~/*aligns with Next.js App Router structure@/*enables importing shared UI components from the monorepoapps/website/app/(home)/auth/forgot-password/page.tsx (1)
1-11: LGTM! Consistent auth page structure.The page component follows the established pattern from other auth pages with proper responsive layout and component composition.
apps/website/app/(home)/auth/sign-up/page.tsx (2)
3-11: LGTM! Consistent layout structure.The page component correctly follows the established auth page pattern with proper responsive layout.
1-1: Verify component naming consistency.The import references
SignupFormbut other components use PascalCase naming likeSignUpForm. Ensure the component file name and export are consistent with the naming convention used elsewhere.#!/bin/bash # Description: Check the actual component name and file structure # Expected: Consistent naming between import and actual component # Find the SignupForm component file and check its export fd -t f -e tsx -e ts "signup" apps/website/app/components/auth/ rg -A 3 "export.*SignupForm|export.*SignUpForm" apps/website/app/components/auth/apps/website/app/(home)/auth/update-password/page.tsx (1)
1-11: LGTM! Maintains consistent auth page pattern.The component structure and layout are consistent with other authentication pages, promoting a unified user experience.
apps/roam/scripts/compile.ts (1)
132-135: Verify environment variables are available at build time.The implementation correctly uses esbuild's
defineto inject Supabase configuration as compile-time constants. However, the non-null assertions (!) will cause build failures if these environment variables are not set.#!/bin/bash # Description: Check if Supabase environment variables are configured in CI/workflows # Expected: SUPABASE_URL and SUPABASE_ANON_KEY should be available in build environments # Check workflow files for Supabase environment configuration rg -A 5 -B 5 "SUPABASE_URL|SUPABASE_ANON_KEY" .github/workflows/apps/roam/src/components/settings/Settings.tsx (3)
28-28: LGTM! Proper import for authentication component.The Account component import supports the new authentication functionality.
112-117: LGTM! Well-placed authentication tab.The Login tab is appropriately positioned under Personal Settings and follows the established tab structure pattern.
188-188: LGTM! Disabled debug logging.Commenting out the console.log is appropriate for cleaner production output.
apps/website/app/(home)/auth/login/page.tsx (1)
1-11: Clean and consistent implementation.The login page follows the established pattern used across other auth pages with proper responsive design and component separation. The layout structure is consistent and appropriate.
packages/ui/src/lib/supabase/client.ts (2)
7-12: Properly typed Supabase client creation.The client creation function is well-implemented with correct TypeScript generics for the database schema and public schema typing.
4-5: Verify the use of declared constants vs environment variables.This implementation uses
declare constfor the Supabase credentials, while the website app's client (apps/website/app/utils/supabase/client.ts) usesprocess.envvariables. This difference suggests build-time injection for the UI package.Please confirm if this approach is intentional for the UI package context. If so, ensure the build process properly injects these constants.
#!/bin/bash # Description: Check if there are build scripts that inject these constants for the UI package # Search for build scripts or webpack configs that might inject these constants fd -t f -e js -e ts -e json | grep -E "(webpack|rollup|vite|build)" | head -10 | xargs grep -l "SUPABASE_URL\|SUPABASE_ANON_KEY" 2>/dev/null || echo "No build scripts found with Supabase constants" # Check if there are any other patterns for these constants rg "SUPABASE_URL|SUPABASE_ANON_KEY" --type-add 'config:*.{json,js,ts,yaml,yml}' -t config -A 2 -B 2apps/website/app/(home)/auth/error/page.tsx (1)
3-26: Proper async handling and consistent error page implementation.The component correctly handles the async
searchParamsand provides appropriate error messaging. The layout is consistent with other auth pages and follows Next.js app router patterns properly.apps/website/app/components/auth/LogoutButton.tsx (1)
1-8: Clean component structure and implementation.The component follows React best practices with proper client-side directive, clean imports, and focused responsibility for logout functionality.
Also applies to: 16-17
apps/website/app/(home)/auth/sign-up-success/page.tsx (1)
9-30: LGTM: Well-structured success page component.The component follows React best practices with proper use of UI components and responsive design. The layout is clean and provides clear user feedback after sign-up.
packages/ui/src/components/ui/label.tsx (1)
13-26: LGTM: Well-implemented Label component.The component follows React best practices with proper ref forwarding, TypeScript typing, and accessibility support through Radix UI primitives. The implementation is consistent with the design system patterns.
packages/ui/package.json (1)
38-49: Verify dependency versions for security and compatibility.The new dependencies support the authentication features, but please verify the versions are secure and compatible:
Are there any known security vulnerabilities in @supabase/supabase-js version 2.50.0 or @supabase/auth-ui-react version 0.4.7?What is the latest stable version of tailwind-merge and are there any breaking changes from version 2.5.5 to 3.3.1?apps/website/app/(home)/auth/confirm/route.ts (2)
14-14: Verify if createClient should be awaited.Based on the relevant code snippets,
createClient()appears to be a synchronous function, but here it's being awaited.#!/bin/bash # Description: Check the server-side createClient implementation # Expected: Determine if the function is async or sync rg -A 10 "export.*function createClient" apps/website/app/utils/supabase/
1-1: Verify the import path for createClient.The import path suggests a server-side client, but the relevant code snippets show the function in
client.ts. Please ensure you're using the correct server-side Supabase client.#!/bin/bash # Description: Verify the correct import path for server-side Supabase client # Expected: Find the server-side createClient function fd -t f -n "server.ts" | head -10 fd -t f -n "client.ts" | head -10 # Check if server.ts exists in the utils/supabase directory cat apps/website/app/utils/supabase/server.ts 2>/dev/null || echo "server.ts not found"apps/website/app/utils/supabase/middleware.ts (1)
30-38: Excellent implementation of session management.The comments and implementation correctly emphasize the critical importance of proper session handling and the
getUser()call. This follows Supabase best practices.apps/roam/src/components/auth/SignUpForm.tsx (2)
22-25: Good password validation implementation.The password matching validation is properly implemented with clear error messaging before making the API call.
35-39: Error handling follows good practices.The error handling properly catches different error types and provides user-friendly feedback while maintaining type safety.
packages/ui/src/components/ui/input.tsx (1)
5-21: Well-implemented reusable input component.The component properly uses forwardRef, spreads props appropriately, and merges classes using the utility function. The implementation follows React best practices.
apps/roam/src/components/auth/LoginForm.tsx (1)
11-11: Good practice: Supabase client at component level.Creating the Supabase client at the component level is more efficient than creating it on each form submission, which is the pattern used in the website version.
apps/website/app/components/auth/ForgotPasswordForm.tsx (2)
28-43: Good error handling and state managementThe async form handler properly manages loading states, clears previous errors, and handles both success and error cases appropriately.
46-103: Well-structured conditional UI with proper accessibilityThe component correctly handles the success/form states with appropriate user feedback. Form elements have proper labels and required attributes for accessibility.
packages/ui/src/components/ui/auth/SignUpForm.tsx (2)
33-36: Good password confirmation validationThe password matching validation is implemented correctly and provides clear user feedback.
28-37: Optimize Supabase client creation and improve validation flowThe Supabase client is created before
preventDefault()and password validation, which is inefficient. Also, the loading state should be set before validation to prevent double submissions.const handleSignUp = async (e: React.FormEvent) => { - const supabase = createClient(); e.preventDefault(); setError(null); if (password !== repeatPassword) { setError("Passwords do not match"); return; } setIsLoading(true); + const supabase = createClient();Likely an incorrect or invalid review comment.
apps/website/app/components/auth/SignupForm.tsx (1)
51-51: Good use of Next.js router for navigationUsing
router.push()for post-signup navigation is the correct approach for Next.js applications, providing better user experience than page reloads.packages/ui/src/components/ui/button.tsx (3)
7-34: Excellent variant system designThe button variants are comprehensive and well-structured. The base classes include proper accessibility features like focus-visible rings and disabled states. The variant system covers common use cases effectively.
42-53: Good polymorphic component implementationThe use of
forwardRefwith Radix UI'sSlotfor polymorphic rendering is implemented correctly, allowing the button to render as different elements when needed while maintaining proper ref forwarding.
36-40: Well-typed component interfaceThe interface properly extends HTML button attributes and includes variant props, with the
asChildprop for polymorphic behavior. The TypeScript usage is exemplary.packages/ui/src/components/ui/auth/LoginForm.tsx (1)
32-37: Good authentication flow implementationThe login logic properly handles the Supabase authentication flow with appropriate error handling.
turbo.json
Outdated
| "GEMINI_API_KEY", | ||
| "SUPABASE_ANON_KEY", | ||
| "SUPABASE_URL" |
There was a problem hiding this comment.
Remove duplicate environment variables.
The SUPABASE_ANON_KEY and SUPABASE_URL variables are duplicated in both the dev and deploy tasks. They already exist earlier in their respective arrays.
Apply this diff to remove the duplicates:
"dev": {
"passThroughEnv": [
"OBSIDIAN_PLUGIN_PATH",
"NODE_ENV",
"SUPABASE_URL",
"SUPABASE_ANON_KEY",
"SUPABASE_SERVICE_ROLE_KEY",
"POSTGRES_URL",
"OPENAI_API_KEY",
"ANTHROPIC_API_KEY",
- "GEMINI_API_KEY",
- "SUPABASE_ANON_KEY",
- "SUPABASE_URL"
+ "GEMINI_API_KEY"
], "deploy": {
"cache": false,
"inputs": ["$TURBO_DEFAULT$", ".env*"],
"passThroughEnv": [
"BLOB_READ_WRITE_TOKEN",
"GITHUB_REF_NAME",
"GITHUB_HEAD_REF",
"NODE_ENV",
"SUPABASE_PROJECT_ID",
"SUPABASE_DB_PASSWORD",
"SUPABASE_ACCESS_TOKEN",
"SUPABASE_URL",
"SUPABASE_ANON_KEY",
"SUPABASE_SERVICE_ROLE_KEY",
"POSTGRES_URL",
"OPENAI_API_KEY",
"ANTHROPIC_API_KEY",
- "GEMINI_API_KEY",
- "SUPABASE_ANON_KEY",
- "SUPABASE_URL"
+ "GEMINI_API_KEY"
]
},Also applies to: 61-63
🤖 Prompt for AI Agents
In turbo.json at lines 36 to 38 and also lines 61 to 63, remove the duplicate
environment variables SUPABASE_ANON_KEY and SUPABASE_URL from the dev and deploy
tasks respectively, as they are already declared earlier in their arrays. Ensure
each environment variable appears only once per task to avoid redundancy.
| process.env.NEXT_PUBLIC_SUPABASE_URL!, | ||
| process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY! |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add environment variable validation to prevent runtime errors.
Using non-null assertion operators (!) without validation can cause runtime errors if environment variables are missing. Consider adding validation:
export function createClient() {
+ const url = process.env.NEXT_PUBLIC_SUPABASE_URL;
+ const key = process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY;
+
+ if (!url || !key) {
+ throw new Error('Missing required Supabase environment variables');
+ }
+
return createSupabaseClient<Database, "public", Database["public"]>(
- process.env.NEXT_PUBLIC_SUPABASE_URL!,
- process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!
+ url,
+ key
)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| process.env.NEXT_PUBLIC_SUPABASE_URL!, | |
| process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY! | |
| export function createClient() { | |
| const url = process.env.NEXT_PUBLIC_SUPABASE_URL; | |
| const key = process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY; | |
| if (!url || !key) { | |
| throw new Error('Missing required Supabase environment variables'); | |
| } | |
| return createSupabaseClient<Database, "public", Database["public"]>( | |
| url, | |
| key | |
| ) | |
| } |
🤖 Prompt for AI Agents
In apps/website/app/utils/supabase/client.ts around lines 6 to 7, the
environment variables NEXT_PUBLIC_SUPABASE_URL and NEXT_PUBLIC_SUPABASE_ANON_KEY
are accessed with non-null assertions without validation, which can cause
runtime errors if they are undefined. Add validation checks to confirm these
environment variables are defined before using them, and throw a clear error or
handle the missing variables gracefully to prevent runtime failures.
| </CardDescription> | ||
| </CardHeader> | ||
| <CardContent> | ||
| <form onSubmit={handleForgotPassword}> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update form submission handler reference.
The form's onSubmit handler still references the old function name.
- <form onSubmit={handleForgotPassword}>
+ <form onSubmit={handleUpdatePassword}>🤖 Prompt for AI Agents
In apps/website/app/components/auth/UpdatePasswordForm.tsx at line 54, the
form's onSubmit handler incorrectly references the old function
handleForgotPassword. Update the onSubmit attribute to reference the correct
current submission handler function for the update password form to ensure
proper form submission behavior.
| const [isLoading, setIsLoading] = useState(false); | ||
| const router = useRouter(); | ||
|
|
||
| const handleForgotPassword = async (e: React.FormEvent) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix misleading function name.
The function is named handleForgotPassword but it's actually updating the user's password, not handling forgot password functionality.
- const handleForgotPassword = async (e: React.FormEvent) => {
+ const handleUpdatePassword = async (e: React.FormEvent) => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleForgotPassword = async (e: React.FormEvent) => { | |
| const handleUpdatePassword = async (e: React.FormEvent) => { |
🤖 Prompt for AI Agents
In apps/website/app/components/auth/UpdatePasswordForm.tsx at line 26, the
function named handleForgotPassword is misleading because it actually updates
the user's password rather than handling forgot password logic. Rename the
function to a more appropriate name such as handleUpdatePassword or
handleChangePassword to accurately reflect its purpose.
| const [error, setError] = useState<string | null>(null); | ||
| const [isLoading, setIsLoading] = useState(false); | ||
|
|
||
| const handleForgotPassword = async (e: React.FormEvent) => { |
There was a problem hiding this comment.
Fix misleading function name.
The function is named handleForgotPassword but actually handles password updates, which is confusing and misleading.
- const handleForgotPassword = async (e: React.FormEvent) => {
+ const handleUpdatePassword = async (e: React.FormEvent) => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleForgotPassword = async (e: React.FormEvent) => { | |
| const handleUpdatePassword = async (e: React.FormEvent) => { |
🤖 Prompt for AI Agents
In packages/ui/src/components/ui/auth/UpdatePasswordForm.tsx at line 25, rename
the function handleForgotPassword to a name that accurately reflects its
purpose, such as handleUpdatePassword, to avoid confusion and better represent
that it handles password updates rather than password reset requests.
| updatePassword, | ||
| } | ||
|
|
||
| export const Account = async () => { |
There was a problem hiding this comment.
React components cannot be async.
React functional components must return JSX synchronously, not a Promise. Remove the async keyword from the component declaration.
-export const Account = async () => {
+export const Account = () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const Account = async () => { | |
| -export const Account = async () => { | |
| +export const Account = () => { |
🤖 Prompt for AI Agents
In packages/ui/src/components/ui/auth/Account.tsx at line 24, the Account
component is declared as async, which is not allowed for React functional
components because they must return JSX synchronously. Remove the async keyword
from the component declaration and handle any asynchronous logic inside
useEffect or other hooks instead.
| import { LoginForm } from "@repo/ui/components/ui/auth/LoginForm"; | ||
| import { ForgotPasswordForm } from "@repo/ui/components/ui/auth/ForgotPasswordForm"; | ||
| import { UpdatePasswordForm } from "@repo/ui/components/ui/auth/UpdatePasswordForm"; | ||
| import { Sign } from "crypto"; |
There was a problem hiding this comment.
Remove unused import.
The Sign import from 'crypto' is not used in this component.
-import { Sign } from "crypto";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { Sign } from "crypto"; |
🤖 Prompt for AI Agents
In packages/ui/src/components/ui/auth/Account.tsx at line 11, remove the unused
import statement for 'Sign' from the 'crypto' module to clean up the code and
avoid unnecessary imports.
| const supabase = createClient(); | ||
| useEffect(() => { | ||
| supabase.auth.getSession().then(({ data: { session } }) => { | ||
| setSession(session); | ||
| }); | ||
|
|
||
| const { | ||
| data: { subscription }, | ||
| } = supabase.auth.onAuthStateChange((_event, session) => { | ||
| setSession(session); | ||
| }); | ||
|
|
||
| return () => subscription.unsubscribe(); | ||
| }, []); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize Supabase client creation and fix useEffect dependencies.
The Supabase client is being recreated on every render. Consider moving it outside the component or using useMemo. Also, supabase should be included in the useEffect dependency array.
- const supabase = createClient();
+ const supabase = useMemo(() => createClient(), []);
useEffect(() => {
supabase.auth.getSession().then(({ data: { session } }) => {
setSession(session);
});
const {
data: { subscription },
} = supabase.auth.onAuthStateChange((_event, session) => {
setSession(session);
});
return () => subscription.unsubscribe();
- }, []);
+ }, [supabase]);Don't forget to import useMemo from React:
-import { useState, useEffect } from "react";
+import { useState, useEffect, useMemo } from "react";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const supabase = createClient(); | |
| useEffect(() => { | |
| supabase.auth.getSession().then(({ data: { session } }) => { | |
| setSession(session); | |
| }); | |
| const { | |
| data: { subscription }, | |
| } = supabase.auth.onAuthStateChange((_event, session) => { | |
| setSession(session); | |
| }); | |
| return () => subscription.unsubscribe(); | |
| }, []); | |
| // at the top of the file | |
| -import { useState, useEffect } from "react"; | |
| +import { useState, useEffect, useMemo } from "react"; | |
| // inside your component, replacing the original lines 28–41 | |
| const supabase = useMemo(() => createClient(), []); | |
| useEffect(() => { | |
| supabase.auth.getSession().then(({ data: { session } }) => { | |
| setSession(session); | |
| }); | |
| const { | |
| data: { subscription }, | |
| } = supabase.auth.onAuthStateChange((_event, session) => { | |
| setSession(session); | |
| }); | |
| return () => subscription.unsubscribe(); | |
| }, [supabase]); |
🤖 Prompt for AI Agents
In packages/ui/src/components/ui/auth/Account.tsx around lines 28 to 41, the
Supabase client is created inside the component causing it to be recreated on
every render, and the useEffect hook is missing supabase in its dependency
array. To fix this, move the Supabase client creation outside the component or
wrap it in a useMemo hook to memoize it, and add supabase to the useEffect
dependency array. Also, ensure that useMemo is imported from React at the top of
the file.
| const user = await supabase.auth.getUser(); | ||
| if (!user) { | ||
| setAction(AuthAction.login); | ||
| } |
There was a problem hiding this comment.
Move authentication check inside useEffect.
The authentication check and state update should be inside the useEffect hook, not in the component body. Also, remove the await keyword and fix the logic.
- const user = await supabase.auth.getUser();
- if (!user) {
- setAction(AuthAction.login);
- }Move this logic into the existing useEffect:
useEffect(() => {
supabase.auth.getSession().then(({ data: { session } }) => {
setSession(session);
+ if (!session) {
+ setAction(AuthAction.login);
+ }
});🤖 Prompt for AI Agents
In packages/ui/src/components/ui/auth/Account.tsx around lines 43 to 46, the
authentication check using await supabase.auth.getUser() and the subsequent
state update are incorrectly placed directly in the component body. Move this
entire logic inside the existing useEffect hook to ensure it runs after the
component mounts. Remove the await keyword and instead handle the promise
properly within useEffect, updating the state accordingly based on the resolved
user value.
| const supabase = createClient(); | ||
| useEffect(() => { | ||
| supabase.auth.getUser().then(({ data: { user } }) => { | ||
| if (user) setAction(AuthAction.loggedIn); | ||
| else setAction(AuthAction.login); | ||
| }); | ||
|
|
||
| const { | ||
| data: { subscription }, | ||
| } = supabase.auth.onAuthStateChange((_event, session) => { | ||
| if (session) setAction(AuthAction.loggedIn); | ||
| else setAction(AuthAction.login); | ||
| }); | ||
|
|
||
| return () => subscription.unsubscribe(); | ||
| }, []); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize Supabase client creation and fix useEffect dependencies.
The Supabase client is being recreated on every render. Consider using useMemo to memoize it. Also, supabase should be included in the useEffect dependency array.
+import React, { useState, useEffect, useMemo } from "react";
-import React, { useState, useEffect } from "react";- const supabase = createClient();
+ const supabase = useMemo(() => createClient(), []);
useEffect(() => {
// ... existing code ...
- }, []);
+ }, [supabase]);🤖 Prompt for AI Agents
In apps/roam/src/components/auth/Account.tsx around lines 25 to 40, the Supabase
client is created inside the component, causing it to be recreated on every
render, and the useEffect hook does not include supabase in its dependency
array. Fix this by memoizing the Supabase client using useMemo to create it only
once, and add supabase to the useEffect dependency array to ensure proper effect
execution.
b56ffd3 to
a8d3b04
Compare
maparent
force-pushed
the
ENG-373-auth-in-roam-and-web
branch
7 times, most recently
from
87c8ba5 to
eddba8b
Compare
maparent
force-pushed
the
ENG-373-auth-in-roam-and-web
branch
2 times, most recently
from
d3fde7f to
ed52562
Compare
maparent
force-pushed
the
ENG-373-auth-in-roam-and-web
branch
4 times, most recently
from
56e180c to
82e30a2
Compare
maparent
force-pushed
the
ENG-373-auth-in-roam-and-web
branch
from
82e30a2 to
af66513
Compare
In ui, roam and website
maparent
force-pushed
the
ENG-373-auth-in-roam-and-web
branch
from
af66513 to
84fa060
Compare
In ui, roam and website.
Will probably not be merged until beta, and maybe then in sections.
Still pushed so people can look at it and see what to expect.
Summary by CodeRabbit