add useSignalEffectWithAbortOnChange
#114
Conversation
| const setupInner = (optional: OptionalSignal<any>) => { | ||
| const inner = optional.peek() | ||
| if (inner !== undefined) subscribeTo(inner, run) | ||
| } |
There was a problem hiding this comment.
We also need to unsubscribe to the inner if the outer signal value changes from defined to undefined.
Also, no any.
| const setupInner = (optional: OptionalSignal<any>) => { | |
| const inner = optional.peek() | |
| if (inner !== undefined) subscribeTo(inner, run) | |
| } | |
| const setupInner = (optional: OptionalSignal<unknown>) => { | |
| const inner = optional.peek() | |
| if (inner === undefined) /*TODO: unsubscribe inner*/ | |
| else subscribeTo(inner, run) | |
| } |
There was a problem hiding this comment.
the unsubs are done at the end:
return () => {
cleanupFns.forEach(fn => fn())
abortController.abort()
}
```There was a problem hiding this comment.
That is too late. You need to unsubscribe from the inner signal anytime the outer signal changes to undefined. You'll then resubscribe to the inner signal when the outer signal changes back to defined. You already have the resubscribe setup correctly, but without the unsubscribe you'll end up subscribing many times. You'll also incorrectly receive update notifications when nothing has changed.
| daiApprovedForRouter.deepValue = await getAllowanceErc20Token(maybeWriteClient, DAI_TOKEN_ADDRESS, maybeWriteClient.account.address, router) | ||
| sharesApprovedToRouter.deepValue = await isErc1155ApprovedForAll(maybeWriteClient, AUGUR_SHARE_TOKEN, maybeWriteClient.account.address, router) | ||
| } | ||
| useSignalEffectWithAbortOnChange([maybeWriteClient] as const, (_abortSignal, ...params) => updateAccountSpecificSignals(...params), console.error) |
There was a problem hiding this comment.
You'll start using the abort later I assume? Without using the abort, I'm not sure we gain much by switching.
Co-authored-by: Micah Zoltu <[email protected]>
No description provided.