-
Notifications
You must be signed in to change notification settings - Fork 87
fix: match edge runtime pages with optional trailing slash #1892
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ 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 next-plugin-edge-middleware 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-next-auth-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-demo 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. |
✅ Deploy Preview for next-plugin-canary 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
7fc0380
to
747af5d
Compare
@@ -0,0 +1,23 @@ | |||
import type { Context } from 'https://edge.netlify.com' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've split this into a separate entrypoint, because functions don't need custom matcher handling or support for advanced middleware - but do want trailing slash handling unlike middleware
|
||
const matchesMiddleware: MiddlewareRouteMatch = getMiddlewareRouteMatcher(matchers || []) | ||
|
||
export interface FetchEventResult { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved all of this into a shared file, so that we can have two separate entrypoints. They could go in a layer eventually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testsed this with https://deploy-preview-1892--next-plugin-edge-middleware.netlify.app/api/edge and https://deploy-preview-1892--next-plugin-edge-middleware.netlify.app/api/edge/ and they both load the hello world example page.
and the redirect from https://deploy-preview-1892--next-plugin-edge-middleware.netlify.app/api/edge/ to https://deploy-preview-1892--next-plugin-edge-middleware.netlify.app/api/edge
🚀
75f4833
to
8252753
Compare
Summary
Currently appDir edge runtime pages use the
matcher
regex to generate edge function pattern. However this regex is designed to match a normalised path, so doesn't include a trailing slash. This meant that some pages with the edge runtime did not match the edge function, causing server errors.This PR changes edge functions to not use the regex from the matcher for pages, but instead use the regex from the routes manifest. This includes the optional trailing slash.
To make this work, I also refactored the handling of these matchers, splitting the generation of the functions from the creation of the matchers, and no longer use the same function for middleware and functions as they use different logic.
Test plan
Relevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal
Fixes #1798
Standard checks:
🧪 Once merged, make sure to update the version if needed and that it was published correctly.