feat: add enhanced middleware support #1479
Conversation
✅ Deploy Preview for next-hp-edge-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-nx-monorepo-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-export-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-static-root-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
github-actions
bot
added
the
type: feature
✅ Deploy Preview for netlify-plugin-nextjs-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-plugin-edge-middleware ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-plugin-rsc-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-i18next-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-next-auth-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-plugin-canary ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for nextjs-plugin-custom-routes-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
ascorbic
force-pushed
the
mk/rewrite-props-middleware
branch
from
2f92ed1 to
b0d9952
Compare
ascorbic
force-pushed
the
mk/rewrite-props-middleware
branch
from
b0d9952 to
a2c88b7
Compare
ascorbic
force-pushed
the
mk/rewrite-props-middleware
branch
from
fbb2223 to
ba284d9
Compare
|
OK, I've had a rethink about naming and changed it to |
ascorbic
force-pushed
the
mk/rewrite-props-middleware
branch
from
b82d367 to
d4c0dc0
Compare
|
OK, I'm marking this ready for review. I suggest we put this out so people can try it. It's opt-in, so shouldn't affect anything. We may decide to publish it in a separate package, but right now it can be imported from here. |
|
|
||
| import { MiddlewareResponse } from './response' | ||
|
|
||
| // TODO: add Context type |
There was a problem hiding this comment.
Looks like this comment can be removed
There was a problem hiding this comment.
No, I mean the real context type, if it gets published at some point
plugin/src/middleware/response.ts
Outdated
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| export type NextDataTransform = <T extends Record<string, any>>(props: T) => T | ||
|
|
||
| // A NextReponse that wraps the Netlify origin response |
There was a problem hiding this comment.
[dust] small typo
| // A NextReponse that wraps the Netlify origin response | |
| // A NextResponse that wraps the Netlify origin response |
|
So it turns out we don't have middleware e2e tests at all, so I've added some, including tests for the new stuff. |
| let response | ||
| const { | ||
| nextUrl: { pathname }, | ||
| } = request | ||
| } = req |
There was a problem hiding this comment.
[dust] Use the full name request and response for variable names.
There was a problem hiding this comment.
request and response are already used for the MiddlewareRequest and MiddlewareResponse. I considered changing them to nextRequest and nextResponse but thought it made the code examples a bit verbose. Hard to say which is better tbh
There was a problem hiding this comment.
I realise I've mixed the different styles, so yeah this should be tidied up in a follow-up.
| import type { ElementHandlers } from './html-rewriter' | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| export type NextDataTransform = <T extends { pageProps?: Record<string, any> }>(props: T) => T |
There was a problem hiding this comment.
[dust] Prefer unknown
| export type NextDataTransform = <T extends { pageProps?: Record<string, any> }>(props: T) => T | |
| export type NextDataTransform = <T extends { pageProps?: Record<string, unknown> }>(props: T) => T |
There was a problem hiding this comment.
The reason I didn't do this is because it would mean that users would need to create a type and explicitly cast the props value whenever they used transformData, which could be quite cumbersome. I'll add a comment.
|
|
||
| get headers(): Headers { | ||
| // If we have the origin response, we should use its headers | ||
| return this.originResponse?.headers || super.headers |
There was a problem hiding this comment.
[dust] Prefer nullish coalescing.
| return this.originResponse?.headers || super.headers | |
| return this.originResponse?.headers ?? super.headers |
There was a problem hiding this comment.
I love nullish coalescing, but IMO it's only worth using if there's a legitimate reason for the value to be falsy. Otherwise it just introduces a (tiny) bit of extra transpiled code and execution time for no reason. This will either be a Headers object or undefined, so a simple || does the job.
There was a problem hiding this comment.
This looks great @ascorbic! This is definitely in a great place where folks can start playing with this. 🚀
Summary
This demo shows a way of adding some of the powers of Netlify Edge Functions to Next.js Middleware. Normal Next.js middleware doesn't provide actual access to the Response but rather allows the user to return a
NextResponseobject which is just used by the handler to set headers etc on the real response when it is returned. CallingNextResponse.next()doesn't actually send a request to the origin, but is just a placeholder for setting response headers which are applied later. It also does not allow changes to be made to the Request, but instead allows arewrite()to be returned, which the wrapper uses to change the request. This library gives proper access to Request and Response in much the same way that Netlify Edge Functions do, but with some additional Next-specific helpers.Full details here: https://github.com/netlify/pod-ecosystem-frameworks/issues/183