fix(react-spa-bff): Enhance broadcaster with subpath handling #17212
Conversation
|
Warning Rate limit exceeded@snaerth has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 43 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThe changes in this pull request involve modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![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
🧹 Outside diff range and nitpick comments (1)
libs/react-spa/bff/src/lib/BffProvider.tsx (1)
Line range hint
48-81: Consider documenting the subpath-based event filtering patternThe implementation introduces a pattern for handling cross-window/tab communication with subpath filtering. Consider documenting this pattern in the component's documentation or README to help other developers understand and consistently implement similar functionality across the application.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
libs/react-spa/bff/src/lib/BffProvider.tsx(2 hunks)libs/react-spa/bff/src/lib/bff.hooks.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
libs/react-spa/bff/src/lib/bff.hooks.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/react-spa/bff/src/lib/BffProvider.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (3)
libs/react-spa/bff/src/lib/bff.hooks.ts (1)
67-67: LGTM! Type definitions properly enhanced for subpath handling
The addition of the subpath: string property to both NewSessionEvent and LogoutEvent types aligns well with the PR objectives. This change enables proper subpath-based event filtering across different application contexts.
Also applies to: 72-72
libs/react-spa/bff/src/lib/BffProvider.tsx (2)
48-68: LGTM! Well-documented subpath filtering implementation
The implementation correctly handles subpath-based event filtering with clear comments explaining the cross-origin broadcasting behavior. The timeout for logout handling is properly justified.
78-78: LGTM! Proper effect dependency management
The addition of subpath to the broadcast message and applicationBasePath to the effect dependencies ensures consistent subpath-based event handling across the application.
Also applies to: 81-81
Datadog ReportAll test runs ✅ 98 Total Test Services: 0 Failed, 90 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (4) |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
libs/react-spa/bff/src/lib/BffPoller.tsx (1)
Line range hint
13-15: Document the subpath handling behavior.Consider enhancing the component's documentation to explain the subpath handling behavior and its impact on session management across different application paths.
Add to the JSDoc comment:
* Features: * - Polls the backend at a specified interval to fetch user session data. * - If the user's session expires or the backend returns an error, it * automatically triggers a sign-in process. * - If a change in user session (e.g., a new session ID) is detected, it * broadcasts a message to all open tabs/windows and triggers the provided * `newSessionCb` callback to handle the current tab/window. +* - Includes application base path in session events to support multiple +* applications running on different paths. * * @param newSessionCb - Callback function to be called when a new session is detected. * @param pollIntervalMS - Polling interval in milliseconds. Default is 10000ms.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
libs/react-spa/bff/src/lib/BffPoller.tsx(1 hunks)libs/react-spa/bff/src/lib/BffProvider.tsx(3 hunks)libs/react-spa/bff/src/lib/bff.hooks.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/react-spa/bff/src/lib/bff.hooks.ts
- libs/react-spa/bff/src/lib/BffProvider.tsx
🧰 Additional context used
📓 Path-based instructions (1)
libs/react-spa/bff/src/lib/BffPoller.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (2)
libs/react-spa/bff/src/lib/BffPoller.tsx (2)
95-103: LGTM! Proper dependency tracking for React hooks.
The addition of bffUrlGenerator to the dependency array is correct as it's used within the effect and ensures proper reactivity when the URL generator changes.
89-89: LGTM! Verify path consistency across different environments.
The addition of bffBasePath aligns with the PR objectives and enables proper subpath handling for session events.
Let's verify the path generation consistency:
Add subpath to NewSessionEvent and LogoutEvent types. Update BffProvider to include applicationBasePath in postMessage dependencies. Modify event handling to only act on events matching the current subpath, ensuring proper session management across multiple tabs/windows/iframes.
Add bffBasePath to broadcast messages for logout and new session events. Update event handling to match against bffBasePath instead of applicationBasePath. This ensures consistent behavior across different components and improves clarity in the event broadcasting mechanism.
Update BffProvider to use a consistent BFF base path variable. This change improves clarity and ensures that broadcast events are filtered correctly by matching the BFF base path, preventing unintended interactions with other applications on the same domain. Adjust dependencies in useEffect hooks to reflect the new variable.
Update the BffPoller component to use bffBasePath directly in the dependency array of the useEffect hook. This change improves readability and ensures that the effect correctly responds to changes in theffBasePath variable
Update variable names from `bffBasePath` to `bffBaseUrl` to enhance clarity and consistency across the codebase. This change improves the understanding of the code explicitly indicating that the variable represents a base URL rather than a path. Adjust related comments and event types to reflect this change.
snaerth
force-pushed
the
fix-bff-broadcaster-subpath
branch
from
92702c0 to
0565f06
Compare
Enhance broadcaster with bffBasePath handling
What
Add subpath to NewSessionEvent and LogoutEvent types. Update BffProvider to include applicationBasePath in postMessage dependencies. Modify event handling to only act on events matching the current subpath,
ensuring proper session management across multiple tabs/windows/iframes.
Checklist:
Summary by CodeRabbit