fix: redirect to home after login (#2726)#2729
Conversation
📝 WalkthroughWalkthroughAppShell now subscribes to a ChangesSession Token Event Synchronization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches⚔️ Resolve merge conflicts
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/App.tsx`:
- Line 85: The handler function's event parameter in App.tsx is untyped; add a
TypeScript annotation to improve type safety by changing the signature for
handler to accept a CustomEvent with the expected shape (CustomEvent<{
sessionToken?: string }>) or, if the event can be non-CustomEvent, type it as
Event and cast when reading detail; update the handler definition (handler) and
any detail access sites to use the chosen typed event so sessionToken is
correctly typed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
|
|
||
| // Listen for core-state:session-token-updated event from deep-link login, and update context | ||
| useEffect(() => { | ||
| const handler = (event) => { |
There was a problem hiding this comment.
Add TypeScript type annotation for the event parameter.
The event handler's parameter lacks an explicit type annotation. For type safety and better IDE support, type it as CustomEvent with the expected detail shape:
const handler = (event: CustomEvent<{ sessionToken?: string }>) => {Alternatively, if the event might not always be a CustomEvent, use Event and cast when accessing detail:
const handler = (event: Event) => {
const sessionToken = (event as CustomEvent<{ sessionToken?: string }>).detail?.sessionToken;🔧 Proposed fix with CustomEvent type
- const handler = (event) => {
+ const handler = (event: CustomEvent<{ sessionToken?: string }>) => {
const sessionToken = event.detail?.sessionToken;
if (sessionToken) {
storeSessionToken(sessionToken);
}
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/App.tsx` at line 85, The handler function's event parameter in
App.tsx is untyped; add a TypeScript annotation to improve type safety by
changing the signature for handler to accept a CustomEvent with the expected
shape (CustomEvent<{ sessionToken?: string }>) or, if the event can be
non-CustomEvent, type it as Event and cast when reading detail; update the
handler definition (handler) and any detail access sites to use the chosen typed
event so sessionToken is correctly typed.
still not working |
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
@Sushanth012 heads up — there are merge conflicts on this PR, so I'll hold off on a full review until those are sorted out. I see CodeRabbit flagged a TypeScript issue with the event handler (untyped parameter) that also needs fixing. Once those are resolved, I'll give this a proper review.
The approach looks solid though — synchronizing the session token from the core event to React context is exactly what's needed to fix the redirect issue after login.
|
@Sushanth012 this PR has merge conflicts with main — please rebase/resolve before review. |
|
This PR has merge conflicts with main — please rebase/resolve before review. |
3 participants
Issue Fixed
What was broken
Fix
storeSessionToken). The routing guards then automatically route users home on login.Summary by CodeRabbit