fix: deduplicate event subscriptions across hooks (#833) - #849
fix: deduplicate event subscriptions across hooks (#833)#849Peolite001 wants to merge 16 commits into
Conversation
|
@Peolite001 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
Auto-review failed (API error). Leaving PR for human review. |
|
Auto-review failed (API error). Leaving PR for human review. |
|
Auto-review failed (API error). Leaving PR for human review. |
|
Auto-review failed (API error). Leaving PR for human review. |
|
Auto-review failed (API error). Leaving PR for human review. |
1 similar comment
|
Auto-review failed (API error). Leaving PR for human review. |
b9d5e05 to
8fa9e7a
Compare
|
Auto-review failed (API error). Leaving PR for human review. |
|
Auto-review failed (API error). Leaving PR for human review. |
|
Auto-review failed (API error). Leaving PR for human review. |
|
Auto-review failed (API error). Leaving PR for human review. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const parts = fullKey.split("."); | ||
| const key = parts[parts.length - 1]; | ||
| const namespace = parts.length > 1 ? parts[0] : ''; | ||
| const namespace = parts.length > 1 ? parts[0] : ""; |
|
Auto-review failed (API error). Leaving PR for human review. |
|
Auto-review failed (API error). Leaving PR for human review. |
| // Step 3: Check for withdrawal action button or navigate directly to withdraw tab | ||
| const withdrawBtn = page.getByRole("button", { name: /withdraw|claim/i }).or(page.locator("body")); | ||
| await expect(withdrawBtn).toBeVisible(); | ||
| const withdrawBtn = page.getByRole("button", { name: /withdraw|claim/i }).first(); |
|
Auto-review failed (API error). Leaving PR for human review. |
|
Auto-review failed (API error). Leaving PR for human review. |
|
Auto-review failed (API error). Leaving PR for human review. |
|
Auto-review failed (API error). Leaving PR for human review. |
|
Auto-review failed (API error). Leaving PR for human review. |
davidmaronio
left a comment
davidmaronio
left a comment
There was a problem hiding this comment.
routing useCampaignContributionEvents and useCampaignVoteEvents through the shared eventSubscriber is exactly what #833 asked for, and the lazy rpc.Server construction is a nice touch for tests. before it can merge:
- the diff spans 38 files, most of it unrelated churn: README/PRE_COMMIT_SETUP, jest config, markdown mocks, and pure formatting rewrites of gamification.ts, badges.ts, middleware.ts, etc. please rebase onto current main so the diff shrinks to the subscriber migration; right now the real change is impossible to review in isolation and the branch is in conflict with main.
- src/hooks/useCampaignContributionEvents.ts:48: the hook calls
eventSubscriber.start()but cleanup only doesoff(). does the subscriber stop its polling loop when the last handler is removed? if not, navigating away from campaign pages leaves a global rpc poll running forever; a ref-counted stop would close that. - note #705 touches the same hook with a different approach; whichever lands second needs coordination.
4 participants
Summary
This PR migrates
useCampaignContributionEventsanduseCampaignVoteEventsto use the sharedeventSubscriberinstance instead of maintaining their own individual polling loops.Previously, if multiple components observed the same campaign on a single page, each hook consumer instantiated a separate RPC polling interval. By routing them through
eventSubscriber, the app now shares a single underlying Soroban event polling stream for the contract across all hooks and deduplicates events efficiently.Closes #833
Type of Change
Contributor Checklist
CONTRIBUTING.mdfor branch, commit, and PR title conventions.eventSubscriber.ts)Validation
npm run lintnpm run format:checknpm run typechecknpm testnpm run buildjestnot found in local environment), but code was structurally verified. Please run standard CI pipelines against this branch.Notes for Reviewers
src/lib/eventSubscriber.tsexplicitly detailing that it satisfies [Performance] eventSubscriber.ts may open duplicate Soroban event subscriptions per hook consumer #833 by serving as the single underlying loop foruseContractEvents,useCampaignContributionEvents, anduseCampaignVoteEvents.