Repository navigation
feat: TASK-21462 delete redux + dead-dep sweep - #2950
Conversation
TASK-21462, last of the three rebuild PRs — the withdraw (TASK-21816) and setup (TASK-21460) rebuilds drained the two slices with real structure, so what remained was mirrors: the user slice echoed the [USER] TanStack query, the wallet slice echoed the ['balance'] query, and bankForm had no consumers left. Readers now use the query-backed useAuth()/useBalance directly. The zeroDev flags move to a module-level external store (src/hooks/useZeroDevFlow.ts, useSyncExternalStore): writers sit on both sides of the provider tree (authContext resets on logout above KernelClientProvider), so a context hook would be an import cycle. Demo mode keeps its never-null-first-render guarantee via placeholderData on the user query (was the slice's initialState seed). Dead weight: drop @reduxjs/toolkit, react-redux, redux, ethers, siwe (the last two had zero imports). knip's entry config listed every src file as an entry, which disabled dead-export detection entirely — now only the real entries are declared (sw.ts is the one the Next plugin misses); the findings it starts reporting are backlog, not regressions. One shared FlowErrorState (interfaces.ts) replaces the withdraw/onramp duplicates. Deferred: @types/react 19 bump — tried, 25 type errors across 12 files including qr-pay (a surface owned by another lane); follow-up PR.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/chip review |
1 similar comment
|
/chip review |
|
/chip review |
![chip-peanut-bot[bot]](https://avatars.githubusercontent.com/in/2915219?s=60&v=4){kind=small}
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The Redux removal is coherent and CI is green, but the new shared ZeroDev external store has no behavioral coverage of its real subscription and reset contract.
Findings
-
MAJOR · src/hooks/useZeroDevFlow.ts:33 · Test the real ZeroDev store contract
useZeroDevFlownow owns wallet address, kernel readiness, and UserOp-in-flight state for multiple consumers, but every test that mentions it replaces the module with a mock. A regression where an action updates the snapshot without notifying all subscribers, or where logout reset leavesaddressorisSendingUserOpbehind, would still pass while the wallet remains blocked on stale identity state or stale-deployment reload becomes unsafe during a live UserOp. Add a focused hook test that mounts multiple real subscribers, drives the actions, verifies fan-out and unsubscribe behavior, and verifiesreset()restores every field. -
MINOR · src/hooks/useZeroDevFlow.ts:52 · [claude-opus] New zerodev shared-state store has no direct test; all consumers mock it
src/hooks/useZeroDevFlow.ts is a new hand-rolled module-level store (mutablestate, alistenersSet,write(),reset()) that replaces the deleted reduxzeroDevslice. It is app-global shared state written from three places on both sides of the provider tree (authContext logout, kernelClient.context as clients build, useZeroDev around register/login/send), yet no test exercises the module itself: useStaleDeploymentReload.test.tsx, useZeroDev-invite-onboarding.test.tsx and useZeroDev-login-failure.test.tsx alljest.mock('@/hooks/useZeroDevFlow', ...), replacing both the hook and every action with stubs.
Exact untested cases:
zeroDevFlowActions.setIsSendingUserOp(true)is observed by a mounteduseZeroDevFlow()subscriber. This is the interlock useStaleDeploymentReload.ts:112 depends on (isSafeRef.current = !hasPendingTransactions && !isSendingUserOp && ...) to refuse a stale-deployment document reload while a userOp is in flight. Ifwrite()ever failed to notify, or the snapshot went stale, the app would reload mid-transaction and every current test would still pass, because they all mock the store.zeroDevFlowActions.reset()clearsaddressand all four flags and notifies subscribers — the logout path in authContext.tsx:298 and the pre-registration wipe in useZeroDev.ts:82 both rely on this to drop the previous account's kernel address.
Fix: add a small src/hooks/tests/useZeroDevFlow.test.ts that renders useZeroDevFlow() with renderHook, asserts a subscriber re-renders with the new value after act(() => zeroDevFlowActions.setIsSendingUserOp(true)), and that after zeroDevFlowActions.setAddress('0x…') a reset() returns address to undefined and the flags to false. The repo already tests a comparable module-level useSyncExternalStore this way in src/hooks/tests/useNotifications.test.ts.
Checked clean
- Verified the detached worktree HEAD, supplied merge base, trusted author, PR metadata, and exact-head CI checks.
- Traced user ownership through useUserQuery and AuthProvider, including demo placeholder data, refresh behavior, and logout query clearing.
- Traced wallet balance ownership through useBalance/useWallet, address matching, stale display caching, and spendability gates.
- Traced all ZeroDev state readers and writers across registration, login, kernel initialization, logout, UserOp submission, and stale-deployment reload.
- Checked the provider tree and all production imports; no runtime Redux imports remain.
- Checked dependency and lockfile removal, knip entry configuration, and whitespace errors. The removed Redux package names remain as harmless stale externalization strings in both Next configs.
Security review by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only and answers only security, privacy and money, so treat its findings as advice.
Third opinion by claude-opus: 1 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 52e258ea07f5 · Context: repo · Took 10m (queued 25m)
…ePackageImports (Chip round 1) Every consumer spec mocks the store, so nothing exercised the actual subscription mechanics: fan-out to every mounted subscriber (the stale-deployment-reload interlock reads isSendingUserOp live), unsubscribe on unmount, per-field writes, and reset() restoring every field — which the logout and pre-registration wipes rely on to drop the previous account's kernel address. New useZeroDevFlow.test.ts runs all of it against the real module. Also removes the two stale redux entries from both Next configs' optimizePackageImports.
|
/chip review |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Clean. The direct useZeroDevFlow contract tests added at this head resolve both prior missing-test findings, and the Redux removal preserves the reviewed user, wallet, ZeroDev, and logout behavior.
Checked clean
- Confirmed the detached worktree HEAD and merge base exactly match the supplied head and base SHAs.
- Checked prior findings P1 and P2 against the real useZeroDevFlow implementation and its new direct fan-out, field-isolation, unsubscription, and full-reset tests; both are fixed.
- Reviewed the Redux-to-AuthContext and TanStack Query user-state migration, including demo seeding, auth failures, and logout cache clearing.
- Reviewed the wallet balance mirror removal and ZeroDev shared-state writers/readers across registration, login, transaction sending, kernel initialization, stale-deployment reload protection, and logout.
- Checked removed dependencies and source imports; no Redux, ethers, or siwe runtime imports remain.
- Exact-head CI completed successfully, including unit, typecheck, eslint, format, and aggregate ci-success checks; the detached worktree has no installed Jest binary for an additional local run.
Security review by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only and answers only security, privacy and money, so treat its findings as advice.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 7f0238ab2000 · Context: repo · Took 8m
|
/chip review |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Clean at the exact head. Both prior shared-store test findings are fixed by direct real-store coverage, and the broader Redux/dependency removal preserves the reviewed auth, wallet, and ZeroDev contracts.
Checked clean
- Pinned worktree HEAD and merge base match the supplied head and base SHAs.
- Prior P1 and P2 are fixed: useZeroDevFlow.test.ts imports the real store and covers initial state, multi-subscriber fan-out, field-preserving writes, unsubscribe, and full reset; the exact-head unit check passed.
- ZeroDev writers and the stale-deployment reader preserve the former singleton transitions, including logout and pre-registration address resets plus the in-flight UserOp reload interlock.
- User reads now share AuthProvider/TanStack data inside the same app provider boundary; the native demo route seeds the USER query synchronously before navigation.
- Removing the wallet Redux fallback leaves unresolved or mismatched addresses undefined/loading and cannot create spendable headroom; the removed slice only mirrored the same address-gated query.
- No imports of the deleted Redux, ethers, or siwe dependencies remain; exact-head unit, typecheck, eslint, format, design-system lint, baseline, and dependency-age checks passed.
- The shared FlowErrorState move is shape-identical, and the web/native Next configs no longer retain deleted Redux packages in optimizePackageImports.
- The aggregate report check was still in progress when this review completed; no completed exact-head check was failing.
Security review by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only and answers only security, privacy and money, so treat its findings as advice.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 85a796a099e4 · Context: repo · Took 10m
…delete-redux # Conflicts: # src/context/authContext.tsx # src/hooks/useZeroDev.ts
Came in with the encode-before-flag merge; its useAppDispatch/useZerodevStore mocks pointed at the deleted module — now mocks useZeroDevFlow like the rest.
|
/chip review |
Code-analysis diffPainscore total: 7188.22 → 7218.88 (+30.66) 🆕 New findings (421)
…and 401 more. ✅ Resolved (128)
…and 108 more. 📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The Redux removal is functionally sound and the prior real-store coverage gap is fixed, but one migrated hook still documents Redux as its live source.
Findings
- MINOR · src/components/Badges/useBadgeEarnToast.ts:16 · Update the badge-state ownership comment
This hook now reads the profile fromuseAuth(), backed by the[USER]TanStack query, but its adjacent contract still says the badge list comes through the Redux user store. After this PR deletes that store entirely, a maintainer tracing badge refreshes is sent to a path that no longer exists. Update the comment to name AuthContext/the user query.
Inline anchors unavailable for 1 finding(s); the findings remain in this summary.
Checked clean
- Verified the detached worktree head, supplied base SHA, and merge base exactly match the requested review target.
- Rechecked prior finding P1: the new test imports the real useZeroDevFlow module and covers multi-subscriber fan-out, unmount, partial writes, and full reset including address, so it is fixed at this head.
- Traced the user, wallet, and ZeroDev migrations through AuthProvider, KernelClientProvider, stale-deployment reload, demo entry, logout, and all changed production consumers.
- Confirmed the repository has no live imports or runtime references to the five removed direct dependencies and that the lockfile prunes their dependency chains.
- Checked exact-head CI: aggregate, unit, typecheck, lint, format, analysis, design-system lint, provenance, baseline, and preview checks passed; the non-gating screenshot job remained in progress.
- Ran git diff --check and confirmed the detached worktree stayed clean; local focused tests were not rerun because this read-only worktree has no installed dependencies.
Security review by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only and answers only security, privacy and money, so treat its findings as advice.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 2acaa1ec5344 · Context: repo · Took 14m
This branch was successfully deployed
1 participant
Lands via the
tech-debtintegration train (dev is merge-frozen). STACKED on #2949 (which stacks on #2917): base is temporarily the #2949 branch so this diff shows only the redux-deletion delta. Merge order: #2917 → #2949 → this; when #2949 merges (delete its branch), GitHub retargets this PR — verify the base is tech-debt before merging.Summary
Deletes
src/redux/(all of it) and the five dead dependencies. The task's premise held: after the withdraw (#2917) and setup (#2949) rebuilds drained the two slices with real structure, what remained was mirrors.Per-slice dissolution (the TASK-21462 map, as found):
user(19 read sites): it mirrored the[USER]TanStack query —useUserQueryfetched, then dispatched the same payload into redux, and the query's ownplaceholderDataread the redux value back (self-referential). Readers now takeuserfromuseAuth()(same object, same query). Demo mode keeps its never-null-first-render guarantee (protected-route redirect race) viaplaceholderData: isDemoMode() ? DEMO_USER : undefinedon the query — that was the slice'sinitialStateseed.wallet(1 consumer): a write-only echo of the['balance']query insideuseWalletitself, plus a fallback that could only ever return the same query's last value. The query is now the single owner; while the address gate is unresolved the balance isundefined, which every downstream gate already treats as loading, never as headroom.zeroDev(flags + kernel address): moved tosrc/hooks/useZeroDevFlow.ts— a module-level external store behinduseSyncExternalStore. Not a context: writers sit on both sides of the provider tree (authContext resets it on logout, and authContext mounts ABOVEKernelClientProvider, so a context hook there is an import cycle). Singleton semantics identical to the slice. Consumers:useZeroDev,kernelClient.context,useStaleDeploymentReload,authContext.bankForm: zero consumers left (feat: TASK-21816 withdraw on a URL-backed stepper + TASK-21454 Field #2917 moved DynamicBankAccountForm to react-hook-form). Deleted, along with the stale jest.mocks that still referenced it.setup: already gone (feat: TASK-21460 setup flow on the URL stepper, kills the redux setup slice #2949).queryClient.clear()(already there, deliberately ordered before the token wipe) is what wipes the user — the removedsetUser(null)dispatch only cleared the mirror.Dead-dep sweep:
@reduxjs/toolkit,react-redux,redux,ethers,siwe(the last two: zero imports anywhere).entry: ["src/**/*"]made every file an entry, which disabled unused-file/dead-export detection entirely — knip could only ever report unused deps. Now the Next plugin derives the real entries (+src/app/sw.ts, which it misses). What it starts reporting — 18 unused files, 125 unused exports, 132 unused types, all pre-existing — is backlog, deliberately NOT deleted here (pnpm knipis a manual script, not a CI gate).FlowErrorStateininterfaces.tsreplaces the withdraw/onramp duplicate{showError, errorMessage}shapes. (TheDirectSendFlowErrorState-style duplicates infeatures/payments/flows are the same shape — left for the payments extraction lane, flagged here.)Deferred (from the task's rider list)
@types/react19 bump: attempted; it surfaces 25 real type errors across 12 files, includingqr-pay/page.tsx(another lane's surface). Reverted — needs its own PR. (reactitself is already 19.2.1, so the mismatch is real and worth that PR.)Risks / breaking changes
useWallet,useZeroDev,kernelClient.context) changed only where their state lives, not what they do.undefinedthere and the existing stable/last-known display cache covers paint. Gates fail closed either way.AppStateProvidersnow mounts only wagmi. Provider order unchanged otherwise.QA
npm run typecheckclean · prettier clean · eslint on all touched files: 0 errors (2 pre-existing advisory warnings untouched) ·npm run buildgreen.add-money-states › loaded EVM depositcase (fails identically on pristine dev; see feat: TASK-21816 withdraw on a URL-backed stepper + TASK-21454 Field #2917).jest.mock('@/redux/...')re-pointed to what the source now reads (useAuth/useZeroDevFlow), merged where an authContext mock already existed; the useZeroDev login-failure assertion now checkssetIsLoggingIn(false)directly; stale mocks of already-drained slices (bankForm, setup) deleted.