fix(factories): refetch on profile switch + add activeProfileId to route deps#47
Merged
Merged
Conversation
…ute deps Switching the wallet's active node via NodePicker left the factories list empty when the user wasn't on /factories at switch time and then navigated there afterward. The route-effect in routerReduxSync also did not re-fire when activeProfileId changed (deps were only pathname / authStatus / navigate), so even a fresh navigate didn't always recover. Two defensive fixes: 1. NodePicker.handleSwitchNode now eagerly refetches ALL section data (CLN, BKPR, factories) on profile switch instead of only the section matching the current pathname. The fetches are async / fire-and-forget so switch UX is unaffected, but the next page the user navigates to has fresh data for the new profile. 2. routerReduxSync's route-fetch useEffect now includes activeProfileId in its deps. When the active node changes without a pathname change, the effect re-fires for the current pathname and refetches against the new node. Repro before this PR: switch the wallet dropdown from ss-demo-lsp to ss-demo-client (CLI factory-list on the client returns 3 factories) -- the wallet's /factories page renders 0. After this PR: the list re-populates on switch and on subsequent /factories navigation. Also removes the now-unused useLocation import in NodePicker (the pathname-conditional bgRefresh is gone).
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
Switching the wallet's active node via the NodePicker dropdown could leave the Factories list empty even though the new node's plugin had factories — surfaced during the post-#46 orchestrator audit (tracked as bug #157).
The audit reproduced it deterministically:
ss-demo-lspon/bookkeeperss-demo-client/factoriesMeanwhile CLI
factory-liston the client returned 3 factories (e2ebf4f0,d607,a823931a), and the Dashboard correctly showed 1 channel from the client POV (confirming backend rebinding worked).Root cause
Two cooperating bugs in the post-switch / route-data plumbing:
NodePicker.handleSwitchNode(apps/frontend/src/components/ui/NodePicker/NodePicker.tsx:63-73)The background refresh after a profile switch fetched only one section's data, chosen by the current
pathname:Switch profile on
/bookkeeper→ only BKPR refetched, factories left empty.routerReduxSync.tsx:118— the route-data effect's deps were[authStatus, pathname, navigate].activeProfileIdwas missing, so a profile change without a pathname change didn't re-fire the effect.Together: switching on one section and navigating to another sometimes didn't refresh — depending on timing — leaving stale or empty data.
Fix
NodePicker—handleSwitchNodeeagerly refetches CLN, BKPR and Factories in parallel on every profile switch, regardless of current pathname. Fire-and-forget so switch UX stays snappy, but downstream views always have data ready for the new profile.routerReduxSync— addsactiveProfileIdto the route-fetch effect deps + importsselectActiveProfileId. Now an active-node change re-runs the current route's data fetch against the new node.Also drops the unused
useLocationimport fromNodePicker(the pathname-conditionalbgRefreshis gone).Verification
CI build/eslint/test. Empirical verification on the demo wallet after deploy: switch dropdown LSP↔Client and the factories list re-populates with the correct per-node view.