Migrate sample app to use login widget 2.0 - #123
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe Login Widget sample now uses ChangesLogin Widget OIDC migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant useLoginWidget
participant LoginWidget
participant user.tokens
participant ProtectedAPI
Browser->>useLoginWidget: Call openModal()
useLoginWidget->>LoginWidget: Start selected or default journey
useLoginWidget->>user.tokens: Retrieve access token
user.tokens-->>useLoginWidget: Return accessToken
useLoginWidget->>ProtectedAPI: Fetch with Bearer authorization
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
…nd journey name URL param support
…ort custom journeys through JOURNEY_LOGIN env variable
vatsalparikh
force-pushed
the
migrate-login-widget-2.0
branch
from
July 17, 2026 17:13
93e33fb to
9871bab
Compare
vatsalparikh
changed the title
8 tasks
vatsalparikh
force-pushed
the
migrate-login-widget-2.0
branch
3 times, most recently
from
July 17, 2026 19:59
2fe709c to
7a74a8a
Compare
![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 `@javascript/reactjs-todo-login-widget/client/utilities/request.js`:
- Around line 34-38: Update the token retrieval flow around tokenEvents.get() to
inspect and handle the returned error before destructuring or accessing
response. Preserve the actual token retrieval failure when present, and only
fall back to the generic “Failed to retrieve access token” error when no error
exists but accessToken is missing.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 95649044-f27c-41df-be47-0287c5e3e780
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
javascript/reactjs-todo-davinci/README.mdjavascript/reactjs-todo-journey/README.mdjavascript/reactjs-todo-login-widget/.env.examplejavascript/reactjs-todo-login-widget/README.mdjavascript/reactjs-todo-login-widget/client/constants.jsjavascript/reactjs-todo-login-widget/client/hooks/useLoginWidget.jsjavascript/reactjs-todo-login-widget/client/index.jsjavascript/reactjs-todo-login-widget/client/utilities/request.jsjavascript/reactjs-todo-login-widget/package.jsonjavascript/reactjs-todo-login-widget/playwright.config.tsjavascript/reactjs-todo-login-widget/webpack.config.jsjavascript/reactjs-todo-oidc/README.md
vatsalparikh
force-pushed
the
migrate-login-widget-2.0
branch
from
July 18, 2026 00:42
7a74a8a to
e8d8d15
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
javascript/reactjs-todo-login-widget/client/utilities/request.js (1)
34-38: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle the token error before using
response.
tokenEvents.get()can resolve with anerrorplus a nullresponse, so destructuringresponsefirst drops the actual failure and falls back to the generic access-token message. Inspect the returned error object instead of immediately extractingresponse.💡 Proposed fix to handle token retrieval errors
if (DEBUGGER) debugger; const tokenEvents = user.tokens(); - const { response: tokens } = await tokenEvents.get(); - if (!tokens?.accessToken) { - throw new Error('Failed to retrieve access token'); + const result = await tokenEvents.get(); + + if (result.error) { + throw new Error(result.error.message || 'Failed to retrieve access token'); + } + + const tokens = result.response; + if (!tokens?.accessToken) { + throw new Error('Failed to retrieve access token'); }🤖 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 `@javascript/reactjs-todo-login-widget/client/utilities/request.js` around lines 34 - 38, Update the token retrieval flow around tokenEvents.get() to capture and inspect the full result before extracting response. Handle and propagate the returned error when present, then validate accessToken and retain the existing generic failure for missing tokens.Source: Learnings
🤖 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.
Duplicate comments:
In `@javascript/reactjs-todo-login-widget/client/utilities/request.js`:
- Around line 34-38: Update the token retrieval flow around tokenEvents.get() to
capture and inspect the full result before extracting response. Handle and
propagate the returned error when present, then validate accessToken and retain
the existing generic failure for missing tokens.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aa8f6d9b-cc89-42a4-adae-9a5e01c39a17
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
javascript/reactjs-todo-davinci/README.mdjavascript/reactjs-todo-journey/README.mdjavascript/reactjs-todo-login-widget/README.mdjavascript/reactjs-todo-login-widget/client/constants.jsjavascript/reactjs-todo-login-widget/client/hooks/useLoginWidget.jsjavascript/reactjs-todo-login-widget/client/index.jsjavascript/reactjs-todo-login-widget/client/utilities/request.jsjavascript/reactjs-todo-login-widget/package.jsonjavascript/reactjs-todo-login-widget/webpack.config.jsjavascript/reactjs-todo-oidc/README.md
🚧 Files skipped from review as they are similar to previous changes (6)
- javascript/reactjs-todo-login-widget/package.json
- javascript/reactjs-todo-oidc/README.md
- javascript/reactjs-todo-login-widget/README.md
- javascript/reactjs-todo-login-widget/webpack.config.js
- javascript/reactjs-todo-login-widget/client/constants.js
- javascript/reactjs-todo-journey/README.md
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 `@javascript/reactjs-todo-login-widget/client/hooks/useLoginWidget.js`:
- Around line 146-153: Update the waitForMount polling in useLoginWidget to
store its animation-frame ID, cancel any existing frame before starting a new
polling loop, and cancel the stored frame during widget cleanup. Ensure
unmounting and repeated early calls cannot leave parallel or indefinitely
scheduled polling loops.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 35a6dc4a-c4c5-409c-8eca-7177b5e3d94d
📒 Files selected for processing (2)
javascript/reactjs-todo-login-widget/client/context/auth.context.jsjavascript/reactjs-todo-login-widget/client/hooks/useLoginWidget.js
Comment on lines
+146
to
+153
| const waitForMount = () => { | ||
| if (widgetMountedRef.current) { | ||
| componentEvents.open(); | ||
| } else { | ||
| requestAnimationFrame(waitForMount); | ||
| } | ||
| }; | ||
| requestAnimationFrame(waitForMount); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Cancel readiness polling during cleanup.
If the component unmounts while waiting, widgetMountedRef remains false and waitForMount schedules animation frames forever. Repeated early calls also create parallel polling loops. Store the frame ID, cancel the previous poll before starting another, and cancel it in the widget cleanup.
Proposed fix
const widgetMountedRef = useRef(false);
+const openFrameRef = useRef(null);
return () => {
widgetMountedRef.current = false;
+ if (openFrameRef.current !== null) {
+ cancelAnimationFrame(openFrameRef.current);
+ openFrameRef.current = null;
+ }
widget.$destroy();
};
+if (openFrameRef.current !== null) {
+ cancelAnimationFrame(openFrameRef.current);
+}
+
const waitForMount = () => {
if (widgetMountedRef.current) {
+ openFrameRef.current = null;
componentEvents.open();
} else {
- requestAnimationFrame(waitForMount);
+ openFrameRef.current = requestAnimationFrame(waitForMount);
}
};
-requestAnimationFrame(waitForMount);
+openFrameRef.current = requestAnimationFrame(waitForMount);🤖 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 `@javascript/reactjs-todo-login-widget/client/hooks/useLoginWidget.js` around
lines 146 - 153, Update the waitForMount polling in useLoginWidget to store its
animation-frame ID, cancel any existing frame before starting a new polling
loop, and cancel the stored frame during widget cleanup. Ensure unmounting and
repeated early calls cannot leave parallel or indefinitely scheduled polling
loops.
vatsalparikh
force-pushed
the
migrate-login-widget-2.0
branch
4 times, most recently
from
July 18, 2026 06:28
557a4cf to
cdb4e6b
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
javascript/reactjs-todo-login-widget/client/context/auth.context.js (1)
51-55: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winReturn the subscription cleanup function instead of invoking it immediately.
Immediately invoking
unsubscribe()cancels the subscription right after creation. If the underlyingsubscribeimplementation is (or ever becomes) asynchronous, the callback may never execute, leading to an incorrect, unauthenticated state.Additionally, returning the cleanup function is the standard React pattern for
useEffectsubscriptions. This prevents race conditions, cleans up on unmount, and keeps your React context automatically in sync with any future token state changes (e.g., token expiration or logout).♻️ Proposed refactor
const unsubscribe = user.tokens().subscribe((tokenState) => { const isAuthenticated = !!tokenState?.response?.accessToken; setAuthentication(isAuthenticated); }); - unsubscribe(); + return unsubscribe;🤖 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 `@javascript/reactjs-todo-login-widget/client/context/auth.context.js` around lines 51 - 55, Update the subscription setup around user.tokens().subscribe in the authentication effect to return the unsubscribe cleanup function instead of invoking it immediately. Preserve the token callback’s setAuthentication behavior and ensure React performs cleanup on unmount while continuing to receive future token state changes.
🤖 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.
Nitpick comments:
In `@javascript/reactjs-todo-login-widget/client/context/auth.context.js`:
- Around line 51-55: Update the subscription setup around
user.tokens().subscribe in the authentication effect to return the unsubscribe
cleanup function instead of invoking it immediately. Preserve the token
callback’s setAuthentication behavior and ensure React performs cleanup on
unmount while continuing to receive future token state changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 056d9b9c-ba45-4e1d-b02d-30708893191f
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
.github/workflows/CI.ymljavascript/reactjs-todo-davinci/README.mdjavascript/reactjs-todo-journey/README.mdjavascript/reactjs-todo-login-widget/README.mdjavascript/reactjs-todo-login-widget/client/constants.jsjavascript/reactjs-todo-login-widget/client/context/auth.context.jsjavascript/reactjs-todo-login-widget/client/hooks/useLoginWidget.jsjavascript/reactjs-todo-login-widget/client/index.jsjavascript/reactjs-todo-login-widget/client/utilities/request.jsjavascript/reactjs-todo-login-widget/package.jsonjavascript/reactjs-todo-login-widget/webpack.config.jsjavascript/reactjs-todo-oidc/README.md
🚧 Files skipped from review as they are similar to previous changes (10)
- .github/workflows/CI.yml
- javascript/reactjs-todo-journey/README.md
- javascript/reactjs-todo-login-widget/client/constants.js
- javascript/reactjs-todo-login-widget/package.json
- javascript/reactjs-todo-login-widget/client/utilities/request.js
- javascript/reactjs-todo-login-widget/webpack.config.js
- javascript/reactjs-todo-davinci/README.md
- javascript/reactjs-todo-oidc/README.md
- javascript/reactjs-todo-login-widget/README.md
- javascript/reactjs-todo-login-widget/client/index.js
vatsalparikh
force-pushed
the
migrate-login-widget-2.0
branch
5 times, most recently
from
July 18, 2026 07:03
8d8d49e to
9010cbd
Compare
SteinGabriel
requested changes
Jul 20, 2026
vatsalparikh
force-pushed
the
migrate-login-widget-2.0
branch
from
July 20, 2026 23:23
9010cbd to
4d2fa2a
Compare
…ge on page reload for a logged in user
… into module scope
vatsalparikh
marked this pull request as draft
July 22, 2026 16:27
vatsalparikh
marked this pull request as ready for review
July 22, 2026 21:25
SteinGabriel
previously approved these changes
Jul 22, 2026
vatsalparikh
force-pushed
the
migrate-login-widget-2.0
branch
from
July 23, 2026 20:25
d9f5ede to
74da094
Compare
SteinGabriel
requested changes
Jul 24, 2026
vatsalparikh
force-pushed
the
migrate-login-widget-2.0
branch
2 times, most recently
from
July 24, 2026 19:30
b667dc0 to
8aa7215
Compare
vatsalparikh
force-pushed
the
migrate-login-widget-2.0
branch
from
July 24, 2026 19:32
8aa7215 to
6028b2d
Compare
cerebrl
reviewed
Jul 24, 2026
cerebrl
previously approved these changes
Jul 27, 2026
SteinGabriel
requested changes
Jul 27, 2026
vatsalparikh
force-pushed
the
migrate-login-widget-2.0
branch
2 times, most recently
from
July 27, 2026 17:06
b604224 to
753f52b
Compare
vatsalparikh
force-pushed
the
migrate-login-widget-2.0
branch
from
July 27, 2026 20:43
753f52b to
be867e3
Compare
SteinGabriel
approved these changes
Jul 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DO NOT MERGE: Pending Login Widget 2.0 release. Dependencies need to be updated to latest.
Explanation
Below are the changes needed after login widget's 2.0 release, mostly config changes
Summary by CodeRabbit
New Features
WELLKNOWN_URLconfiguration for the login widget.Authorization: Bearer …access token.Bug Fixes
Documentation
sdk-sample-appsrepo root.WELLKNOWN_URLto the login widget.env.example.Chores