-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(nextjs): Fix middleware detection logic #9637
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
size-limit report 📦
|
const middlewareJsPath = path.join(pagesDirPath, '..', 'middleware.js'); | ||
const middlewareTsPath = path.join(pagesDirPath, '..', 'middleware.ts'); | ||
const middlewareJsPath = path.join(middlewareLocationFolder, 'middleware.js'); | ||
const middlewareTsPath = path.join(middlewareLocationFolder, 'middleware.ts'); |
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.
Note that some people use the convention of using .tsx
for all typescript files, even if they don't contain JSX
Next.js allows middlewares with .tsx
extension (and probably also jsx, not tested)
Perhaps it would be a good idea to accept middlewares with any of the pageExtensions
.
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.
Ugh, you might be right. I just tested it. Once again not in line with what the Next.js docs say: https://nextjs.org/docs/app/building-your-application/routing/middleware#convention Thanks!
Fixes #9631
Our middleware detection logic was flawed because it looked into
{root}/src
if there was no pages folder while, in fact, the logic for middleware seems to beAlways follow the location of the pages folder, except for when it doesn't exist, then follow the location of the app folder.
.These are the results of my tests: