-
Notifications
You must be signed in to change notification settings - Fork 0
Fix: Next.js 16 ssr detection #9
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
WalkthroughThe pull request updates Next.js SSR detection: the framework mapping now recognizes Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
hmacr
left a comment
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.
LGTM
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/unit/DetectorTest.php (2)
355-357: Add coverage for “.next/turbopack” (non-server layout) or align mapping.There’s an SSR case for “.next/server/turbopack”, but none for “.next/turbopack”. Either add a test for the non-server path or confirm we won’t support it.
return [ - [['server/pages/index.html', 'server/pages/api/users.js', '.next/server/pages/_app.js'], 'nextjs', 'static', null], - [['server/pages/index.html', 'server/pages/api/users.js', '.next/server/turbopack'], 'nextjs', 'ssr', null], - [['server/pages/index.html', 'server/pages/api/users.js', '.next/server/webpack-runtime.js'], 'nextjs', 'ssr', null], + [['server/pages/index.html', 'server/pages/api/users.js', '.next/server/pages/_app.js'], 'nextjs', 'static', null], + [['server/pages/index.html', 'server/pages/api/users.js', '.next/server/turbopack'], 'nextjs', 'ssr', null], + [['server/pages/index.html', 'server/pages/api/users.js', '.next/server/webpack-runtime.js'], 'nextjs', 'ssr', null], + [['server/pages/index.html', 'server/pages/api/users.js', '.next/turbopack'], 'nextjs', 'ssr', null], ];
355-355: Confirm intentional drop of legacy Next ≤15 SSR marker.Changing “.next/server/pages/_app.js” to classify as static will make older Next SSR builds detect as static. OK to drop backward compatibility?
In next.js 16 they removed _app.js we used to use for detection.
I now use same files as in Open Runtimes: https://github.com/open-runtimes/open-runtimes/blob/e9d2ba04ebc1c414acb551355cbeff904f0c326b/runtimes/node/versions/latest/helpers/next-js/bundle.sh#L10-L11
If this breaks in future, I will involve Next.js team to help us find futureproof solution. But this should be good and solid for now.
Summary by CodeRabbit