Optimistic updates via pending sats in item context

[ekzyis]

Description

Since we removed optimistic updates in #1220 because the implementation broke replies, there is no immediate visual feedback if you zap from a non-custodial wallet. This is bad UX since it looks like zaps don't work since nothing happens. However, the payment is just pending.

This PR fixes this by implementing optimistic updates via a new context ItemContext (maybe not the best name). This context keeps track of pending sats. Since it uses the Context API, this is completely separate of the Apollo cache and thus does not interfere with any mutation. All this does is to add pending sats to the item context, remove them after the mutation (if it failed or not) and hook into the context in the components that need to be aware of pending sats.

To update Item.commentSats for all ancestors on a zap, setPendingSats also calls setPendingCommentSats of the parent ItemContext context. This is done recursively up to the root item.

This is just meant as a temporary fix until #1195 is released.

TODO:

• [x] also include pending sats in Item.commentSats
• [x] optimistic update for poll votes
• [x] ~optimistic update for bounty payments~ abandoned because would require too many changes for a temporary fix

Checklist

Are your changes backwards compatible? Please answer below:

Yes

Did you QA this? Could we deploy this straight to production? Please answer below:

Yes

For frontend changes: Tested on mobile? Please answer below:

Did you introduce any new environment variables? If so, call them out explicitly here:

[huumn]

As I review, and I'm just curious because it would've been my first approach, did you try using read/writeFragment for this?

[ekzyis]

As I review, and I'm just curious because it would've been my first approach, did you try using read/writeFragment for this?

Mhh, I think it was one of my first approaches when I started implementing optimistic UX (so not in this PR but like weeks ago) but I must have had some problems with (undoing) it

[huumn]

This works well. The only thing I'm noticing in QA is that for a brief moment, it appears as if I zapped double.

e.g. If I zap 100 sats, it'll briefly show 200 sats, then 100.

I'll see if it's easy to fix, but it isn't a huge deal. I still need to QA attached wallets too.

[huumn]

Ah, I think this happens because useZap both does your optimistic context AND the optimisticResponse. I don't think it needs both anymore. In prod, where there's more latency, this would probably be more noticeable and pretty unsettling (and it affects not just attached wallet zaps but all of them).

[ekzyis]

Ah, I think this happens because useZap both does your optimistic context AND the optimisticResponse. I don't think it needs both anymore. In prod, where there's more latency, this would probably be more noticeable and pretty unsettling (and it affects not just attached wallet zaps but all of them).

Ah yes, the optimistic context is undone in the finally branch. We could remove the optimistic response and only run revert during errors.

This would change the semantics of the item context though but we will remove this code anyway[^1] so ...

[^1]: and I didn't realize it's going to be more noticeable in production

[huumn]

I committed my changes to remove the doubling. Please QA it again yourself. Then I can deploy tomorrow if that works for you.

[ekzyis]

LGTM.

Regarding c4b52dd: I created a PR where one can disable --max-amount and --daily-limit: https://github.com/benthecarman/nostr-wallet-connect-lnd/pull/4. I forgot to use this flag in our NWC container.