-
Notifications
You must be signed in to change notification settings - Fork 430
fix: Remove unsafe type assertion in withPageAuthRequired HOC #2305
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 #2305 +/- ##
==========================================
+ Coverage 85.34% 85.37% +0.03%
==========================================
Files 26 26
Lines 2613 2619 +6
Branches 481 482 +1
==========================================
+ Hits 2230 2236 +6
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 ✅Todo List:
SummaryCompleted code review of PR #2305. The change successfully improves type safety by replacing unsafe |
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 PR improves type safety by removing the unsafe as any
cast
The change successfully replaces the problematic as any
with a more specific type assertion. While there's still a minor type safety concern with potential prop conflicts, this is a significant improvement over the previous implementation. The change maintains the same runtime behavior while providing better TypeScript checking.
…wing type Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
….ComponentProps<typeof Component>, 'user'> here
Improves type safety by replacing unsafe
as any
cast with proper TypeScript type assertion in thewithPageAuthRequired
higher-order component.Fixes: #2288
🔍 RCA
The
withPageAuthRequired
HOC used an unsafeas any
type assertion for prop spreading, which bypassed TypeScript's type checking and indicated potential type compatibility issues between generic constraints and component props.📋 Changes
This change improves type safety without affecting runtime behavior. The HOC now uses proper TypeScript type assertions that maintain compile-time type checking while preserving the same functionality.
Changed
src/client/helpers/with-page-auth-required.tsx
: Replace unsafeas any
cast with type-safeReact.ComponentProps<typeof Component>
assertion📎 References
🎯 Testing
Automated:
All existing tests continue to pass (302/302 test suite). No new tests required as this is a type-level improvement without functional changes.
Manual:
npx tsc --noEmit
pnpm build
yalc publish
andyalc add @auth0/nextjs-auth0
withPageAuthRequired
HOC functions identically to previous behavior