Skip to content

fix(react): build @streamfi/react again — 20 type errors, 40 failing tests - #1039

Open
arkhindi12 wants to merge 1 commit into
conduit-protocol:mainfrom
arkhindi12:fix/react-package-ci
Open

arkhindi12 wants to merge 1 commit into
conduit-protocol:mainfrom
arkhindi12:fix/react-package-ci

Conversation

@arkhindi12

Copy link
Copy Markdown
Contributor

@streamfi/react has never passed CI. The job is gated on needs: [build], and build has been skipped on every run since main stopped typechecking — so these errors were never surfaced. Now that #1038 unblocks the chain, the package fails 20 type errors across 10 files and 40 of its 102 tests.

Typecheck: 20 errors              -> 0
Tests:     40 failed / 62 passed  -> 0 failed / 111 passed

Depends on #1038. The @streamfi/react job is gated on needs: [build], and build is skipped whenever Lint & typecheck fails — which is every run on main since 2026-10-31. So this job's failure is only observable once #1038 lands. The two branches touch disjoint files (git diff --name-only for this one is entirely packages/react/), so they can be reviewed in parallel; this one's CI will show @streamfi/react as skipped until #1038 merges, and will go green on the resulting main without further changes.

StreamFiProvider — rpcHeaders was never implemented

#1009 (a9d49a2) added a call site without the feature:

const customFetch = rpcHeaders && Object.keys(rpcHeaders).length > 0
  ? makeHeaderFetch(rpcHeaders)
  : undefined;

Neither name existed:

$ git log --all --oneline -S "makeHeaderFetch"
a9d49a2 feat(react): rpc headers, cancel fee warning, clawback guard, two-step transfer (#1009)

$ git show a9d49a2 | grep -c "interface StreamFiProviderProps\|makeHeaderFetch"
1

That single hit is the added use of makeHeaderFetch — the commit never added the helper, and rpcHeaders is not a member of StreamFiProviderProps (which has only config, defaultNetwork, children, queryClient). So the header-forwarding feature was merged as a reference to two things that do not exist: the package could not compile, and rpcHeaders could not have done anything.

I implemented the intended behaviour rather than deleting the call site, because ConduitConfig.customFetch is already documented as the hook for exactly this — "for edge runtimes (Cloudflare Workers), proxies/custom HTTP agents, request-tracing headers, or timeouts that the global fetch does not provide":

  • makeHeaderFetch(headers, baseFetch?) wraps fetch and merges headers into each request. A header set per request wins; Headers handles the case-insensitive merge. undefined values are skipped — Headers would otherwise stringify them into the literal "undefined".
  • RpcHeaders = Record<string, string | undefined>, exported.
  • rpcHeaders added to StreamFiProviderProps and destructured, memoised on the prop so a fresh wrapper is not registered on every render (registerCustomFetch is keyed by rpcUrl and would be overwritten constantly).

An empty object or absent prop leaves customFetch undefined, so the SDK keeps using the platform fetch.

Covered by 9 new tests: header injection, caller-wins precedence, Headers and tuple-array init forms, undefined values, omitted init, plus three provider cases (headers forwarded to the constructed client, no prop, empty object).

rootDir violations

useCircuitState and useTransactionHistory reached across the package boundary into the SDK's source tree (../../../../src/...), which tsc refuses to emit outside rootDir. They now import the public surface: @conduit-protocol/sdk and @conduit-protocol/sdk/dashboard.

The circuit test had the same import, and that mattered beyond rootDir: the hook resolved the package (→ dist/) while the test resolved src/, producing two separate circuit registries. recordFailure in the test never affected the state the hook read, so those assertions could not pass. The test now imports what the hook imports.

Stale SDK signatures

  • useStreamsByRecipient / useStreamsBySender treated factory.streamsBySender() as bigint[]; it returns { ids, hasMore }. They also re-derived hasMore from ids.length === limit, while the SDK already computes it against the clamped limit — they now use the SDK's value, which is correct when limit is out of range.
  • useCancelStream — CancelStreamFn claimed Promise<string>, but the documented behaviour is "Opens the in-app confirmation flow; does not contact the wallet", and both return paths yield undefined. Corrected to Promise<void>; confirmCancel is the call that produces a hash. No test asserted a hash from cancel.

Smaller fixes

  • useTransferStreamRecipient: useState used but never imported.
  • useGovernorConfig / useGovernorProposals: optional result and option fields did not admit undefined under exactOptionalPropertyTypes; the implementation overload now matches the second public overload.
  • useTransactionHistory: walletAddress spread conditionally instead of passed as undefined.
  • useStreamNotes: noUncheckedIndexedAccess — guard the key/value pair before using it as an index.

Test fixes

Four of these were tests asserting something the code never promised:

  • Seven files mocked @conduit-protocol/sdk with only ConduitClient, so every render threw No "SUPPORTED_NETWORKS" export is defined on the "@conduit-protocol/sdk" mock. The mocks now stub what StreamFiProvider reads at module scope. (useNetworkSwitcher and useWithdrawableAmount already spread importActual/importOriginal and were fine.)
  • useStreamCountdown — the hook polls the clock on requestAnimationFrame, and jsdom quantises frames to ~16.67ms, so advanceTimersByTime(1000) stops at ~983ms, one frame short of the second boundary. Verified directly: advancing 1016ms ticks, 1000ms does not. Advancing one frame past the boundary makes the tests exercise the hook rather than the frame scheduler; the two progressFraction equality checks became toBeCloseTo.
  • issue-989 — the test awaited resume() while get was a never-resolving promise, but resume() settles only after the post-submit refresh. The assertion is that the transaction is submitted, so it now waits for the mutation call rather than the promise.

Verification

packages/react:  tsc --noEmit clean · 111/111 tests · tsup build succeeds
SDK (root):      tsc --noEmit clean · 1643/1643 tests

The SDK suite is unchanged by this PR — git diff --name-only main...HEAD touches only packages/react/.

…tests

`@streamfi/react` has never passed CI: the job is gated on `needs: [build]`,
and `build` has been skipped on every run since main stopped typechecking, so
these errors were never surfaced. Now that conduit-protocol#1038 unblocks the chain, the
package fails 20 type errors across 10 files and 40 of its 102 tests.

Typecheck: 20 errors -> 0
Tests:     40 failed / 62 passed -> 0 failed / 111 passed

`StreamFiProvider` — `rpcHeaders` was never implemented
-------------------------------------------------------

conduit-protocol#1009 (`a9d49a2`) added a call site without the feature:

```tsx
const customFetch = rpcHeaders && Object.keys(rpcHeaders).length > 0
  ? makeHeaderFetch(rpcHeaders)
  : undefined;
```

Neither name existed. `rpcHeaders` is not a member of `StreamFiProviderProps`
(only `config`, `defaultNetwork`, `children`, `queryClient`), and
`makeHeaderFetch` appears nowhere in the repository — `git log -S` finds only
that one commit. The feature was merged as a use of things that were never
added, so the package could not compile and `rpcHeaders` did nothing.

Implemented the intended behaviour rather than deleting the call site, since
`ConduitConfig.customFetch` is already the documented hook for exactly this
("request-tracing headers"):

* `makeHeaderFetch(headers, baseFetch?)` wraps `fetch` and merges `headers`
  into each request. A header set per request wins; `Headers` handles the
  case-insensitive merge. `undefined` values are skipped — `Headers` would
  otherwise stringify them into the literal `"undefined"`.
* `RpcHeaders = Record<string, string | undefined>` exported from the package.
* `rpcHeaders` added to `StreamFiProviderProps` and destructured, memoised on
  the prop so a new wrapper is not registered on every render
  (`registerCustomFetch` is keyed by `rpcUrl` and would be overwritten).

Covered by 9 new tests: header injection, caller-wins precedence, `Headers`
and tuple init forms, `undefined` values, omitted `init`, and three provider
cases (headers forwarded, no headers, empty object).

`rootDir` violations
--------------------

`useCircuitState` and `useTransactionHistory` imported across the package
boundary into the SDK's source tree (`../../../../src/...`), which `tsc`
refuses to emit outside `rootDir`. They now import the package's public
surface: `@conduit-protocol/sdk` and `@conduit-protocol/sdk/dashboard`.

The circuit test had the same import, which mattered beyond `rootDir`: the
hook resolved the package (→ `dist/`) while the test resolved `src/`, creating
two separate circuit registries, so `recordFailure` never affected the state
the hook read. Aligned the test with the hook.

Stale SDK signatures
--------------------

* `useStreamsByRecipient` / `useStreamsBySender` treated
  `factory.streamsBySender()` as `bigint[]`; it returns
  `{ ids, hasMore }`. They also re-derived `hasMore` from `ids.length === limit`
  while the SDK already computes it against the *clamped* limit — now uses the
  SDK's value.
* `useCancelStream`: `CancelStreamFn` claimed `Promise<string>`, but the
  documented behaviour is "opens the in-app confirmation flow; does not
  contact the wallet", and both paths return `undefined`. Corrected to
  `Promise<void>`; `confirmCancel` remains the call that yields a hash.

Smaller fixes
-------------

* `useTransferStreamRecipient`: `useState` used but not imported.
* `useGovernorConfig` / `useGovernorProposals`: optional result and option
  fields did not admit `undefined` under `exactOptionalPropertyTypes`.
* `useTransactionHistory`: `walletAddress: undefined` spread conditionally
  instead of passed explicitly.
* `useStreamNotes`: `noUncheckedIndexedAccess` — guard the key/value pair
  before indexing.

Test fixes
----------

* Seven files mocked `@conduit-protocol/sdk` with only `ConduitClient`, so
  every render threw "No SUPPORTED_NETWORKS export". The mocks now stub what
  `StreamFiProvider` reads at module scope.
* `useStreamCountdown`: the hook polls on `requestAnimationFrame`, and jsdom
  quantises frames to ~16.67ms, so `advanceTimersByTime(1000)` lands at
  ~983ms — one frame short of the boundary. Advancing by a frame extra makes
  the tests exercise the hook rather than the frame scheduler; the two
  `progressFraction` equality checks became `toBeCloseTo`.
* `issue-989`: the test awaited `resume()` against a never-resolving `get`,
  but `resume()` settles only after the post-submit refresh. It asserts
  submission, so it now waits for the mutation call instead of the promise.

Verified: `tsc --noEmit` clean, 111/111 tests, `tsup` build succeeds. SDK
suite still 1643/1643 and typechecks clean.
@arkhindi12

Copy link
Copy Markdown
Contributor Author

Cross-PR verification: both green together

The two PRs are mutually blocking in CI, so neither can prove the other:

To prove they are green together rather than just locally, I built a throwaway branch on top of main with both PRs' commits (2d748b1 + 5e98cd4 from this PR, 1e29484 from #1039) and ran the real CI matrix on it — PR #1040, a draft used only to trigger the workflow:

@streamfi/react            pass   1m38s
Bundle size check          pass     59s
Build                      pass   1m11s
Build TypeDoc              pass   1m09s
Lint & typecheck           pass   1m04s
Size limit                 pass     50s
Test                       pass   2m14s

7/7 pass, including the @streamfi/react job that has failed on every run in this repository. No skipped jobs besides the ones that are always skipped (Browser compatibility (E2E) and Deploy to GitHub Pages only run on main).

Local state on that combined branch:

SDK (root)      tsc --noEmit   0 errors
packages/react  tsc --noEmit   0 errors
packages/react  vitest         111/111 passed

Merge order

  1. This PR (fix: restore green CI — 23 type errors and 28 failing tests #1038) — turns Lint & typecheck, Test, Build etc. green, which un-skips Build and therefore @streamfi/react.
  2. fix(react): build @streamfi/react again — 20 type errors, 40 failing tests #1039 — will then run @streamfi/react and pass on its own, no further changes needed.

The two branches touch disjoint files (git diff --name-only for #1039 is entirely under packages/react/), so they can be reviewed in parallel — but they must merge in the order above for CI to stay green in between.

I am closing the temporary verification PR now; it contains no changes of its own.

This branch has not been deployed

No deployments
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.

1 participant