Feature/react native subscriptions improvement #279
Conversation
vitorguima
requested review from
anicioalexandre,
deboracosilveira and
priscilladeroode
as code owners
|
|
Warning Rate limit exceeded@vitorguima has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 15 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 (1)
WalkthroughThis update introduces a new hook, Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant useAppStateSubscription
participant useMessagesListSubscription
App->>useMessagesListSubscription: Mounts component
useMessagesListSubscription->>useAppStateSubscription: Registers onAppResume callback
useAppStateSubscription-->>useMessagesListSubscription: Notifies on app resume
useMessagesListSubscription->>useMessagesListSubscription: Calls subscribe/unsubscribe as needed
App-->>useMessagesListSubscription: Unmounts component
useMessagesListSubscription->>useAppStateSubscription: Cleans up subscription
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
packages/utils/package.json (1)
4-4: Consider using semantic versioning correctly for the new feature.Adding a new hook (
useAppStateSubscription) is a backward-compatible feature addition that should warrant a minor version bump (4.1.0) rather than a patch version bump (4.0.1). Patch versions are typically reserved for bug fixes.packages/utils/CHANGELOG.md (1)
5-7: Semantic versioning inconsistency between changelog and version number.The changelog correctly categorizes the new hook as "Minor Changes", but the version number is 4.0.1 (patch) instead of 4.1.0 (minor). The categorization in the changelog is correct - adding new features should be minor version bumps.
Consider updating the version to 4.1.0 to match the "Minor Changes" categorization in the changelog.
packages/utils/hooks/useAppStateSubscription/index.ts (1)
22-22: Consider optimizing useEffect dependencies to prevent unnecessary re-subscriptions.The current implementation will recreate the subscription whenever
onAppResumechanges, which could happen frequently if the parent component doesn't memoize the callback. Consider usinguseCallbackin the consuming components or add a ref pattern to avoid unnecessary subscription churn.Example optimization using useRef:
export const useAppStateSubscription = (onAppResume: () => void) => { const appState = useRef<AppStateStatus>(AppState.currentState) + const onAppResumeRef = useRef(onAppResume) + + useEffect(() => { + onAppResumeRef.current = onAppResume + }) useEffect(() => { const handleAppStateChange = (nextAppState: AppStateStatus) => { if (appState.current.match(/inactive|background/) && nextAppState === 'active') { - onAppResume() + onAppResumeRef.current() } appState.current = nextAppState } const stateChangeSubscription = AppState.addEventListener('change', handleAppStateChange) return () => { stateChangeSubscription.remove() } - }, [onAppResume]) + }, [])packages/components/modules/messages/native/graphql/subscriptions/useMessagesListSubscription.tsx (2)
35-39: Well-structured subscription callback with minor improvement opportunity.The
subscribecallback is well-implemented with proper guard clauses and memory leak prevention. The logic correctly disposes existing subscriptions before creating new ones.Consider simplifying the dispose call since
Disposable.disposeshould always exist:- disposableRef.current?.dispose?.() + disposableRef.current?.dispose()
41-44: Clean unsubscribe implementation.The
unsubscribecallback is well-structured with appropriate dependencies and proper cleanup logic.Same minor improvement as in the subscribe function:
- disposableRef.current?.dispose?.() + disposableRef.current?.dispose()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
packages/authentication/CHANGELOG.md(1 hunks)packages/authentication/package.json(1 hunks)packages/components/CHANGELOG.md(1 hunks)packages/components/modules/messages/native/graphql/subscriptions/useMessagesListSubscription.tsx(3 hunks)packages/components/package.json(1 hunks)packages/design-system/CHANGELOG.md(1 hunks)packages/design-system/package.json(1 hunks)packages/graphql/CHANGELOG.md(1 hunks)packages/graphql/package.json(1 hunks)packages/provider/CHANGELOG.md(1 hunks)packages/provider/package.json(1 hunks)packages/utils/CHANGELOG.md(1 hunks)packages/utils/hooks/useAppStateSubscription/index.ts(1 hunks)packages/utils/package.json(1 hunks)packages/wagtail/CHANGELOG.md(1 hunks)packages/wagtail/package.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/components/modules/messages/native/graphql/subscriptions/useMessagesListSubscription.tsx (1)
packages/utils/hooks/useAppStateSubscription/index.ts (1)
useAppStateSubscription(7-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (13)
packages/utils/hooks/useAppStateSubscription/index.ts (1)
7-23: LGTM! Solid implementation with one optimization opportunity.The hook correctly implements app state monitoring for React Native with proper cleanup and accurate state transition detection.
packages/graphql/package.json (1)
4-4: LGTM! Appropriate patch version bump for dependency updates.The patch version increment is semantically correct for a dependency-only update with no API changes to this package.
packages/wagtail/package.json (1)
4-4: Version bump looks good – please confirm matching CHANGELOG entry.
versionwas incremented to1.0.35, which is fine sem-ver-wise.
Double-check thatpackages/wagtail/CHANGELOG.md(not shown here) contains a corresponding1.0.35section so consumers can track what changed.packages/authentication/package.json (1)
4-4: Patch version increment is correct.
5.0.1 → 5.0.2follows sem-ver for the dependency bump to utils. No further action needed if the CHANGELOG already reflects this.packages/components/package.json (1)
4-4: Minor patch bump acknowledged.
1.2.7 → 1.2.8aligns with the small hook refactor. Ensure the dist artefacts are rebuilt before publishing.packages/provider/CHANGELOG.md (1)
3-9: Semantic-version mismatch between CHANGELOG and actual change.A new heading
3.0.0is introduced, yet the notes list only a patch dependency bump.
Either:
- Rename the section to
2.0.15(or the next patch), or- If a breaking change really exists, bump
packages/provider/package.jsonto3.0.0.Publishing with mismatched versions will confuse consumers and automation.
packages/authentication/CHANGELOG.md (1)
3-9: Changelog entry looks goodPatch-level bump only updates a dependency – the versioning and wording are consistent.
packages/graphql/CHANGELOG.md (1)
3-10: Changelog entry consistentPatch bump with dependency updates only – no issues spotted.
packages/wagtail/CHANGELOG.md (1)
3-10: Changelog entry consistentNew major version (2.0.0) lists only dependency bumps; acceptable if other breaking changes occurred elsewhere in the repo. No structural issues detected.
Ensure Wagtail actually contains breaking changes; if not, consider minor/patch bump instead of major.
packages/components/CHANGELOG.md (1)
3-16: Changelog entry reads wellMinor version increment matches functional refactor; dependency bumps recorded.
packages/components/modules/messages/native/graphql/subscriptions/useMessagesListSubscription.tsx (3)
3-3: LGTM: Import statement is correct.The import for
useAppStateSubscriptionis properly structured and aligns with the package architecture described in the PR objectives.
46-53: Excellent refactoring of focus effect logic.The extraction of subscription logic into reusable callbacks improves code organization and maintainability while preserving the original behavior.
55-57: Perfect implementation of app state subscription enhancement.This addition directly addresses the PR objectives by ensuring subscriptions are refreshed when the app returns to the foreground. The integration with the existing
subscribecallback maintains consistency and simplicity.
vitorguima
force-pushed
the
feature/react-native-subscriptions-improvement
branch
2 times, most recently
from
32ac684 to
a65ad4c
Compare
vitorguima
force-pushed
the
feature/react-native-subscriptions-improvement
branch
from
63d52b9 to
1610978
Compare
vitorguima
force-pushed
the
feature/react-native-subscriptions-improvement
branch
2 times, most recently
from
9cb0032 to
1610978
Compare
|
This PR introduces two key changes to improve the reliability of GraphQL subscriptions when the app transitions between background and foreground states:
useAppStateSubscriptionAdded a reusable hook that listens to app state changes using React Native’s AppState API. Executes a provided callback (onAppResume) when the app returns to the foreground (e.g., after being backgrounded or locked).
Useful for re-subscribing to real-time data or refetching stale content.
Example:
useMessagesListSubscriptionHookEnhanced the subscription lifecycle by subscribing and unsubscribing based on the app's active/inactive state.
Ensures real-time updates are re-established automatically after app resumes from the background or device unlock.
Prevents missed events or stale subscriptions due to suspended WebSocket connections on iOS.
Summary by CodeRabbit
New Features
Improvements
Chores