Next 15 support #226
Next 15 support #226
Conversation
DanielRivers
requested review from
DaveOrDead and
peterphanouvong
as code owners
WalkthroughThe pull request introduces significant changes to enhance the asynchronous handling of session management across multiple files. Key modifications include converting several synchronous function calls to asynchronous ones, particularly in session management functions. This ensures that session-related operations complete before proceeding with subsequent logic, affecting the control flow in various components, including API client creation and router handling. Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (8)
src/session/getRoles.js (1)
Line range hint
1-27: Consider enhancing error documentation.While the error handling is correct, it would be beneficial to document the specific types of errors that could be thrown and handled.
Add error types to the JSDoc:
/** * @callback getRoles + * @throws {KindeClientError} When there's an authentication error + * @throws {SessionError} When session cannot be retrieved * @returns {Promise<import('../../types').KindeRoles | null>} */src/session/getPermissions.js (1)
Line range hint
14-26: Consider monitoring performance impact.While the async changes are necessary for Next.js 15 compatibility, the additional await could impact response times. Consider adding performance monitoring to track any potential latency increases.
src/session/getPermission.js (1)
Line range hint
23-28: Consider enhancing error handling.The current error handling returns
nullfor all error types, which might mask different error conditions. Consider differentiating between common error scenarios (e.g., invalid permissions, session errors) to help with debugging.Example enhancement:
} catch (error) { if (config.isDebugMode) { - console.error(error); + console.error('Failed to get permission:', { + permissionName: name, + error: error instanceof Error ? error.message : String(error) + }); } return null; }src/session/getClaim.js (1)
21-21: LGTM! Consider monitoring response times.The conversion to async/await is appropriate for Next.js 15 compatibility and ensures proper session resolution. However, since this introduces an additional await, consider monitoring response times in production to ensure acceptable performance.
src/session/index.ts (1)
Line range hint
27-33: Consider enhancing error handling for async context.While the error handling remains functional, the async context might benefit from more specific error handling.
Consider this enhancement:
try { // @ts-ignore const response = await kindeClient.refreshTokens( await sessionManager(req, res) ); return response; } catch (error) { + // Handle specific async operation failures + if (error instanceof TypeError && error.message.includes('await')) { + console.error('Async operation failed:', error); + return null; + } if (config.isDebugMode) { console.error(error); } return null; }src/api-client.js (2)
Line range hint
34-45: Consider enhancing error handling for token fetch.The OAuth token fetch operation lacks proper error handling. If the fetch fails or returns an invalid response, the code will throw an undefined error when trying to access the access_token property.
Consider applying this improvement:
- const response = await fetch(`${config.issuerURL}/oauth2/token`, { - method: 'POST', - headers: { - 'content-type': 'application/x-www-form-urlencoded' - }, - body: new URLSearchParams({ - grant_type: 'client_credentials', - client_id: config.clientID || '', - client_secret: config.clientSecret || '', - audience: config.issuerURL + '/api' - }) - }); - apiToken = (await response.json()).access_token; + try { + const response = await fetch(`${config.issuerURL}/oauth2/token`, { + method: 'POST', + headers: { + 'content-type': 'application/x-www-form-urlencoded' + }, + body: new URLSearchParams({ + grant_type: 'client_credentials', + client_id: config.clientID || '', + client_secret: config.clientSecret || '', + audience: config.issuerURL + '/api' + }) + }); + if (!response.ok) { + throw new Error(`Failed to fetch token: ${response.status} ${response.statusText}`); + } + const data = await response.json(); + if (!data.access_token) { + throw new Error('No access token received in response'); + } + apiToken = data.access_token; + } catch (error) { + throw new Error(`Failed to obtain API token: ${error.message}`); + }
Line range hint
47-51: Consider adding request timeout for security.The Configuration initialization should include a reasonable timeout to prevent hanging requests.
Consider applying this improvement:
const cfg = new Configuration({ basePath: config.issuerURL, accessToken: apiToken, - headers: {Accept: 'application/json'} + headers: {Accept: 'application/json'}, + timeoutMs: 10000 // 10 second timeout });src/routerClients/AppRouterClient.js (1)
45-47: Add error handling and documentation for createStore method.While moving session initialization to an async method is good for Next.js 15 compatibility, consider these improvements:
- Add JSDoc documentation explaining the method's purpose and usage
- Add error handling for the cookies() call
- Consider adding a return value for better async operation chaining
Here's a suggested implementation:
+ /** + * Initializes the session manager asynchronously. + * Must be called after constructor and before using any session-related features. + * @returns {Promise<void>} + * @throws {Error} If cookies cannot be accessed + */ async createStore () { - this.sessionManager = appRouterSessionManager(await cookies()); + try { + const cookieStore = await cookies(); + this.sessionManager = appRouterSessionManager(cookieStore); + } catch (error) { + console.error('Failed to initialize session manager:', error); + throw new Error('Session initialization failed'); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package.jsonis excluded by!**/*.json
📒 Files selected for processing (15)
- src/api-client.js (1 hunks)
- src/handlers/auth.js (1 hunks)
- src/routerClients/AppRouterClient.js (1 hunks)
- src/session/getAccessToken.js (1 hunks)
- src/session/getClaim.js (1 hunks)
- src/session/getFlag.js (1 hunks)
- src/session/getOrganization.js (1 hunks)
- src/session/getPermission.js (1 hunks)
- src/session/getPermissions.js (1 hunks)
- src/session/getRoles.js (1 hunks)
- src/session/getUser.ts (1 hunks)
- src/session/getUserOrganizations.ts (1 hunks)
- src/session/index.ts (1 hunks)
- src/session/sessionManager.js (1 hunks)
- src/utils/appRouter/setVerifierCookie.js (1 hunks)
🧰 Additional context used
🔇 Additional comments (21)
src/session/getRoles.js (1)
18-18: LGTM! Proper async handling added.The addition of
awaitbeforesessionManagerensures proper promise resolution, which is crucial for reliable session management in Next.js 15.src/session/getAccessToken.js (1)
Line range hint
15-27: LGTM: Error handling and type safety.The function maintains proper error handling with debug mode logging and preserves TypeScript types. The changes align with Next.js 15 async patterns while maintaining backward compatibility.
src/session/getPermissions.js (1)
18-18: LGTM! Proper async handling added.The addition of
awaitensures proper sequential execution of session management before fetching permissions, which is crucial for Next.js 15 compatibility.Let's verify this change is consistent across the codebase:
✅ Verification successful
Let me modify the search strategy to properly verify the sessionManager usage.
This will help us:
- See all sessionManager calls with their context to check for await
- Verify if the functions calling sessionManager are properly marked as async
The results will allow us to confirm if the async/await pattern is consistently applied across the codebase.
Async/await pattern is consistently applied for sessionManager
Based on the search results, I can confidently verify that:
- All direct calls to
sessionManagerare properly awaited across the codebase- The only exception is in
PagesRouterClient.jswhere it's used as a class property initialization, which is the correct pattern- The change in
getPermissions.jsaligns with the consistent async usage pattern throughout the codebase🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistent async usage of sessionManager # Expect: All calls to sessionManager should be awaited # Search for non-awaited calls to sessionManager rg "sessionManager\((?!.*await)" --type jsLength of output: 291
Script:
#!/bin/bash # Search for all sessionManager calls with surrounding context rg "sessionManager\(" -A 1 -B 1 --type js # Also search for async function declarations containing sessionManager ast-grep --pattern 'async function $NAME($_) { $$$ sessionManager($$$) $$$ }'Length of output: 2728
src/session/getPermission.js (1)
20-20: LGTM! Proper async handling added.The addition of
awaitensures proper async handling of the session management operation, maintaining the Promise chain correctly.src/session/getClaim.js (1)
21-21: Verify consistent async implementation across session management.Let's ensure this async pattern is consistently implemented across all session-related files.
✅ Verification successful
Let me analyze the results from the
rgcommand to verify the async implementation consistency across session management files.From the search results, I can see two distinct patterns:
- Direct await:
await sessionManager(req, res)
- Double await (for getting session items):
await (await sessionManager(req, res)).getSessionItem()There's one potential inconsistency in
src/routerClients/PagesRouterClient.js:this.sessionManager = sessionManager(req, res);Let me verify if this is actually an issue by checking the implementation context:
Let me check how this sessionManager is used within the PagesRouterClient class to ensure we're not missing any async operations:
Async implementation is consistent across session management
The verification shows that the async pattern is correctly implemented across all session-related files. The single case in
PagesRouterClient.jswheresessionManageris assigned withoutawaitis actually correct because:
- It's used in a constructor where async operations are not allowed
- The instance variable is only stored but never used within the class (confirmed by the last search)
- All other session management operations properly use
awaitwith either direct calls or when accessing session items🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistent async implementation of sessionManager across files # Expected: All sessionManager calls should be awaited # Search for sessionManager usage patterns echo "Checking for sessionManager usage patterns..." rg -A 2 "sessionManager\(req,\s*res\)" src/Length of output: 3083
Script:
#!/bin/bash # Check the implementation context of PagesRouterClient rg -B 5 -A 10 "this.sessionManager = sessionManager" src/routerClients/PagesRouterClient.jsLength of output: 429
Script:
#!/bin/bash # Search for usage of this.sessionManager within PagesRouterClient rg "this\.sessionManager" src/routerClients/PagesRouterClient.jsLength of output: 118
src/session/getFlag.js (2)
22-22: LGTM: Proper async session management implementation.The addition of
awaitforsessionManageris correct and aligns with the Next.js 15 support requirements while maintaining the existing interface.
Line range hint
28-35: Verify error handling with async flow.While the error handling logic looks correct, let's verify that errors from the async
sessionManagerare properly caught and handled.✅ Verification successful
Let me try a different approach to verify the error handling in the codebase.
This should help us understand:
- If there are dedicated test files for getFlag functionality
- The complete implementation context of getFlagFactory
- How sessionManager is used across the codebase, particularly its error handling patterns
Based on the shell script results, I can now provide a conclusive response about the error handling in getFlag.js:
Error handling for async sessionManager flow is properly implemented
The code analysis reveals that:
- The error handling in getFlag.js correctly catches errors from sessionManager, as it's within the try-catch block that wraps the await statement.
- The error handling pattern is consistent across the codebase, as seen in similar implementations in getBooleanFlag.js, getStringFlag.js, and getIntegerFlag.js.
- The sessionManager is used consistently throughout the codebase with proper error handling patterns, particularly for async operations.
- The specific error message check for 'no default value has been provided' is appropriate as it allows propagation of configuration-related errors while handling other types of errors gracefully by returning the default value.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for tests covering error scenarios in getFlag rg -l "getFlagFactory.*catch|test.*getFlag.*error" --type js --type ts # Look for other implementations to ensure consistent error handling ast-grep --pattern 'sessionManager($_, $_).catch($_)'Length of output: 128
Script:
#!/bin/bash # Search for test files related to getFlag fd "getFlag.*\.(test|spec)\.(js|ts|jsx|tsx)$" # Look for the implementation of getFlag and its error handling rg -A 5 "getFlagFactory" --type js --type ts # Search for sessionManager usage patterns rg -A 5 "sessionManager" --type js --type tsLength of output: 24763
src/session/getUser.ts (1)
Line range hint
1-24: Verify Next.js 15 compatibility requirementsWhile the async changes improve session management, it's not immediately clear if these changes are specifically required for Next.js 15 compatibility. Let's verify if these modifications are necessary.
src/session/getOrganization.js (2)
17-17: LGTM: Efficient session management implementation.The introduction of a stored session variable prevents redundant calls to
sessionManager, improving performance while maintaining the async flow required for Next.js 15 compatibility.
18-26: Verify error handling with async operations.The refactoring to use the stored session looks good. However, since we're dealing with multiple sequential async operations, we should ensure proper error handling for each async call.
Let's verify the error handling patterns across the codebase:
✅ Verification successful
Let me try a different approach to verify the error handling patterns.
The first command will help us find try-catch blocks around async operations, the second will show us raw async calls to compare against, and the third will help identify if there's a centralized error handling approach.
Let's analyze these results to make a proper assessment of the error handling patterns.
Let me check one more aspect to see if there's error handling at a higher level.
This will help us understand if error handling is implemented at a higher level in the component hierarchy or through middleware/wrappers.
Let me try one final search with corrected syntax.
This should give us a better picture of how errors are handled across the codebase.
Based on the search results, I can now provide a final response about the error handling verification:
Error handling is properly implemented for async operations
The code in
getOrganization.jsis already wrapped in a try-catch block that handles errors appropriately:try { const org = await kindeClient.getOrganization(session); // ... other async calls ... } catch (error) { console.error(error); return null; }The search results show consistent error handling patterns across the codebase:
- All session-related functions implement try-catch blocks
- Errors are properly logged using
console.error- Failed operations gracefully return
nullinstead of throwing unhandled exceptions- Similar error handling patterns are found in related files like
getUserOrganizations.ts,getPermissions.js, etc.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check error handling patterns in async operations # Look for try-catch blocks around async operations in session-related files # Search for error handling patterns in session-related files rg -A 5 "try\s*{.*await.*}\s*catch" src/session/Length of output: 165
Script:
#!/bin/bash # Search for try-catch blocks around async operations with proper regex rg -U "try\s*\{[\s\S]*?await.*?\}.*?catch" src/session/ # Also search for specific async operations without error handling rg "await kindeClient\." src/session/ # Look for any error handling middleware or utilities fd "error" src/Length of output: 1208
Script:
#!/bin/bash # Check the parent components/handlers that might be handling errors rg -B 3 -A 3 "getOrganization|getUserOrganizations" "src/**/*.{js,ts,jsx,tsx}" # Look for any error boundary or error handling wrappers rg "catch|throw|error" "src/**/*.{js,ts,jsx,tsx}" --type js --type typescriptLength of output: 260
Script:
#!/bin/bash # Check for error handling patterns in the entire src directory rg -B 3 -A 3 "getOrganization|getUserOrganizations" src/ # Look for any error handling patterns rg "catch|throw|error" src/ # Check if there's any middleware or wrapper that might handle errors rg -B 2 -A 2 "export.*function.*kindeClient" src/Length of output: 13749
src/session/getUserOrganizations.ts (2)
11-11: LGTM: Proper async/await conversionThe conversion of
sessionManagerto useawaitis correct and necessary for Next.js 15 compatibility while maintaining the existing interface.
Line range hint
8-35: Verify consistent async pattern across session managementThe async conversion pattern should be consistent across all session-related files to ensure proper session handling in Next.js 15.
✅ Verification successful
Let me analyze the results and generate one more verification script to get a complete picture of the async patterns.
This will help us verify if all functions using sessionManager are properly declared as async and follow a consistent pattern.
Based on the results from both verification attempts, I can now generate the final response.
Async patterns are consistently implemented in session management
The verification shows that all
sessionManagerusages across the codebase properly implement async/await patterns:
- In
src/session/index.ts: Usesawait sessionManager(req, res)- In
src/session/getUser.ts: Usesawait sessionManager(req, res)with proper chaining- In
src/session/getUserOrganizations.ts: Usesawait sessionManager(req, res)with proper async/await flowThe implementation in the reviewed code follows the same consistent pattern as other session-related files, and there are no instances of improper async handling detected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistent async/await usage with sessionManager across files # Expected: All sessionManager calls should use await # Search for sessionManager usage patterns echo "Checking sessionManager usage patterns:" rg -A 2 "sessionManager\(.*\)" --type ts # Search for potentially missed async function declarations echo -e "\nChecking for non-async functions using await:" ast-grep --pattern 'function $NAME($_) { $$$ await $$$ $$$ }'Length of output: 1092
Script:
#!/bin/bash # Description: Check for all async function declarations in session-related files # to verify consistency in async patterns # Search for async function declarations in session files echo "Checking async function declarations in session files:" ast-grep --pattern '(async function|async $_) $NAME($_) { $$$ }' # Search for arrow function patterns with async echo -e "\nChecking async arrow functions:" ast-grep --pattern '($_) => async $_'Length of output: 306
src/session/index.ts (1)
28-28: Verify the impact of async sessionManager across the codebase.The change to await sessionManager is part of the Next.js 15 compatibility updates. While the implementation is correct, we should verify that all consumers are prepared to handle the promise-based response.
Let's verify the usage:
Consider adding a comment in the function documentation to explicitly note that this is an async operation, as it represents a potentially breaking change in behavior.
✅ Verification successful
Let me gather more information about the session management implementation and its usage across the codebase.
The previous search was too specific. Let's broaden our search to understand the full context of session management and its consumers in the codebase. This will help us verify if the async change could have any breaking impact.
Based on the search results, I can now generate a final response as we have comprehensive information about sessionManager usage across the codebase.
The async sessionManager change is safe and already properly handled
The codebase scan reveals that sessionManager is consistently used with await throughout all its consumers:
- All direct uses of sessionManager are already properly awaited
- All functions using sessionManager are async functions
- Key files like
src/api-client.js,src/session/*.js|ts, and router clients are consistently handling promisesNotable examples of existing async usage:
src/api-client.js:const store = await sessionManager(req, res)src/session/getUser.ts:await (await sessionManager(req, res)).getSessionItem('id_token')src/session/getOrganization.js:const session = await sessionManager(req, res)The change to make sessionManager async in the refreshTokens method aligns with the existing async pattern throughout the codebase, making this a safe modification for Next.js 15 support.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for direct calls to refreshTokens to ensure they're properly awaited rg -l "refreshTokens\(" | xargs rg -A 2 "refreshTokens\(" # Look for any test files that might need updates fd "test|spec" | grep -i "refresh.*token"Length of output: 213
Script:
#!/bin/bash # Search for sessionManager usage rg -A 3 "sessionManager\(" # Look for files importing from session/index.ts rg -l "from ['\"].*session['\"]" # Search for any async/await patterns around session management rg -B 2 -A 2 "await.*session" # Look for test files related to session management fd "test|spec" | xargs rg -l "session"Length of output: 9447
src/api-client.js (2)
31-31: LGTM: Async session management aligns with Next.js 15.The conversion to async/await for session management is a good improvement that aligns with Next.js 15's enhanced support for asynchronous operations.
31-32: Verify the necessity of removing the API access token.The code immediately removes 'kinde_api_access_token' after getting the session store. This seems suspicious as:
- It's unclear why we need to remove this token
- There's no error handling if the removal fails
- The removal might affect other operations that need this token
Let's verify the token usage across the codebase:
✅ Verification successful
Let's gather more context about how this token is being used in the file.
Let me analyze the code further to understand the token management flow.
After analyzing the code, I can now provide a final response:
The removal of 'kinde_api_access_token' is intentional and correct
The code in
createKindeManagementAPIClientfollows a proper token management flow:
- It removes any existing management API token
- Immediately requests a new token using client credentials
- Uses the fresh token to configure the API client
This is the correct pattern because:
- The function generates a new client credentials token for management API access
- The old token is cleared to prevent token leakage or reuse
- Error handling isn't needed as the token removal is just cleanup
- The removal doesn't affect other operations as this token is specifically for management API access, separate from the regular user authentication tokens (which use different cookie names like 'access_token' and 'id_token')
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other uses of this token to understand the impact rg -l 'kinde_api_access_token' # Check if there are any comments or documentation explaining this behavior rg -l -g '!*.{lock,json}' 'kinde_api_access_token.*remov'Length of output: 108
Script:
#!/bin/bash # Get the full context of api-client.js to understand the token usage cat src/api-client.js # Search for any session-related operations to understand the session management flow rg -A 5 -B 5 'sessionManager|getSessionItem|setSessionItem|removeSessionItem'Length of output: 37892
Script:
#!/bin/bash # Get the context around where the token is removed rg -B 10 -A 10 'removeSessionItem.*kinde_api_access_token' # Look for any token generation or management related code rg -A 5 'oauth2/token|access_token' # Check if there are any tests that might explain the behavior fd -e test.js -e spec.js -e test.ts -e spec.ts | xargs catLength of output: 17656
src/routerClients/AppRouterClient.js (2)
39-39: Formatting change noted.
39-47: Verify proper initialization sequence across the codebase.The new async initialization pattern requires careful handling to prevent runtime errors. The session manager must be properly initialized before any session-related operations.
Let's verify the initialization sequence across the codebase:
✅ Verification successful
Let me gather more information about the usage of sessionManager to ensure it's not accessed before initialization.
Based on the gathered information, I can now generate the final response:
Initialization sequence is properly handled across the codebase
The verification shows that:
- In
src/handlers/auth.js,createStore()is always called immediately after instantiation and before any route handling:await routerClient.createStore()- All handler functions (
login,register,setup, etc.) receive the already initializedrouterClientas a parameter- No direct access to
sessionManageroccurs before initializationThe async initialization pattern is correctly implemented with proper sequencing, preventing potential runtime errors.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for AppRouterClient instantiation and verify createStore() calls echo "Checking AppRouterClient usage patterns..." rg -A 5 "new AppRouterClient" # Search for sessionManager usage to ensure it's not accessed before initialization echo "Checking sessionManager access patterns..." ast-grep --pattern 'this.sessionManager'Length of output: 818
Script:
#!/bin/bash # Search for methods in AppRouterClient that might use sessionManager echo "Checking methods using sessionManager in AppRouterClient..." ast-grep --pattern 'class AppRouterClient { $$$ $method($_) { $$$ this.sessionManager $$$ } $$$ }' # Search for direct usage of routerClient methods after instantiation echo "Checking usage of routerClient methods..." rg -B 2 -A 2 "routerClient\."Length of output: 9212
src/handlers/auth.js (2)
90-101: Verify Next.js version compatibility.The changes look good for Next.js 15 support, but let's verify the compatibility across different Next.js versions.
Run the following script to check for version-specific patterns:
✅ Verification successful
Based on the results, I can now generate the final response since we have clear evidence about Next.js version compatibility from both the codebase structure and package.json.
✓ Next.js version compatibility is properly maintained
The changes are verified to be compatible across Next.js versions because:
- The peer dependency in package.json explicitly supports versions
"^12.2.5 || ^13 || ^14 || ^15", showing intentional multi-version compatibility- The codebase structure separates router-specific logic into distinct clients (AppRouterClient.js and PagesRouterClient.js), indicating proper handling of different Next.js routing paradigms
- The async changes in auth.js align with Next.js 15's patterns while maintaining backward compatibility through the version range specified in peer dependencies
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for Next.js version compatibility # Look for Next.js imports and usage patterns echo "Checking Next.js imports and usage patterns..." rg -l "next|Next" --type js # Check package.json for Next.js version constraints echo "Checking Next.js version constraints..." if [ -f "package.json" ]; then cat package.json | grep -A 5 -B 5 "\"next\":" fiLength of output: 1331
90-92: LGTM! Proper handling of async params in Next.js 15.The change correctly handles the new async params behavior in Next.js 15 while maintaining backward compatibility.
Let's verify the params handling across different Next.js versions:
✅ Verification successful
Let me gather more context about the params usage to ensure a comprehensive verification.
The additional context will help us understand if these params usages are related to the authentication flow and if they need similar async handling.
Based on the gathered context, I can now generate the final response:
The async params handling change is correctly scoped to the auth handler
The params usage in
LoginLinkandRegisterLinkcomponents are different from the auth handler's params - they're usingURLSearchParamsfor query string manipulation, not route params. The async params change inauth.jsis specifically for handling Next.js 15's dynamic route params in the app router, and doesn't affect other params usage in the codebase.Key findings:
- The only
params.kindeAuthusage is in the auth handler- Other params usages are for URL query string manipulation
- The async handling is correctly isolated to the app router authentication flow
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any direct params access that might need similar updates # Look for patterns that might need async handling ast-grep --pattern 'params.$_'Length of output: 384
Script:
#!/bin/bash # Check the implementation context of params in these files rg -B 5 -A 5 "params" src/components/RegisterLink.js src/components/LoginLink.js # Also check for any other auth-related params usage rg -B 5 -A 5 "params.*kindeAuth"Length of output: 4463
src/session/sessionManager.js (2)
32-36: LGTM: Consistent async implementation.The async conversion aligns well with the existing Promise-based internal methods (getSessionItem, setSessionItem, etc.), making the overall implementation more consistent.
30-30: Verify the impact of async conversion.The change from synchronous
SessionManagertoPromise<SessionManager>is a breaking change that requires consumers to await the result. This seems to conflict with the PR objective of "no interface changes".Let's verify the usage across the codebase:
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- src/session/getAccessTokenRaw.js (1 hunks)
- src/session/getUser.ts (1 hunks)
- src/session/sessionManager.js (1 hunks)
- src/utils/appRouter/setVerifierCookie.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/session/getUser.ts
- src/session/sessionManager.js
- src/utils/appRouter/setVerifierCookie.js
🧰 Additional context used
🔇 Additional comments (1)
src/session/getAccessTokenRaw.js (1)
15-15: Verify TypeScript types for Next.js 15 compatibility.The
@ts-ignorecomment suggests potential type issues. Since this PR aims to add Next.js 15 support, we should ensure proper typing.✅ Verification successful
Let me gather more context about the specific file and its type-related concerns.
I now have enough context to provide the final response.
@ts-ignoreis justified here - no type issues with Next.js 15The
@ts-ignoreingetAccessTokenRaw.jsis actually warranted because:
- The file is using JSDoc annotations for typing in a JavaScript file
- It correctly imports and uses Next.js types via JSDoc's
@param {import('next').NextApiRequest}syntax- The function is properly typed and integrated with TypeScript files as seen in
types.d.tswhere it's declared asgetAccessTokenRaw: () => string | null- The implementation matches its TypeScript usage in the codebase
The presence of this
@ts-ignoredoesn't indicate any Next.js 15 compatibility issues, but rather handles the expected type checking behavior for a JavaScript file using JSDoc annotations.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for Next.js type definitions and their usage # Look for Next.js type imports and their usage rg -A 3 "from 'next'" # Check for other ts-ignore comments that might need review rg "@ts-ignore"Length of output: 2174
Script:
#!/bin/bash # Check the content of getAccessTokenRaw.js and surrounding context cat src/session/getAccessTokenRaw.js # Look for type definitions related to access tokens rg -g "*.ts" -g "*.d.ts" "interface.*Token|type.*Token" -A 2 # Check if there are TypeScript files importing or using getAccessTokenRaw rg "getAccessTokenRaw" -g "*.ts"Length of output: 1622
Explain your changes
Adds nextjs 15 support, no interface changes
Resolves: #222
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.