middleware/proxy fixes for next.js 15/16 #9631
Conversation
Summary of ChangesHello @leoortizz, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Next.js framework adapter to ensure full compatibility with Next.js 16. It introduces support for the new Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request does an excellent job of updating the Next.js framework adapter to support middleware changes in Next.js 15 and 16, particularly the new proxy convention. The implementation correctly reads from functions-config-manifest.json while maintaining backward compatibility by falling back to middleware-manifest.json. The updates to development mode detection and the addition of comprehensive TypeScript definitions with source links are great for maintainability. The test suite has also been thoughtfully refactored to cover the new scenarios. My feedback is focused on minor improvements to the new TypeScript interfaces to enhance consistency and reusability.
| | MiddlewareManifestV1 | ||
| | MiddlewareManifestV2FromNext | ||
| | MiddlewareManifestV3; |
There was a problem hiding this comment.
Are there reasonable more descriptive names that could be used instead of V1 or V3?
Are these all just different manifest types introduced at different nextjs versions?
For example could MiddlewareManifestNext16 or something work for MiddlewareManifestV3 ?
There was a problem hiding this comment.
ah nvm, these are literally the versions on the interface itself.
There was a problem hiding this comment.
Actually, would we some day remove support for very old nextjs versions? In which case would some of these manifest versions would no longer be needed? could those versions be in the docstrings for MiddlewareManifestv2 and v1 ?
I think the "Middleware manifest types for Next.js 16" you added for v3 is very helpful.
annajowang
left a comment
annajowang
left a comment
There was a problem hiding this comment.
More questions than feedback. As we're still ramping up I'll defer to james for final merge approval
| | MiddlewareManifestV1 | ||
| | MiddlewareManifestV2FromNext | ||
| | MiddlewareManifestV3; |
There was a problem hiding this comment.
ah nvm, these are literally the versions on the interface itself.
| * @param isDevMode whether the project is running on dev or production | ||
| */ | ||
| export async function isUsingMiddleware(dir: string, isDevMode: boolean): Promise<boolean> { | ||
| if (isDevMode) { |
There was a problem hiding this comment.
Sorry maybe it's more obvious to those who know nextjs really well. But why is checking for middleware and proxy files in devMode enough? Could that be documented?
| | MiddlewareManifestV1 | ||
| | MiddlewareManifestV2FromNext | ||
| | MiddlewareManifestV3; |
There was a problem hiding this comment.
Actually, would we some day remove support for very old nextjs versions? In which case would some of these manifest versions would no longer be needed? could those versions be in the docstrings for MiddlewareManifestv2 and v1 ?
I think the "Middleware manifest types for Next.js 16" you added for v3 is very helpful.
| @@ -1,3 +1,3 @@ | |||
| import type { RouteHas } from "next/dist/lib/load-custom-routes"; | |||
| import type { ImageConfigComplete } from "next/dist/shared/lib/image-config"; | |||
| import type { MiddlewareManifest as MiddlewareManifestV2FromNext } from "next/dist/build/webpack/plugins/middleware-plugin"; | |||
There was a problem hiding this comment.
Was there a specific reason we decided not to copy v2 over like the others?
are we pinning to a specific version of the plugin somewhere? if that version gets wouldn't this break?
2 participants
Description
This PR updates the Next.js framework adapter to seamlessly support middleware changes introduced in Next.js 16, specifically handling both the new
proxyconvention and the deprecatedmiddlewareconvention.functions-config-manifest.json, ensuring compatibility with Next.js 16's Node.js runtime middleware (proxy.ts/js).middleware.ts/jsconvention by reading frommiddleware-manifest.json.isUsingMiddlewareto automatically detectproxy.jsandproxy.tsfiles in development mode, in addition to existing checks.FunctionsConfigManifest,MiddlewareManifestV3, etc.) with documentation linking to the source.Scenarios Tested
functions-config-manifest.jsonwhen using the newproxy.tsconvention.middleware-manifest.jsonwhen using the deprecatedmiddleware.tsconvention.proxy.js/tsalongsidemiddleware.js/tsin dev mode(emulator).Sample Commands
firebase deploywill now automatically handle Next.js 16 projects using either the new proxy or legacy middleware conventions without user intervention.