refactor: replace recursion with promise sequence in useInvoiceable
#752
Conversation
This commit refactors `useInvoicable`. The hard-to-follow recursion was replaced by awaiting promises which resolve or reject when one step of our JIT invoice flow is done. Therefore, `onSubmit` is now fully agnostic of JIT invoices. The handler only returns when payment + action was successful or canceled - just like when a custodial zap was successful.
* move invoice cancel logic into hook where invoice is also created * fix missing invoice cancellation if user closes modal or goes back. * refactor promise logic: it makes more sense to wrap the payment promise with the modal promise than the other way around.
ekzyis
force-pushed
the
533-refactor-invoiceable
branch
from
ea3a6f9
to
ddd0f02
Compare
| @@ -33,6 +33,7 @@ export default function useModal () { | |||
| } | |||
| const previousModalContent = modalStack[modalStack.length - 1] | |||
| setModalStack(modalStack.slice(0, -1)) | |||
| modalOptions?.onClose?.() | |||
There was a problem hiding this comment.
This is required to also cancel HODL invoices in case the user clicks on <- and already paid:
components/invoice.js
Outdated
| await onRetry() | ||
| } catch (err) { | ||
| console.error('retry error:', err) | ||
| setRetryError(retryError + 1) |
There was a problem hiding this comment.
nit: if you're referencing prior state, you usually want to use the function form of setState:
setRetryError(retryError => retryError + 1)
Thank you :) Key takeaway from this PR for me: Don't ignore the eslint rule I spent hours debugging why my errors are not getting caught by the TL;DR If you use an async promise executor, you essentially create a detached promise with no error handlers attached iiuc. Some warnings really make sense. |
Commit description:
This was done while working on #749 since we will heavily rely on JIT invoices there and thus this was a good time to tackle this TODO:
I think this is much better now. There is no recursion anymore.
Keeping in draft since I want to sleep over this and do another review and test tomorrow.
update: this should be stable now. video