fix(auto-withdraw): eliminate stale closure in settings persistence - #411
Merged
Austinaminu2 merged 2 commits intoJul 26, 2026
Conversation
…pdateSettings read from the closed-over settings snapshot rather than the current state, meaning a settings change made between renders could be silently overwritten in localStorage when a withdrawal history entry was added shortly after. Fix: - Add settingsRef (useRef) that is kept in sync with state on every write, giving all callbacks a live view of settings without adding them to dependency arrays. - Rewrite updateSettings to use the functional setSettings((prev) => ...) form so it merges from the actual current state, not a stale closure; remove the settings dep from its useCallback array. - �ddWithdrawalHistory already used the functional updater correctly; add settingsRef.current = next so the ref stays in sync here too. - Rewrite calculateWithdrawAmount to read from settingsRef.current at call time instead of capturing settings in a closure; drop the [settings] dependency entirely so the callback is stable. Closes FlowwStar#276
brightfootlimited-collab
had a problem deploying
to
staging
July 26, 2026 15:28 — with
GitHub Actions
Failure
|
@brightfootlimited-collab 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
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.
Problem updateSettings (and by extension calculateWithdrawAmount) captured the settings state value from their outer closure. Because React state updates are asynchronous, a settings change made between renders could be silently overwritten in localStorage when �ddWithdrawalHistory was called shortly after — no error, no warning, user changes just vanished. Reported in #276. ## Root cause
s // Before — stale closure const updateSettings = useCallback((update) => { const next = { ...settings, ...update }; // 'settings' may be stale setSettings(next); saveSettings(stream.id, next); // writes the stale snapshot }, [stream, settings]);settings in the dep array means the callback is recreated on every render, but between a setState call and the next render the closed-over value is still the old one. ## Fix Three changes, all in hooks/use-auto-withdraw.ts: 1. Add settingsRef — a useRef kept in sync with state on every write. Gives all callbacks a live, synchronous view of settings without being a stale closure. 2. updateSettings → functional updaters const updateSettings = useCallback((update) => { setSettings((prev) => { const next = { ...prev, ...update }; // always current settingsRef.current = next; saveSettings(stream.id, next); return next; }); }, [stream]); // 'settings' dep removed3. calculateWithdrawAmount → reads settingsRef.current Drops the [settings] dep array entirely, making the callback stable and never stale. �ddWithdrawalHistory already used the functional updater correctly; settingsRef.current = next was added to keep the ref in sync. ## Testing - Change a setting (e.g. toggle strategy), then trigger a withdrawal history entry before the next render — localStorage now reflects the latest setting, not the snapshot from the previous render. - calculateWithdrawAmount uses the live strategy/threshold values at the moment the interval fires, not the value captured when the interval was set up. Closes #276