Skip to content

Cleanup - #318

Open
RaceeyXo wants to merge 13 commits into
devfrom
cleanup
Open

Cleanup#318
RaceeyXo wants to merge 13 commits into
devfrom
cleanup

Conversation

@RaceeyXo

@RaceeyXo RaceeyXo commented Sep 8, 2026

Copy link
Copy Markdown
Owner

No description provided.

Certifieddonnie and others added 13 commits September 5, 2026 18:08
…the docs

Works through the solutions the closed issue templates describe, and fixes what
verifying them surfaced. The unit suite went from 57 failures across 15 suites to
449 passing across 35; lint, typecheck, build, size budget and the package smoke
test are all green.

Correctness
- useSendPayment and useAddTrustline built their Horizon client from the network
  *name* rather than the resolved config, so a custom horizonUrl was ignored on
  exactly the two paths that move value. Every other hook already passed the
  config (bug-04).
- StellarProvider rebuilt its context value on every render, re-rendering every
  consumer in the tree regardless of what changed. Memoized, along with the
  autoConnect options it derives (bug-02).
- toSubmissionError re-tested Horizon result codes with a stale copy of the table
  fromResultCodes already owns, so tx_bad_seq and tx_insufficient_fee were named
  on the rejection path but flattened to TRANSACTION_FAILED when Horizon answered
  200-with-failure. It now delegates to the one table.
- useSorobanContract re-wrapped its error on every render, handing consumers a new
  object each time and re-firing any useEffect(..., [error]) indefinitely. Its
  cache key now also distinguishes spec-aware from raw calls, which decode through
  different paths and must not share an entry.
- A Soroban argument outside Number.MAX_SAFE_INTEGER is refused with a message
  naming the precision loss rather than the generic width question.
- freighterAdapter imports the package's default export. @stellar/freighter-api is
  a minified CommonJS bundle whose named exports Node's ESM loader cannot detect,
  so `import { WatchWalletChanges }` threw "Named export not found" at load time
  for every ESM consumer. The bundled build had hidden this.

Test infrastructure (test-03, test-04)
- The SDK double now re-exports the real, deterministic SDK — Asset, Operation,
  Memo, Networks, BASE_FEE, StrKey, Keypair, TransactionBuilder — and fakes only
  Horizon.Server and SorobanRpc.Server, through per-test factories rather than a
  shared mutable singleton. A full payment now builds, signs and round-trips its
  XDR through the real encoder, which was impossible before.
- Six fixture addresses were not valid strkey; the real SDK rejects them. Replaced
  with freshly generated testnet keys throughout src and docs.
- jest.config.js dropped resetMocks/restoreMocks, which stripped the module-scope
  mock implementations each file defines once.
- useBalance.test.tsx compiled to the same output name as useBalance.test.ts, so
  TypeScript skipped emitting one of them; renamed to useBalance.watch.test.tsx.
- useAnchor's SSR test deleted global.window before renderHook, which react-dom
  cannot survive, and the leaked global failed the next three tests. It mocks
  isBrowser instead, as ssr-guard.test.tsx already did.
- useSendPayment.504's jest.mock sat inside describe(), where it ran after the
  file's imports had already resolved and therefore never applied.
- Refetch tests asserted the pre-state-02 behaviour of blanking data on a failed
  refresh; they now assert stale-while-revalidate. A failed *first* fetch still
  yields null, and that distinction is asserted separately.

Packaging (pkg-01, pkg-02, pkg-04)
- exports declares separate types for the import and require conditions, so ESM
  consumers resolve index.d.mts instead of a CommonJS declaration file, and
  ./package.json is exported.
- The wallet SDKs are external instead of inlined; they were shipped twice, once
  bundled and once installed, which stopped consumers deduping or overriding them.
- Dropped @lobstrco/signer-extension-api: imported nowhere, backing an explicitly
  unsupported adapter, downloaded by everyone. Dropped the root's duplicate
  @albedo-link/intent.
- src is published so the shipped source maps resolve; target and engines.node
  state one support floor. treeshake stays off deliberately — its rollup pass
  strips the "use client" banner the package cannot ship without.
- The smoke test now typechecks under bundler and node16 as well as legacy node;
  only the modern algorithms consult the exports map at all.

CI (ci-07, test-02)
- Restored --frozen-lockfile in the test job and the release workflow.
- The integration workflow filtered on a package name that does not exist, so pnpm
  matched nothing and exited 0 — a green check certifying that no test ran. It
  also ordered setup-node before pnpm/action-setup, so its cache could not work.

Repo and docs (repo-01)
- Untracked the committed junk: a file named "et --hard HEAD@{5}", dev-server
  logs, a misspelled CLUADE.MD, one-off notes, personal scripts, a tsup temp
  artifact, and the raw Figma dumps. Rewrote .gitignore to cover them plus the
  smoke test's leftovers.
- Both READMEs list all 20 exported hooks (10 were undocumented), the full
  provider prop set, the cache and its stale-while-revalidate contract, and the
  fee strategy. CHANGELOG's Unreleased section moved to the top and its
  LEDGER_OUT_OF_RELENTION typo corrected to match the exported code.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DBQmbmwJJUgb5MMtdsiyfW
A Windows checkout with core.autocrlf=true rewrote the working tree to CRLF,
so git reported ~90 files as modified when only a handful had real changes.
`* text=auto eol=lf` normalizes the repository so the next contributor's diff
shows what they actually changed.

Also marks binaries so git never rewrites them, and pnpm-lock.yaml as generated
so it stays out of diffs and language stats.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DBQmbmwJJUgb5MMtdsiyfW
The merge wave that landed on `dev` resolved conflicts by concatenating both
sides. Nine files ended up holding two or three implementations at once and no
longer parsed at all, so `tsc`, `prettier` and `jest` each failed before they
could report anything useful.

Repaired, keeping the newest architecture in each case:

- types/index.ts — two interfaces never closed before the next declaration, the
  Asset type and a doc comment were duplicated (orphaning a `*/`), plus a stray
  brace and a leftover `refetch` fragment.
- errors/codes.ts — a stray `o` prefixed to SEP10_VALIDATION_FAILED and a
  duplicate closing brace.
- useOffers.ts — an entire dead useState implementation, importing from a
  `../providers/StellarProvider` path that does not exist, sat above the current
  one. Restored to b0fe56f, which is byte-identical to the surviving half.
- useAccount.ts / useClaimableBalance.ts — the old useState half was spliced
  through the middle of the useQuery one. Rebuilt from the live halves. Both
  declare `isStale` in their return type but the surviving code had dropped it,
  so it is reinstated from the documented stale-while-revalidate contract.
- usePayments.ts / useTransactionHistory.ts — three parallel branches
  (reducer pagination a38f351, the limit+1 hasNext heuristic 8339135, and 429
  backoff 581b65b) were never actually integrated; no single revision ever
  contained all three. Took the reducer architecture as the base and ported the
  limit+1 heuristic and `maxRetries` onto it. limit+1 matters because
  `records.length >= limit` reports hasNext incorrectly whenever the total is an
  exact multiple of the page size.
- usePayments.test.tsx — restored to the revision matching that architecture.
- __mocks__/@stellar/stellar-sdk.ts — two independent solutions to the same
  test-04 issue had been concatenated. Kept 25b8f66, the superset: it exports
  everything the other did except TESTNET_ISSUER/resetMockServers, which nothing
  references.

This commit only restores parseability. 223 type errors of the same origin
remain across 53 files and are not addressed here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DBQmbmwJJUgb5MMtdsiyfW
StellarNetwork had reverted to a two-network union while NETWORK_PASSPHRASES,
NETWORK_CONFIGS and getNetworkPassphrase all still assumed futurenet and custom.
StellarContextValue had lost queryStore and autoConnect, which alone accounted
for 66 'Property does not exist' errors across the hooks. AutoConnectOptions had
gone entirely, and fourteen exported types (Anchor*, ContractEvents*, Payment
paths, path payment, WalletNetworkId) were missing while src/index.ts still
re-exported them.

Also deduplicated UseOffersReturn/UseOffersOptions and UseManageOfferReturn,
which two branches had each declared with incompatible shapes; kept whichever
matches the surviving implementation in each case. Corrected two import paths in
the barrel (useManageOffer -> useManagerOffer, useOrderbook -> useOrderBook,
the latter a casing collision that breaks case-sensitive filesystems).

useBalance.ts had the old useState implementation spliced through the middle of
the useQuery one, including two 'refetch' keys in its returned object.

223 -> 122 type errors.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DBQmbmwJJUgb5MMtdsiyfW
…r existed

useLiquidityPool, useLiquidityPoolActions and useManagerOffer all imported
`useStellar` from `../providers/StellarProvider` — a module that does not exist
in this repository — and destructured `{ server, adapter, publicKey }` from it.
They were merged in without ever compiling.

Rewired to the real `useStellarContext`, deriving the Horizon client from
`networkConfig` and the signer from `getWalletAdapter(wallet.wallet)`. The two
writing hooks now follow the same build/sign/submit path as useAddTrustline:
the adapter signs an XDR string rather than an operation object, and the fee is
bid from the network's current base fee.

Also fixed along the way:
- useLiquidityPool had the literal token `ts` pasted into its finally block.
- manageBuyOffer takes `buyAmount`, not `amount`; the two operations are not
  interchangeable and the shared param object silently produced the wrong one.
- Added the missing `isLiquidityPoolShares` guard to utils. It was imported but
  never existed; pool shares cannot be either side of an offer, so the paths
  that build operations need to reject them rather than fall through to the
  issued-asset branch.
- Restored networkPassphrase on NetworkConfig/CustomNetworkConfig and
  walletNetworkPassphrase on WalletState, and retyped walletNetwork as
  WalletNetworkId — the extension can report a network this library ships no
  config for.

122 -> 85 type errors.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DBQmbmwJJUgb5MMtdsiyfW
- SendPaymentOptions and AddTrustlineOptions lost `extends FeeOptions`, so the
  `fee`/`feeMultiplier` they document could not be passed to resolveFee.
- ContractCallOptions lost `spec` and `sourceAccount`, both of which
  useSorobanContract reads and its tests exercise.
- UsePaymentHistoryOptions/Return lost maxAccumulationPages and
  accumulationBoundHit, which the hook sets and its tests assert on.
- LOW_RESERVE had a default message but was never added to STELLAR_ERROR_CODES.
- getWalletAdapter was imported from ../utils in six files; it lives in
  ../wallets.
- useFriendBot.test.ts and useOrderBook.test.tsx imported their subjects with
  the wrong filename casing, which resolves on Windows and fails on Linux.
- Asset narrowing: several sites tested `!== "XLM"` and then read `.code`, but
  Asset also includes the pool-share pseudo-asset, so that never narrowed to
  IssuedAsset. They now use the isIssuedAsset guard, which also rejects pool
  shares from order books and payment filters, where they are not valid.
- The paginated hooks passed `toStellarError(err)` straight into a reducer
  action typed StellarError, but it returns null for an abort. An abort is a
  deliberate cancellation, so those paths now leave page state untouched
  instead of coercing null into an error.

85 -> 54 type errors.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DBQmbmwJJUgb5MMtdsiyfW
WalletType had been narrowed to a closed union, but registerWalletAdapter()
exists precisely so an application or wallet vendor can add its own; restored
the open form so custom adapters typecheck again.

SDK alignment (these hooks had never compiled against the real SDK):
- WalletAdapter.signTransaction takes (xdr, options); useSorobanWrite,
  useSep10Auth and useCreateAccount each called it with three positional args.
- SendTransactionResponse exposes errorResult, not errorResultXdr.
- GetTransactionStatus has no PENDING member. NOT_FOUND is how the RPC reports
  "not yet in a ledger", which is what the polling loop should test.
- submitTransaction's response carries no created_at.
- base_reserve_in_stroops is already a number.

useSendPayment only ever built text memos, so the MemoInput union added by
issue-202 could not actually be used. It now builds the right memo per type,
which matters on the wire: an exchange expecting an id memo will not credit a
text memo holding the same digits.

usePayments.test.tsx did not contain tests at all. The merge had written a copy
of useTransactionHistory.ts source to that path. Restored the real suite from
8339135, whose hasNext expectations match the limit+1 heuristic.

Removed src/hooks/__tests__/useOffers.test.tsx, a vitest-style duplicate that
used "vi" and imported a provider path that does not exist, of the working jest
test beside the hook. This project runs jest, so it could never have executed.

54 -> 0 type errors.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DBQmbmwJJUgb5MMtdsiyfW
…t strkeys

The merged mock called jest.requireActual("@stellar/stellar-sdk"). requireActual
bypasses a manual mock but NOT moduleNameMapper, and the mapper points that
specifier straight back at this file — so `actual` was the mock itself and every
re-export from it was undefined ("actual.Account is not a constructor", which
failed 36 suites before a single test ran). It now requires the real SDK by
relative file path, the only form that escapes both the mapper and the package's
`exports` map.

With the real SDK actually loaded, its strkey validation rejected seven fixture
addresses that were never valid Ed25519 public keys despite being named
TESTNET_ACCOUNT, TESTNET_SOURCE and TARGET. Replaced with freshly generated
testnet keys across 16 files. The deliberate GXXXX... invalid-address fixture is
left alone.

399 -> 543 tests discovered; 384 -> 501 passing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DBQmbmwJJUgb5MMtdsiyfW
…bypassed it

useFriendbot, useCreateAccount and useSep10Auth all threw plain Errors with
`err.name = "SOME_CODE"`. Their `error` state is typed StellarError, and the
README promises a stable `error.code`, but `name` on a StellarError is always
"StellarError" — so every one of those codes was unreachable to a consumer
branching on it. Converted 21 throws to createStellarError, and moved the
matching assertions from `.name` to `.code`.

WALLET_NOT_FOUND was not a code this library defines; the situation it named is
"this wallet has no registered adapter", which is WALLET_UNSUPPORTED.

Test doubles: getWalletAdapter moved to ../wallets, so the suites that drive it
now automock that module rather than ../utils; useTrades' utils mock replaced
the whole module and lost the pure asset guards the hook uses, so it spreads
requireActual; and the SDK double now re-exports SorobanDataBuilder and the
other pure builders a Soroban fixture needs.

518 -> 531 passing, typecheck still clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DBQmbmwJJUgb5MMtdsiyfW
…page on an empty result

useBalance and useAccount both key on accountKey but cached different shapes
under it, so whichever fetched first decided what the other read — the
cross-hook dedup the integration suite asserts could never have worked.
useBalance now caches the whole account and selects its balances, so one Horizon
request genuinely serves both.

useQuery left `loading: true` forever when a query became disabled mid-flight
(the usual cause being the wallet disconnecting and the address going null): the
effect returned early, so nothing was ever going to arrive and clear it. It now
projects the current key's snapshot on the disabled path.

The paginated hooks regained two behaviours the reducer rewrite had dropped:
- Landing on an empty page keeps the page already on screen and updates only the
  navigation state, rather than blanking the list.
- A superseded fetchNext can no longer install its results over a newer page. The
  reducer's queryKey check cannot catch this because a refetch does not change
  the key, so the hooks carry a monotonic request id and discard stale responses.

lastUpdated is memoized on its timestamp; a fresh Date each render looked like a
change to any consumer comparing it or using it as an effect dependency.

The three dedup integration tests each rendered their hooks in *separate*
renderHook calls, so every hook got its own provider and therefore its own
store — they could not have observed deduplication under any implementation.
They now share one provider, and the remount test toggles a child component
rather than tearing down the tree (which took the cache with it).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DBQmbmwJJUgb5MMtdsiyfW
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