fix(hooks): guard campaign query keys against NaN and zero ids - #886
Open
Rafiat30 wants to merge 2 commits into
Open
fix(hooks): guard campaign query keys against NaN and zero ids#886Rafiat30 wants to merge 2 commits into
Rafiat30 wants to merge 2 commits into
Conversation
Replace the fragile !!campaignId && !isNaN(id) enabled checks in useContribution and useRevenueSharing with Number.isFinite(id), which correctly excludes NaN without also disabling the query for a falsy (zero) campaign id. Refs Iris-IV#553
|
@Rafiat30 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! 🚀 |
Contributor
|
Auto-review failed (API error). Leaving PR for human review. |
…ueSharing
Adds regression coverage for the enabled-guard fix in this branch: confirms
queries stay disabled for NaN campaign ids (e.g. parseInt("abc")), confirms
they still fire for a legitimate campaign id of 0 (the bug the previous
!!campaignId check introduced), and confirms string ids are parsed correctly
before querying.
Refs Iris-IV#553
Contributor
|
Auto-review failed (API error). Leaving PR for human review. |
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
2 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.
Closes #553
Summary
NaN query keys cause identity-check failures in React Query, leading to redundant cache entries and RPC calls.
useContributionanduseRevenueSharingalready guarded against NaN via!isNaN(id), but the accompanying!!campaignIdcheck is fragile: it evaluates tofalsefor a legitimate campaign id of0, disabling the query unnecessarily.Changes
src/hooks/useContribution.ts—enablednow uses!!userAddress && Number.isFinite(id).src/hooks/useRevenueSharing.ts—enablednow usesenabled && Number.isFinite(id).Number.isFinitecorrectly excludesNaN(andInfinity) while treating0as a valid, enabled id — a strict improvement over!!campaignId && !isNaN(id).How to test
npm run typechecknpm test -- useContribution useRevenueSharing/causes/abcid and confirm no repeated RPC calls fire in the network tab.