feat: add support for invitationCode#213
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughOn mount (SSR-safe) the provider detects Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser (window)
participant Provider as KindeProvider
participant AuthSDK as Auth SDK
participant Popup as Auth Popup / Redirect
Note over Browser,Provider: Client mount (SSR-safe detection)
Browser->>Provider: mount -> read URL search (invitation_code)
Provider->>Provider: set invitationCodeRef & isInvitationRedirectPending
Provider->>AuthSDK: login({ prompt: create, invitationCode }) -- once
AuthSDK->>Popup: open popup / redirect flow
Popup->>Provider: handleResult (auth params)
Provider->>Provider: processAuthResult (try/catch/finally) -> clear pending flag
Provider->>AuthSDK: exchangeAuthCode / checkAuth (init resumes)
Provider->>Browser: render children (if not pending or forceChildrenRender)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 0
🧹 Nitpick comments (4)
src/state/KindeProvider.tsx (4)
228-234: URL parsing runs on every render.
new URLSearchParams(window.location.search)executes on every render, even though the refs only initialize once. Consider memoizing this or using a lazy initialization pattern:- // Check for invitation_code synchronously before any hooks/rendering - const params = new URLSearchParams(window.location.search); - const hasInvitationCode = params.has("invitation_code"); - const invitationCodeRef = useRef<string | null>( - hasInvitationCode ? params.get("invitation_code") : null, - ); - const isRedirectingRef = useRef(hasInvitationCode); + // Check for invitation_code synchronously before any hooks/rendering + const invitationCodeRef = useRef<string | null>(null); + const isRedirectingRef = useRef(false); + + // Initialize refs only once + if (invitationCodeRef.current === null && isRedirectingRef.current === false) { + const params = new URLSearchParams(window.location.search); + if (params.has("invitation_code")) { + invitationCodeRef.current = params.get("invitation_code"); + isRedirectingRef.current = true; + } + }Alternatively, use a dedicated initialization ref to track whether parsing has been done.
375-376:loginRefis assigned but never read.
loginRefis assigned at line 436 but never used elsewhere. The useEffect at line 439 directly uses thelogincallback from the dependency array instead ofloginRef.current. Either remove the unused ref or use it for the intended immediate access pattern.If the ref is no longer needed:
const initRef = useRef(false); - const loginRef = useRef<typeof login | null>(null); const redirectInitiatedRef = useRef(false);And remove the assignment at lines 435-436.
439-456: Consider invokingonErrorcallback on invitation redirect failure.The catch block logs the error but doesn't notify the application via the
onErrorcallback. This could leave users on a blank screen (since children aren't rendered during redirect) without understanding what went wrong.}).catch((error) => { console.error("Error processing invitation code:", error); + mergedCallbacks.onError?.( + { + error: "ERR_INVITATION_REDIRECT", + errorDescription: String(error), + }, + {}, + {} as KindeContextProps, + ); isRedirectingRef.current = false; redirectInitiatedRef.current = false; });Also note: the
login &&check on line 443 is always true sinceloginis a stableuseCallbackreference.
788-793: Unused variable declaration before early return.The
paramsvariable declared at line 788 is unused whenisRedirectingRef.currentis true (the early return at line 792). Additionally, it shadows theparamsvariable from line 229. Consider removing this redundant parsing or moving it after the guard clause:const init = useCallback(async () => { if (initRef.current) return; try { - const params = new URLSearchParams(window.location.search); - // Skip initialization if redirecting for invitation (handled in useEffect above) if (isRedirectingRef.current) { return; } + const params = new URLSearchParams(window.location.search); + try { initRef.current = true;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package.jsonis excluded by!**/*.json
📒 Files selected for processing (1)
src/state/KindeProvider.tsx(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-05T12:17:11.365Z
Learnt from: DanielRivers
Repo: kinde-oss/kinde-auth-react PR: 173
File: src/state/KindeProvider.tsx:584-585
Timestamp: 2025-09-05T12:17:11.365Z
Learning: In Kinde auth React applications, the callback URL (redirectUri) is always absolute because it's required for hosted auth to work properly.
Applied to files:
src/state/KindeProvider.tsx
🔇 Additional comments (1)
src/state/KindeProvider.tsx (1)
890-893: LGTM!The conditional render suppression correctly prevents rendering children during the invitation redirect flow while respecting the
forceChildrenRenderescape hatch.
dtoxvanilla1991
left a comment
dtoxvanilla1991
left a comment
There was a problem hiding this comment.
great stuff. Left my thoughts.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/state/KindeProvider.tsx (1)
228-235:⚠️ Potential issue | 🟠 MajorGuard the invitation lookup from browser-only render access.
This still reads
window.locationduring render, so any SSR/prerender/test path that evaluates the provider without a real browser will throw before effects can guard it. Prefer atypeof window !== "undefined"lazy initializer and seed the ref/state from that value instead.Possible adjustment
- const params = new URLSearchParams(window.location.search); - const hasInvitationCode = params.has("invitation_code"); - const invitationCodeRef = useRef<string | null>( - hasInvitationCode ? params.get("invitation_code") : null, - ); - const [isInvitationRedirectPending, setIsInvitationRedirectPending] = - useState(hasInvitationCode); + const [initialInvitationCode] = useState<string | null>(() => { + if (typeof window === "undefined") return null; + return new URLSearchParams(window.location.search).get("invitation_code"); + }); + const invitationCodeRef = useRef<string | null>(initialInvitationCode); + const [isInvitationRedirectPending, setIsInvitationRedirectPending] = + useState(Boolean(initialInvitationCode));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/state/KindeProvider.tsx` around lines 228 - 235, The code reads window.location during render which will break SSR/prerender/test environments; change the initializers to be guarded by typeof window !== "undefined" and lazily compute the params only when window exists. Specifically, replace the eager creation of params/hasInvitationCode with guarded initial values for invitationCodeRef and isInvitationRedirectPending: initialize invitationCodeRef using typeof window !== "undefined" ? new URLSearchParams(window.location.search).get("invitation_code") : null, and initialize isInvitationRedirectPending via a lazy useState initializer that checks typeof window !== "undefined" && new URLSearchParams(window.location.search).has("invitation_code"). Keep references to invitationCodeRef, isInvitationRedirectPending, and setIsInvitationRedirectPending when making the change.
🧹 Nitpick comments (1)
src/state/KindeProvider.test.tsx (1)
89-126: Add a success-path invitation test for popup/in-place auth.This only covers the rejection case. The more fragile branch is a successful invitation flow that does not unload the current page, because that is where
isInvitationRedirectPendinghas to be cleared to avoid a blank screen. A focused test for that path would catch the regression above.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/state/KindeProvider.test.tsx` around lines 89 - 126, Add a focused test that covers the successful invitation flow for popup/in-place auth: mock generateAuthUrl to resolve (vi.mocked(generateAuthUrl).mockResolvedValue("http://fake")) instead of rejecting, set window.location to include ?invitation_code=... but do not simulate a navigation/unload, render <KindeProvider ...> with a test child, then assert the child renders (screen.getByText) and that the invitation flag is cleared (verify isInvitationRedirectPending is false or call the exported accessor/behavior that indicates it was cleared). This ensures the success-path that doesn’t unload the page (popup/in-place) clears isInvitationRedirectPending and avoids the blank-screen regression.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/state/KindeProvider.tsx`:
- Around line 439-457: The provider never clears the invitation pending gate
after a successful invitation-based login; update the success path inside
processAuthResult() (after mergedCallbacks.onEvent() is invoked) to call
setIsInvitationRedirectPending(false) so that isInvitationRedirectPending is
reset and init()/rendering can continue; ensure this only runs on successful
auth results and does not alter the existing error/catch behavior tied to
login().
---
Duplicate comments:
In `@src/state/KindeProvider.tsx`:
- Around line 228-235: The code reads window.location during render which will
break SSR/prerender/test environments; change the initializers to be guarded by
typeof window !== "undefined" and lazily compute the params only when window
exists. Specifically, replace the eager creation of params/hasInvitationCode
with guarded initial values for invitationCodeRef and
isInvitationRedirectPending: initialize invitationCodeRef using typeof window
!== "undefined" ? new
URLSearchParams(window.location.search).get("invitation_code") : null, and
initialize isInvitationRedirectPending via a lazy useState initializer that
checks typeof window !== "undefined" && new
URLSearchParams(window.location.search).has("invitation_code"). Keep references
to invitationCodeRef, isInvitationRedirectPending, and
setIsInvitationRedirectPending when making the change.
---
Nitpick comments:
In `@src/state/KindeProvider.test.tsx`:
- Around line 89-126: Add a focused test that covers the successful invitation
flow for popup/in-place auth: mock generateAuthUrl to resolve
(vi.mocked(generateAuthUrl).mockResolvedValue("http://fake")) instead of
rejecting, set window.location to include ?invitation_code=... but do not
simulate a navigation/unload, render <KindeProvider ...> with a test child, then
assert the child renders (screen.getByText) and that the invitation flag is
cleared (verify isInvitationRedirectPending is false or call the exported
accessor/behavior that indicates it was cleared). This ensures the success-path
that doesn’t unload the page (popup/in-place) clears isInvitationRedirectPending
and avoids the blank-screen regression.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 539de17c-07c2-4534-9b57-0819f6b68fa3
📒 Files selected for processing (2)
src/state/KindeProvider.test.tsxsrc/state/KindeProvider.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/state/KindeProvider.tsx (1)
247-252:⚠️ Potential issue | 🟠 MajorEnsure every invitation-pending path unblocks the provider.
Line 251 treats
?invitation_code=as pending, but Line 482 rejects the empty value, so no login runs and pending never clears. Also, Line 490 only catches rejectedlogin()calls;navigateToKindeerrors are swallowed insidelogin, so that failure path also leaves the pending gate active and keepsinit()/rendering blocked.Suggested fix
const params = new URLSearchParams(window.location.search); const invitationCode = params.get("invitation_code"); return { invitationCode, - redirectPending: params.has("invitation_code"), + redirectPending: Boolean(invitationCode), };Also clear the pending gate in
login’s internal navigation-error path when this is an invitation login:} catch (error) { setLoading(false); + if (options.invitationCode) { + redirectInitiatedRef.current = false; + setIsInvitationRedirectPending(false); + } if (!contextRef.current) { console.error("Login error (context unavailable):", error); return; }Also applies to: 487-494
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/state/KindeProvider.tsx` around lines 247 - 252, The provider considers any presence of ?invitation_code= as pending but elsewhere rejects empty values and navigation errors leave the pending gate set; update the invitation detection to only treat non-empty invitationCode as pending (use params.get("invitation_code") !== null && params.get("invitation_code") !== "") instead of params.has), and in the login() flow where navigateToKinde can fail (the internal navigation-error path), ensure you clear/unblock the invitation pending flag (the same state used for redirectPending) before swallowing or rethrowing the error so init()/rendering is not permanently blocked; reference the invitationCode/redirectPending logic and the login()/navigateToKinde() error branch when making the changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/state/KindeProvider.test.tsx`:
- Line 399: The test sets exchangeAuthCodeMock to always resolve success which
can leak across tests because beforeEach only calls vi.clearAllMocks(); update
the test setup so beforeEach resets the mock implementation for
exchangeAuthCodeMock (e.g., call exchangeAuthCodeMock.mockReset() or
exchangeAuthCodeMock.mockImplementation(() => default) after vi.clearAllMocks())
to restore default behavior between tests; locate exchangeAuthCodeMock and the
beforeEach block in KindeProvider.test.tsx and add the reset so individual tests
can set their own mockResolvedValue without leaking.
---
Duplicate comments:
In `@src/state/KindeProvider.tsx`:
- Around line 247-252: The provider considers any presence of ?invitation_code=
as pending but elsewhere rejects empty values and navigation errors leave the
pending gate set; update the invitation detection to only treat non-empty
invitationCode as pending (use params.get("invitation_code") !== null &&
params.get("invitation_code") !== "") instead of params.has), and in the login()
flow where navigateToKinde can fail (the internal navigation-error path), ensure
you clear/unblock the invitation pending flag (the same state used for
redirectPending) before swallowing or rethrowing the error so init()/rendering
is not permanently blocked; reference the invitationCode/redirectPending logic
and the login()/navigateToKinde() error branch when making the changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3913a114-a9b5-4baf-ba6a-bbe9ed622703
📒 Files selected for processing (2)
src/state/KindeProvider.test.tsxsrc/state/KindeProvider.tsx
dtoxvanilla1991
left a comment
dtoxvanilla1991
left a comment
There was a problem hiding this comment.
I have taken a deeper look at this again. I got a few non-critical but worthy patching that could be done on this, if there is some time. Should not take much. Leaving it as comments.
…for invitation redirect failure
4 participants
Explain your changes
Add support for invitations
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.