-
Notifications
You must be signed in to change notification settings - Fork 3
feat: add clerk authorized redirect origins #84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds Clerk authentication configuration by wrapping the Next.js root component with a ClerkProvider and whitelisting specific redirect origins, and introduces the Clerk package dependency.
- Wraps
<Component>inClerkProviderand setsallowedRedirectOrigins - Adds
@clerk/nextjsto project dependencies
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/pages/_app.tsx | Wraps the app in ClerkProvider with allowed redirect origins |
| package.json | Adds new dependency @clerk/nextjs@^6.19.5 |
Comments suppressed due to low confidence (2)
src/pages/_app.tsx:9
- [nitpick] You can combine multiple icons in a single call (e.g.
library.add(faBars, faCircle, faCircleHollow)) to reduce repetitive invocations.
library.add(faBars);
src/pages/_app.tsx:14
- Introducing
ClerkProviderwith custom redirect origins adds new auth flow behavior; consider adding integration or CI tests to verify correct redirect handling.
<ClerkProvider
| allowedRedirectOrigins={[ | ||
| 'https://www.cmucourses.com', | ||
| 'https://cmucourses.com', | ||
| ]} |
Copilot
AI
May 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoding redirect origins can make updates error-prone; consider extracting these URLs into environment variables or a configuration constant.
| allowedRedirectOrigins={[ | |
| 'https://www.cmucourses.com', | |
| 'https://cmucourses.com', | |
| ]} | |
| allowedRedirectOrigins={ | |
| process.env.NEXT_PUBLIC_ALLOWED_REDIRECT_ORIGINS | |
| ? process.env.NEXT_PUBLIC_ALLOWED_REDIRECT_ORIGINS.split(',') | |
| : [] | |
| } |
| function MyApp({ Component, pageProps }: AppProps) { | ||
| library.add(faBars); | ||
| library.add(faCircle); | ||
| library.add(faCircleHollow); | ||
| return <Component suppressHydrationWarning {...pageProps} />; | ||
|
|
||
| return ( | ||
| <ClerkProvider | ||
| allowedRedirectOrigins={[ | ||
| 'https://www.cmucourses.com', | ||
| 'https://cmucourses.com', | ||
| ]} | ||
| > |
Copilot
AI
May 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The array for allowedRedirectOrigins is recreated on every render, which may trigger unnecessary re-renders; consider defining it outside the component scope.
No description provided.