Skip to content

refactor(web): decouple repositories and services from QueryClient#1163

Open
pmilic021 wants to merge 1 commit into
masterfrom
sdk/decouple-query-client
Open

refactor(web): decouple repositories and services from QueryClient#1163
pmilic021 wants to merge 1 commit into
masterfrom
sdk/decouple-query-client

Conversation

@pmilic021

Copy link
Copy Markdown
Contributor

Repositories and services no longer take a TanStack QueryClient; they run their underlying async actions directly and return Promises. This is a prerequisite for the stateless @agicash/wallet-sdk extraction, where the SDK holds no read cache and the web keeps TanStack Query.

The Breez wallet connection is the one piece that must still be held: connect() opens a stateful, resource-owning session (disconnect/free, event listeners, background loops, a storageDir DB), so there must be exactly one instance per wallet, not a fresh one per account load. It moves to a process-wide singleton (getSparkWallet, keyed by mnemonic/network/storageDir), cleared on signOut alongside queryClient.clear().

Everything else is a plain read fetched directly:

  • Mint metadata (info/keysets/keys): getInitializedCashuWallet and account-service call the mint over HTTP directly (idempotent, nothing to leak). The web's mint *QueryOptions are untouched and still cache those reads for the settings form / token decode.
  • Accounts / exchange rate: claim-cashu-token-service reads via accountRepository.getAllActive and ExchangeRateService directly.

claim-cashu-token-service no longer writes to the cache either: it returns the changed accounts + updated user, and the token route's clientLoader applies them.

Behavior note: the account-load path (getInitializedCashuWallet via toAccount) no longer shares the QueryClient mint cache, so it refetches mint metadata on each load; only the web's own mint reads stay cached. typecheck + biome clean; web/bolt11/ecies/cashu unit tests pass.

Repositories and services no longer take a TanStack QueryClient; they run their
underlying async actions directly and return Promises. This is a prerequisite
for the stateless @agicash/wallet-sdk extraction, where the SDK holds no read
cache and the web keeps TanStack Query.

The Breez wallet connection is the one piece that must still be held: connect()
opens a stateful, resource-owning session (disconnect/free, event listeners,
background loops, a storageDir DB), so there must be exactly one instance per
wallet, not a fresh one per account load. It moves to a process-wide singleton
(getSparkWallet, keyed by mnemonic/network/storageDir), cleared on signOut
alongside queryClient.clear().

Everything else is a plain read fetched directly:
- Mint metadata (info/keysets/keys): getInitializedCashuWallet and account-service
  call the mint over HTTP directly (idempotent, nothing to leak). The web's mint
  *QueryOptions are untouched and still cache those reads for the settings form /
  token decode.
- Accounts / exchange rate: claim-cashu-token-service reads via
  accountRepository.getAllActive and ExchangeRateService directly.

claim-cashu-token-service no longer writes to the cache either: it returns the
changed accounts + updated user, and the token route's clientLoader applies them.

Behavior note: the account-load path (getInitializedCashuWallet via toAccount) no
longer shares the QueryClient mint cache, so it refetches mint metadata on each
load; only the web's own mint reads stay cached. typecheck + biome clean;
web/bolt11/ecies/cashu unit tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@pmilic021 pmilic021 requested a review from jbojcic1 June 26, 2026 18:17
@pmilic021 pmilic021 self-assigned this Jun 26, 2026
@vercel

vercel Bot commented Jun 26, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agicash Ready Ready Preview, Comment Jun 26, 2026 6:17pm

Request Review

@supabase

supabase Bot commented Jun 26, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project hrebgkfhjpkbxpztqqke because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

const { keysets } = await this.queryClient.fetchQuery(
allMintKeysetsQueryOptions(account.mintUrl),
);
const { keysets } = await new Mint(account.mintUrl).getKeySets();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mint details throughout this PR are left uncached, and if we see issues with it we can try optimize it then. Makes sense @jbojcic1 ?

queryClient.clear();
// The spark wallet connection memo is framework-free (not in the query
// cache), so clear it alongside so a next session reconnects fresh.
clearSparkWallets();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This call would be part of sdk-exposed clear method in the future (like .stop).

destinationAccount: Pick<Account, 'id' | 'purpose'>;
/**
* Accounts created or updated while claiming, for the caller to write into
* its cache (the service no longer owns one).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit but (the service no longer owns one) should be removed. comments shouldn't reflect old state of the code

const changedAccounts = new Map<string, Account>();
let updatedUser: User | null = null;

const accounts = await this.accountRepository.getAllActive(user.id);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would rather pass accounts as a new param. that way web won't be making one new db call

accountRepository: this.accountRepository,
}),
);
const changedAccounts = new Map<string, Account>();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this a map and not a simple array?

}

if (
receiveAccount.currency !== user.defaultCurrency ||

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wonder if we should take this out of this service now and do it before calling it in the loader? the whole purpose of this is that when user is receiving to the account that is not the default, we set this one to default for ui benefits (if user is joining for the first time and don't have a balance on regular default account it's better to make this one default). this is probably irrelevant for mcp

} else {
const exchangeRate = await getExchangeRate(
this.queryClient,
const exchangeRate = await new ExchangeRateService().getRate(

@jbojcic1 jbojcic1 Jun 27, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we have one additional network request here right? because it was reading from the cache before. i wonder if we should change the service to just take the rate as param or change ExchangeRateService to handle caching

]),
new Promise<never>((_, reject) => {
setTimeout(() => {
queryClient.cancelQueries({

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no way to cancel those?

}): Promise<BreezSdk> {
const key = `${computeSHA256(mnemonic)}:${network}:${storageDir}`;
const existing = sparkWalletPromises.get(key);
if (existing) return existing;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm not sure if we should even do this. is calling connect/disconnect multiple times "expensive"? remind me when would we even call it multiple times?

{ 'spark.network': network },
);
sparkWalletPromises.set(key, walletPromise);
void walletPromise.catch(() => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this void needed?

);
sparkWalletPromises.set(key, walletPromise);
void walletPromise.catch(() => {
if (sparkWalletPromises.get(key) === walletPromise) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this if check needed? can't you just do sparkWalletPromises.delete(key);?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants