fix(hooks): stable load identity, single mount fetch and in-flight cancellation - #518
Open
Sam-Rytech wants to merge 1 commit into
Open
fix(hooks): stable load identity, single mount fetch and in-flight cancellation#518Sam-Rytech wants to merge 1 commit into
Sam-Rytech wants to merge 1 commit into
Conversation
…ncellation Closes Sorokit#442 `load` is already wrapped in useCallback and listed in the effect deps in both components, so ESLint react-hooks was already quiet. What was still unmet were the behaviours those deps are supposed to guarantee: ContractEventFeed - `live` was seeded from `pollInterval` at mount and never re-synced, so a feed mounted with polling off never started polling when the prop turned on. `pollInterval` is now synced during render, mirroring the existing `prevContractId` pattern. - Added a generation counter to `load`. Every call claims a generation and the counter is bumped when `contractId` changes, so a response for the previous contract is discarded instead of writing its events - or its loading state - over the current feed. - The polling effect now only owns the timer: it clears and re-arms on `pollInterval` changes, and nulls `intervalRef` on teardown. - Dropped the duplicate `setEvents([])` mount effect and its react-hooks/set-state-in-effect suppression; the render-phase reset above it already clears the previous contract's events. FeeEstimator - `onFeeLoad` is usually an inline arrow, so a new identity on every parent render rebuilt `load`, re-ran the effect and fired another request per render (and the callback typically sets parent state, so it fed itself). It is held in a ref now and `load` depends only on `client`, so mount performs exactly one request while the newest callback is still the one invoked. - Same generation guard, so a slow estimate cannot overwrite fresher data. Tests: both suites still mocked the removed `getClient` entrypoint (the client moved to SorokitContext), so they were red on main - a small `useSorokit` shim routes the hook at the same mock every existing test configures, and three export-button queries are matched against the current aria-label. New cases cover single fetch on mount, runtime pollInterval changes, timer re-arming, stale-response rejection and the inline-callback refetch loop. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@Sam-Rytech 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! 🚀 |
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
1 participant
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 #442
loadis already wrapped in useCallback and listed in the effect deps in both components, so ESLint react-hooks was already quiet. What was still unmet were the behaviours those deps are supposed to guarantee:ContractEventFeed
livewas seeded frompollIntervalat mount and never re-synced, so a feed mounted with polling off never started polling when the prop turned on.pollIntervalis now synced during render, mirroring the existingprevContractIdpattern.load. Every call claims a generation and the counter is bumped whencontractIdchanges, so a response for the previous contract is discarded instead of writing its events - or its loading state - over the current feed.pollIntervalchanges, and nullsintervalRefon teardown.setEvents([])mount effect and its react-hooks/set-state-in-effect suppression; the render-phase reset above it already clears the previous contract's events.FeeEstimator
onFeeLoadis usually an inline arrow, so a new identity on every parent render rebuiltload, re-ran the effect and fired another request per render (and the callback typically sets parent state, so it fed itself). It is held in a ref now andloaddepends only onclient, so mount performs exactly one request while the newest callback is still the one invoked.Tests: both suites still mocked the removed
getCliententrypoint (the client moved to SorokitContext), so they were red on main - a smalluseSorokitshim routes the hook at the same mock every existing test configures, and three export-button queries are matched against the current aria-label. New cases cover single fetch on mount, runtime pollInterval changes, timer re-arming, stale-response rejection and the inline-callback refetch loop.