Skip to content

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

Merged
merged 4 commits into from
Nov 23, 2023

Conversation

lforst
Copy link
Contributor

@lforst lforst commented Nov 22, 2023

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 be Always 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:

Screenshot 2023-11-22 at 15 33 17

Copy link
Contributor

github-actions bot commented Nov 22, 2023

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped) 66.02 KB (0%)
@sentry/browser (incl. Tracing, Replay) - Webpack with treeshaking flags (gzipped) 56.21 KB (0%)
@sentry/browser (incl. Tracing) - Webpack (gzipped) 31.19 KB (0%)
@sentry/browser - Webpack (gzipped) 21.4 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped) 62.65 KB (0%)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped) 29.47 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped) 21.55 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed) 197.27 KB (0%)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed) 89.15 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed) 64.12 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped) 32.12 KB (0%)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped) 66.43 KB (0%)
@sentry/react - Webpack (gzipped) 21.45 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped) 83.16 KB (0%)
@sentry/nextjs Client - Webpack (gzipped) 48.32 KB (0%)
@sentry-internal/feedback - Webpack (gzipped) 16.19 KB (0%)

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');
Copy link

@pladaria pladaria Nov 22, 2023

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.

Copy link
Contributor Author

@lforst lforst Nov 22, 2023

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!

@lforst lforst marked this pull request as ready for review November 23, 2023 12:03
@lforst lforst requested review from mydea and AbhiPrasad November 23, 2023 12:04
@lforst lforst merged commit 8c9ff6b into develop Nov 23, 2023
@lforst lforst deleted the lforst-fix-middleware-detection-logic branch November 23, 2023 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Not working from Next.js middleware (edge runtime) when not using src folder
3 participants