fix: Update code to infer Vercel URL for WorkOS redirect#3061
fix: Update code to infer Vercel URL for WorkOS redirect#3061
Conversation
mcstepp
requested review from
chronark,
ogzhanolguncu and
perkinsjr
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThis pull request refactors how the application derives its base URL by extracting the logic into a dedicated Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SSO_Route as SSO Callback Route
participant Utils
Client ->> SSO_Route: GET request
SSO_Route ->> Utils: getBaseUrl()
Utils -->> SSO_Route: base URL
SSO_Route ->> Client: Respond with base URL-based data
sequenceDiagram
participant Client
participant WorkOS as WorkOSAuthProvider
participant Utils
Client ->> WorkOS: Initiate signInViaOAuth()
WorkOS ->> Utils: getBaseUrl()
Utils -->> WorkOS: base URL
WorkOS ->> Client: Redirect with URL built using base URL
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
perkinsjr
left a comment
perkinsjr
left a comment
There was a problem hiding this comment.
Changes needed here I am pushing them up
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/dashboard/app/auth/sign-in/oauth-signin.tsx(0 hunks)apps/dashboard/app/auth/sign-up/oauth-signup.tsx(2 hunks)apps/dashboard/lib/auth/types.ts(0 hunks)apps/dashboard/lib/auth/workos.ts(6 hunks)
💤 Files with no reviewable changes (2)
- apps/dashboard/lib/auth/types.ts
- apps/dashboard/app/auth/sign-in/oauth-signin.tsx
🧰 Additional context used
🧬 Code Definitions (1)
apps/dashboard/lib/auth/workos.ts (1)
apps/dashboard/lib/auth/types.ts (1)
VerificationResult(78-81)
🪛 Biome (1.9.4)
apps/dashboard/app/auth/sign-up/oauth-signup.tsx
[error] 16-16: This variable is unused.
Unused variables usually are result of incomplete refactoring, typos and other source of bugs.
Unsafe fix: If this is intentional, prepend baseUrl with an underscore.
(lint/correctness/noUnusedVariables)
🪛 GitHub Actions: autofix.ci
apps/dashboard/app/auth/sign-up/oauth-signup.tsx
[warning] 16-16: lint/correctness/noUnusedVariables: This variable is unused. Unsafe fix: If this is intentional, prepend baseUrl with an underscore.
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: Test Go API Local / Test (Shard 5/8)
- GitHub Check: Test Go API Local / Test (Shard 2/8)
- GitHub Check: Test Go API Local / Test (Shard 6/8)
- GitHub Check: Test Go API Local / Test (Shard 7/8)
- GitHub Check: Test Packages / Test ./packages/nextjs
- GitHub Check: Test Packages / Test ./packages/cache
- GitHub Check: Test Go API Local / Test (Shard 4/8)
- GitHub Check: Test Packages / Test ./packages/hono
- GitHub Check: Test Packages / Test ./packages/api
- GitHub Check: Test Packages / Test ./internal/encryption
- GitHub Check: Build / Build
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
apps/dashboard/lib/auth/workos.ts (3)
7-7: Good implementation of dynamic base URL detectionThe code now dynamically infers the base URL using the centralized
getBaseUrl()function instead of relying on hardcoded values or environment variables, which makes the code more robust across different environments (local, Vercel, etc.).Also applies to: 784-785, 788-788
196-199: Improved code formatting for method signaturesYou've improved the formatting of several method signatures to make them more readable with consistent line breaks and indentation, which is a good practice.
Also applies to: 227-229, 462-465, 655-658
782-782:Details
❓ Verification inconclusive
Parameter removed from destructuring without changing interface
You've removed
redirectUrlfrom the destructuring of options, but it might still be part of theSignInViaOAuthOptionsinterface. If this parameter is no longer needed, consider updating the interface definition as well to avoid confusion.
🏁 Script executed:
#!/bin/bash # Check if redirectUrl is still part of SignInViaOAuthOptions interface rg -A 10 "type SignInViaOAuthOptions" --type=typescriptLength of output: 92
Below is an updated review comment. It now asks for verification using a corrected shell command with the proper file type filter for TypeScript.
Action: Verify if
redirectUrlstill appears in the SignInViaOAuthOptions interfaceIt looks like the earlier command didn’t work because of the incorrect file type flag. Please run the following command to double-check whether
redirectUrlis still included in the interface. If it is present and no longer needed, remember to update the interface accordingly.#!/bin/bash # Check if redirectUrl is still part of SignInViaOAuthOptions interface using the correct file type filter for TypeScript rg -A 10 "interface SignInViaOAuthOptions" --type ts
I don't trust Vercel
2 participants
What does this PR do?
Fixes linear ENG-1694
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
New Features
Refactor
Style