-
Notifications
You must be signed in to change notification settings - Fork 432
bugfix: Remove React dependency from server helpers to fix edge runtime bundling #2329
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2329 +/- ##
=======================================
Coverage 85.47% 85.47%
=======================================
Files 26 26
Lines 2636 2636
Branches 486 486
=======================================
Hits 2253 2253
Misses 377 377
Partials 6 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Claude finished @tusharpandey13's task —— View job Code Review Complete ✅
Summary: This PR effectively solves the edge runtime bundling issue by removing the React import dependency from server helpers. The changes are minimal, focused, and maintain type safety while eliminating the problematic React reference. The solution is appropriate and ready for merge. |
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.
Overall Assessment
✅ This pull request is ready to be merged
The changes effectively solve the edge runtime bundling issue by removing the React import dependency from server helpers. The type safety is maintained while eliminating the problematic React reference that caused bundling errors in Next.js middleware and edge runtime contexts.
Key points:
- Clean removal of React import without compromising functionality
- Appropriate use of
unknown
type maintains type safety while avoiding React dependency - Test updates correctly handle the type assertion
- All existing tests pass, indicating no regressions
Fixes React import dependency in server-side helpers that caused bundling errors in Next.js middleware and edge runtime environments.
🔍 RCA
The
import type React from "react"
statement inwith-page-auth-required.ts
created a transitive dependency that caused Next.js bundler to include React references in edge runtime contexts where React is not available.This affected ANY import from
@auth0/nextjs-auth0/server
due to module export bundling, not just direct usage ofwithPageAuthRequired
.📋 Changes
Changes
AppRouterPageRoute
return type fromPromise<React.JSX.Element>
toPromise<unknown>
to maintain type safety without requiring React.src/server/helpers/with-page-auth-required.ts
: Remove React import and changeAppRouterPageRoute
return type toPromise<unknown>
src/server/helpers/with-page-auth-required.test.ts
: Add type assertion for test compatibility with ReactDOMServer📎 References
Fixes: #2328
🎯 Testing
Automated:
Manual:
Auth0Client
from@auth0/nextjs-auth0/server
in Next.js middlewarewithPageAuthRequired
still works with App Router pagesdist/server/
files