🐞 fix (server): support for awaiting next.js 15 params #1805
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe changes involve modifying the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant Handler
Client->>Server: Request
Server->>Handler: Process Request
Handler->>Handler: Await context.params
Handler->>Handler: Check if path exists
Handler->>Handler: Construct path variable
Handler-->>Server: Response
Server-->>Client: Return Response
Assessment against linked issues
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: 0
🧹 Outside diff range and nitpick comments (2)
packages/server/src/next/app-route-handler.ts (2)
31-42: Consider enhancing error handling for params resolution.The async params handling is implemented correctly. However, consider adding explicit error handling for the
await context.paramsoperation.Consider wrapping the params resolution in a try-catch:
- const params = await context.params; + let params; + try { + params = await context.params; + } catch (error) { + return NextResponse.json( + { message: 'Failed to resolve request parameters' }, + { status: 500 } + ); + }
Line range hint
13-19: Consider updating function documentation.The function documentation should mention that
context.paramsis now asynchronous in Next.js 15+. This helps developers understand the potential need for error handling around params resolution.Consider adding a note in the JSDoc:
/** * Creates a Next.js 13 "app dir" API route request handler which encapsulates Prisma CRUD operations. * + * @remarks Since Next.js 15, `context.params` is asynchronous and must be awaited. * @param options Options for initialization * @returns An API route request handler */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- packages/server/src/next/app-route-handler.ts (2 hunks)
🔇 Additional comments (1)
packages/server/src/next/app-route-handler.ts (1)
9-9: LGTM! Note: Breaking change in Context type.The update to make
paramsasynchronous aligns with Next.js 15's behavior. This is a breaking change that requires consumers to update their code to await params.Let's check for any direct usage of this type in the codebase:
✅ Verification successful
Let me try a different search strategy to verify the usage of this Context type.
Let me search for any references to this specific file to ensure we catch all potential imports, even if they're importing the type differently.
Let me check how the AppRouteHandler is being used in the importing file to understand the impact.
Based on the verification results, I can now provide the final response:
Breaking change is properly encapsulated and has limited impact
The Context type change is well-contained within the
app-route-handler.tsmodule and is only used internally. The module is exported as a default export and consumed through theNextRequestHandlerfactory function inindex.ts, which properly encapsulates the internal changes. External consumers interact with the module through the documentedAppRouteRequestHandlerOptionsinterface, which is not affected by this change.
- The type
Contextis not exported or directly used outside ofapp-route-handler.ts- The only import of this module is in
index.tswhich uses it as an implementation detail- The public API (
NextRequestHandlerand its options interfaces) remains unchanged🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any other files that might be using the Context type rg -l "Context.*=.*{.*params.*}"Length of output: 154
Script:
#!/bin/bash # Search for Context type usage with proper escaping and context rg -A 2 "type Context" # Also search for imports of Context type rg "import.*Context.*from.*app-route-handler"Length of output: 1007
Script:
#!/bin/bash # Search for any imports from app-route-handler.ts rg "from ['\"].*app-route-handler['\"]" # Search for any usage of the specific params pattern within the codebase ast-grep --pattern 'const { params } = $_: Context'Length of output: 199
Script:
#!/bin/bash # Get the content of the importing file to see how AppRouteHandler is used cat packages/server/src/next/index.ts # Also search for any other files that might be using the handler rg -l "AppRouteHandler"Length of output: 1914
|
I refactored the code a little bit to include @coderabbitai's suggestions, they were actually pretty good and didn't expect AI to generate good code reviews. |
|
Tip For best results, initiate chat on the files or code changes. @AmruthPillai Thank you for incorporating my suggestions! I'm delighted to hear that you found them helpful. (^・ω・^ ) If you found this review helpful, would you consider giving us a shout-out on X? |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
packages/server/src/next/app-route-handler.ts (2)
14-14: Consider enhancing the documentation with a reference link.While the remark clearly explains the Next.js 15 change, it would be more helpful to include a link to the relevant Next.js documentation for further reference.
- * @remarks Since Next.js 15, `context.params` is asynchronous and must be awaited. + * @remarks Since Next.js 15, `context.params` is asynchronous and must be awaited. + * @see https://nextjs.org/docs/app/api-reference/file-conventions/route#params-and-searchparams
35-39: Consider adding error logging for better debugging.While the error handling is correct, logging the actual error would help with debugging in production environments.
try { params = await context.params; } catch (error) { + options.logger?.error('Failed to resolve request parameters:', error); return NextResponse.json({ message: 'Failed to resolve request parameters' }, { status: 500 }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- packages/server/src/next/app-route-handler.ts (2 hunks)
🔇 Additional comments (1)
packages/server/src/next/app-route-handler.ts (1)
9-9: LGTM: Type definition properly handles async params.The updated
Contexttype correctly accommodates both Promise-based params (Next.js 15+) and synchronous params (older versions), ensuring backward compatibility.
Yes, it sometimes helps me find important issues. I go through its comments every time 😄. The change looks good to me and I'm merging it. Than you for the contribution! |
|
Tip For best results, initiate chat on the files or code changes. @ymc9 Thank you for your support! I'm happy to hear that my suggestions were helpful. (^・ω・^ ) |
paramsin Route Handlers to be awaited #1797