Skip to content

Conversation

@littlesmilelove
Copy link
Contributor

@littlesmilelove littlesmilelove commented Dec 29, 2025

Home 首屏性能优化

TL;DR

进入 Home(尤其是 All‑Networks)时,首屏“Loading 迟到/卡顿”的根因不是单点,而是首屏窗口内并发触发了多条重链路(All‑Network 账户构建、兼容网络解析、autoSelect 级联刷新、TokenManagement、Earn 推荐拉取、Approvals 风险检测等),导致主线程/桥接被占用,UI 不能及时 paint。

这个 PR 的核心策略是:不改业务结果,只改执行时机与并发结构,把“首屏不必需”的重工作稳定后置到 UI 空闲后执行,并减少首屏阶段的重复计算与无效等待。

背景与目标

现象(用户可感知)

在进入 Home(All‑Networks 场景)后常见:

  • Loading 出现很晚Home:mount -> Home:loading:show 之间出现“卡住”的空窗。
  • 首屏掉帧/卡顿:首屏关键窗口内有同步重计算与多条并发重任务,导致 UI 不能及时渲染。
  • 刷新存在长尾:即使 Loading 出来后,Home:done:tokens 仍可能被尾部网络/计算拖长(本 PR 不以消除长尾为目标)。

目标(本 PR 要保证的事情)

  • 更快展示 Loading / 首屏结构:把首屏窗口里的同步重活挪走,让 UI 更早 paint。
  • 降低首屏并发重任务数量:避免多个重链路在首屏抢 CPU/桥接。
  • 不改变业务正确性:资产/余额/账户选择/网络兼容关系的最终结果不变;只允许“非首屏必需信息”延后展示。

非目标(本 PR 不做什么)

  • 不在此 PR 内彻底解决 All‑Networks token 刷新长尾(仍可能受网络并发与合并逻辑影响)。

策略总览(为什么这样做不会影响正确性)

  1. 用“UI 空闲边界”替代固定毫秒延时

    • 固定 setTimeout(1200/2000ms) 在不同设备/负载下不稳定,且容易带来偶发状态竞态。
    • 改为:至少等待若干帧(requestAnimationFrame)+(native)等交互完成(InteractionManager.runAfterInteractions)+ 再等待一帧,确保首屏 paint 优先。
  2. 把“非首屏必需”的入口链路后置

    • 例如:All‑Networks 的网络管理器提示、Earn 推荐拉新、Approvals 风险检测、TokenManagement 的本地数据聚合。
    • 这些功能的“最终正确性”依赖的是后台服务与状态源,不依赖“必须在首屏窗口内完成”;后置只会让 UI 更早可见。
  3. 减少重复计算/无效等待,同时保留后续稳定性

    • 首轮要快(去掉首轮 debounce),后续轮次要稳(保留节流/防抖)。

改动明细(按主题解释每个改动)

下文每项都会说明:改动点为什么能提升首屏为什么不影响正确性

A) 统一的“UI 空闲后执行”工具

  • 文件:packages/kit/src/utils/deferHeavyWork.ts
  • 改动点:
    • 新增 deferHeavyWorkUntilUIIdle({ minFrames, includeInteractions })
    • web/extension:requestAnimationFrame(无 raf 则 setTimeout(0)
    • native:额外等待 InteractionManager.runAfterInteractions
  • 提升原因:
    • 确保首屏至少完成 1–2 帧 paint 后再开始重工作,减少“Loading 迟到/空窗”。
  • 正确性说明:
    • 只是“延后执行”,不改变任何业务输入/输出;调用方均带取消保护,避免卸载后 setState。

B) All‑Networks 兼容网络解析后置(减少首屏同步重算)

  • 文件:packages/kit/src/components/AccountSelector/AllNetworksManagerTrigger.tsx
  • 改动点:
    • All‑Networks 下,首屏阶段不立刻触发 useEnabledNetworksCompatibleWithWalletIdInAllNetworks 的重计算:
      • walletId 先置空(不跑 compat)
      • UI 空闲后再恢复真实 walletId 触发计算
    • deferred 未 ready 时,刷新事件直接跳过
  • 提升原因:
    • “网络管理器提示/红点/数量”不是首屏资产展示的硬依赖;把 compat 解析从首屏窗口移走能明显减少 CPU/桥接压力。
  • 正确性说明:
    • 资产/余额/Token 列表不依赖该组件的 compat 结果;只会让网络管理器相关 UI 延后出现,最终结果不变。

C) Home 场景延后 autoSelect(减少首屏级联刷新)

  • 文件:packages/kit/src/components/AccountSelector/hooks/useAutoSelectAccount.tsx
  • 改动点:
    • sceneName === home 时:先 deferHeavyWorkUntilUIIdle() 再触发 autoSelectNextAccount
    • 增加取消保护(组件卸载/依赖变化后不再继续)
  • 提升原因:
    • autoSelect 常伴随账户/网络切换与状态派发,叠加 All‑Networks 解析会放大首屏卡顿。
  • 正确性说明:
    • autoSelect 的最终结果不变,只是更晚触发;首屏优先展示 Loading/骨架更符合 UX。

D) useAllNetworkRequests:首轮加速 + 后续保持稳定(同时修复并发与等待细节)

  • 文件:packages/kit/src/hooks/useAllNetwork.ts

D1. 首轮取消 debounce、后续轮次保留节流

  • 改动点:
    • usePromiseResultdebounced 设为 0
    • runCountRef 控制:仅第二轮及以后wait(POLLING_DEBOUNCE_INTERVAL)
    • wait() 只在“确认本轮会真正发起 All‑Networks 请求”时才执行(避免 disabled/缺参/正在 fetching 时白等)
  • 提升原因:
    • 首轮启动更早,Loading 更早出现;后续轮次仍有节流,避免轮询抖动。
  • 正确性说明:
    • 只是改变调度时机;轮询/刷新逻辑的触发条件与最终数据源不变。

D2. onStartedgetAllNetworkAccounts 并行化(并避免 Promise 未处理异常)

  • 改动点:
    • onStarted() 提前启动为 task,与 getAllNetworkAccounts 并行
    • 为避免 onStarted 过早 reject 触发 unhandledRejection,创建 task 时先 .catch(),随后在 await 后按原语义抛出
    • isFetching.current 改为 try/finally 复位,避免异常路径下卡死
  • 提升原因:
    • 把准备工作与账户构建并行化,减少关键路径总耗时;并让状态机在异常时更稳。
  • 正确性说明:
    • onStarted 仍会被等待完成(不会提前进入后续步骤);失败仍会按原语义抛出,只是更安全地处理时序。

F) TokenManagement:All‑Networks 首屏延后启用(避免重查询抢首屏资源)

  • 文件:
    • packages/kit/src/components/TokenListView/index.tsx
    • packages/kit/src/views/AssetList/hooks/useTokenManagement.ts
    • packages/kit/src/views/Home/pages/TokenListContainer.tsx
    • packages/kit/src/views/Home/components/TokenListBlock/TokenListBlock.tsx
  • 改动点:
    • TokenListView 新增 deferTokenManagement?: boolean
    • useTokenManagement 新增 enabled?: boolean:disabled 时直接返回空结果,不触发 All‑Network 账户构建/本地聚合
    • Home All‑Networks 下传 deferTokenManagement={true},并以 tokenListState.initialized 作为启用时机
  • 提升原因:
    • TokenManagement 在 All‑Networks 下会触发 getAllNetworkAccounts 等重链路,首屏阶段会与 token refresh/账户构建抢资源。
  • 正确性说明:
    • TokenManagement 影响的是“管理态/隐藏/自定义 token 的聚合与展示策略”,不是首屏必须;延后只会让相关能力晚一点可用,最终结果不变。

G) Earn:Home 首屏只读缓存,不触发重 fetch

  • 文件:
    • packages/kit/src/views/Earn/components/Recommended.tsx
    • packages/kit/src/views/Home/components/EarnListView/EarnListView.tsx
    • packages/kit/src/views/Earn/EarnHome.tsx
    • packages/kit/src/views/Earn/hooks/useEarnPortfolio.ts
  • 改动点:
    • Recommended 新增 enableFetch?: booleanenableFetch=false 时不触发 fetch(并通过 overrideIsFocused 绑定可见性)
    • Home 的 Earn 卡片:<Recommended enableFetch={false} />
    • Earn Tab:isEarnTabFocused 初始 false,并用 isActive guard 限制 portfolio 刷新仅在 Earn 真正 active 时执行
  • 提升原因:
    • Home 首屏不需要 Earn 实时拉新;把 staking/all-network 资产拉取留到 Earn 真正可见时执行,可减少首屏竞争。
  • 正确性说明:
    • Home 只展示缓存/入口;进入 Earn Tab 后仍会正常加载最新数据,业务结果不变。

H) Approvals:不阻塞首屏,但保证红点状态不 stale

  • 文件:packages/kit/src/views/Home/pages/HomePageView.tsx
  • 改动点:
    • Home 仍会获取一次 fetchAccountApprovals 来更新 hasRiskApprovals(红点)
    • 但改为:先 deferHeavyWorkUntilUIIdle() 再 fetch,避免与首屏 token 刷新/All‑Network 构建抢资源
    • 切换账号/网络时,会清理旧红点(避免 stale),再在 UI 空闲后更新为新结果
  • 提升原因:
    • 把“风险检测”的网络/序列化开销挪出首屏窗口,减少 Loading 迟到。
  • 正确性说明:
    • 该逻辑只影响红点提示,不影响资产/交易数据;fetch 仍会执行(只是后置),避免“必须进 Approvals Tab 才更新红点”的产品行为回退。

效果(来自带监控验证分支的趋势)

不同设备/冷启动状态会波动,但趋势稳定:

  • Home:mount -> Home:loading:show:明显提前(常见 1–2s 量级改善)
  • 首屏关键窗口内 jsblock:main 明显减少(Loading 迟到问题基本消失)
  • Home:tokens:allnet:deps:ready -> run:从“秒级等待”降到“接近 0”
  • 仍可能存在的长尾:
    • All‑Networks token refresh 的并发网络请求/合并(Home:done:tokens

回归关注点 / QA Checklist

  • All‑Networks 下网络管理器提示(network count/红点)可能延后出现,但应能在 UI 空闲后正常更新。
  • Home 场景 autoSelect 延后:确认不会把“loading 中”误判为“空账户/空网络”。
  • All‑Networks 下 TokenManagement 延后:确认列表/筛选逻辑在 tokenListState.initialized 后恢复一致。
  • Earn:Home 卡片不再首屏拉新;进入 Earn Tab 后应能正常加载/刷新。
  • Approvals:Home 红点应在 UI 空闲后更新;切换账号/网络不应出现旧红点残留。

Summary by CodeRabbit

  • Performance Improvements

    • Deferred heavy UI work until idle on All Networks and Home pages to improve responsiveness
    • Debounced and reduced redundant network data fetching; guarded refreshes to avoid unnecessary work
  • New Features

    • New optional prop to defer token management initialization for All Networks
    • Opt-out props to disable background fetching in Earn/recommended components
    • Earn tab no longer initializes focused by default

✏️ Tip: You can customize this high-level summary in your review settings.

- Defer non-critical heavy work until UI idle (compat networks, autoSelect, approvals red-dot)
- Speed up first All-Network requests; keep later debounce
- Defer TokenManagement and Earn fetches from Home first screen
- Add safe caching/forceRefresh for enabled-network compat calc
@revan-zhang
Copy link
Contributor

revan-zhang commented Dec 29, 2025

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 29, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds UI-idle deferment and debounce flows to delay expensive compatibility, approval, token, and earn fetches. Introduces deferHeavyWorkUntilUIIdle, guards and cancellation around listeners and initial fetches, feature gates to skip token/earn work, and a deferMs path for all-network compatibility resolution.

Changes

Cohort / File(s) Summary
Deferred Work Utility
packages/kit/src/utils/deferHeavyWork.ts
New export deferHeavyWorkUntilUIIdle plus helpers (nextFrame, nextFrames, runAfterInteractions) to wait frames and optionally await InteractionManager; platform-aware no-op on non-native.
Account selector & auto-select
packages/kit/src/components/AccountSelector/AllNetworksManagerTrigger.tsx, packages/kit/src/components/AccountSelector/hooks/useAutoSelectAccount.tsx
Add isDeferredReady state and shouldEnableCompatQuery; defer compatibility queries and initial auto-select on Home scene via deferHeavyWorkUntilUIIdle; use cancellation guards and memoized compatQueryWalletId; skip/guard appEventBus refreshes while deferred; rewire event listeners with proper cleanup.
All-network fetching & compat logic
packages/kit/src/hooks/useAllNetwork.ts
Rework main all-network flow: add debounced runs (runCountRef/timers), new IEnabledNetworksCompatResult and getEmptyEnabledNetworksResult, try/finally with onStarted/onFinished hooks, enhanced caching and empty-account early returns, and add deferMs?: number to useEnabledNetworksCompatibleWithWalletIdInAllNetworks to delay compatibility computation.
Token management gating
packages/kit/src/components/TokenListView/index.tsx, packages/kit/src/views/AssetList/hooks/useTokenManagement.ts
TokenListView gains new optional prop deferTokenManagement?: boolean. useTokenManagement gains enabled (default true) and early-return when disabled to skip token loads.
Token list integration
packages/kit/src/views/Home/components/TokenListBlock/TokenListBlock.tsx, packages/kit/src/views/Home/pages/TokenListContainer.tsx
Pass deferTokenManagement={!!network?.isAllNetworks} into TokenListView for all-networks mode.
Earn fetch gating
packages/kit/src/views/Earn/EarnHome.tsx, packages/kit/src/views/Earn/components/Recommended.tsx, packages/kit/src/views/Earn/hooks/useEarnPortfolio.ts, packages/kit/src/views/Home/components/EarnListView/EarnListView.tsx
EarnHome sets initial focus false. Recommended adds enableFetch?: boolean (default true) and skips fetch when false. useEarnPortfolio gates fetches on isActive. EarnListView disables Recommended fetch via enableFetch={false}.
Home page approvals deferred
packages/kit/src/views/Home/pages/HomePageView.tsx
Replace prior promise logic with an effect that defers approval fetch until UI idle when bulk revoke enabled; uses cancellation flags and CanceledError handling; keeps hasRiskApprovalsRef in sync.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor User
    participant UI as UI Component
    participant Defer as deferHeavyWorkUntilUIIdle
    participant Frames as RAF Loop
    participant Interact as InteractionManager
    participant Worker as Heavy Work (compat/approvals/tokens)
    Note over UI,Defer: Idle-deferral flow added for expensive tasks
    User->>UI: Mount or trigger feature (All Networks / Home)
    UI->>Defer: call deferHeavyWorkUntilUIIdle(minFrames, includeInteractions)
    Defer->>Frames: wait minFrames (requestAnimationFrame)
    Frames-->>Defer: frames elapsed
    alt includeInteractions == true
        Defer->>Interact: wait for interactions/layout
        Interact-->>Defer: interactions complete
    end
    Defer->>Frames: wait final frame
    Frames-->>Defer: ready
    Defer-->>UI: resolve (defer complete)
    UI->>Worker: run heavy task (compat/approvals/tokens) with cancellation guard
    Worker-->>UI: results (UI verifies not cancelled before setState)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.25% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: performance optimization for Home's first screen without performance marks.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 906d0a9 and 0e7b644.

📒 Files selected for processing (13)
  • packages/kit/src/components/AccountSelector/AllNetworksManagerTrigger.tsx
  • packages/kit/src/components/AccountSelector/hooks/useAutoSelectAccount.tsx
  • packages/kit/src/components/TokenListView/index.tsx
  • packages/kit/src/hooks/useAllNetwork.ts
  • packages/kit/src/utils/deferHeavyWork.ts
  • packages/kit/src/views/AssetList/hooks/useTokenManagement.ts
  • packages/kit/src/views/Earn/EarnHome.tsx
  • packages/kit/src/views/Earn/components/Recommended.tsx
  • packages/kit/src/views/Earn/hooks/useEarnPortfolio.ts
  • packages/kit/src/views/Home/components/EarnListView/EarnListView.tsx
  • packages/kit/src/views/Home/components/TokenListBlock/TokenListBlock.tsx
  • packages/kit/src/views/Home/pages/HomePageView.tsx
  • packages/kit/src/views/Home/pages/TokenListContainer.tsx
🧰 Additional context used
🧬 Code graph analysis (5)
packages/kit/src/components/TokenListView/index.tsx (1)
packages/kit/src/views/AssetList/hooks/useTokenManagement.ts (1)
  • useTokenManagement (23-249)
packages/kit/src/utils/deferHeavyWork.ts (2)
packages/shared/src/polyfills/polyfillsPlatform.js (1)
  • platformEnv (19-19)
apps/desktop/app/libs/react-native-mock.ts (1)
  • InteractionManager (9-9)
packages/kit/src/views/Home/components/TokenListBlock/TokenListBlock.tsx (1)
packages/kit-bg/src/providers/ProviderApiAptos.ts (1)
  • network (200-202)
packages/kit/src/views/Home/pages/HomePageView.tsx (1)
packages/kit/src/utils/deferHeavyWork.ts (1)
  • deferHeavyWorkUntilUIIdle (47-61)
packages/kit/src/components/AccountSelector/hooks/useAutoSelectAccount.tsx (1)
packages/kit/src/utils/deferHeavyWork.ts (1)
  • deferHeavyWorkUntilUIIdle (47-61)
🔇 Additional comments (30)
packages/kit/src/components/TokenListView/index.tsx (3)

117-118: LGTM! New prop follows consistent patterns.

The deferTokenManagement prop is threaded correctly through the component interface and destructuring.

Also applies to: 158-159


252-266: Confirm customTokens dependency is intentional.

Adding customTokens to the dependencies at line 265 is correct since it's used in the hideZeroBalanceTokens filter (lines 232-240). However, verify this doesn't cause unintended re-renders when customTokens updates frequently.


181-188: Add error recovery for token list initialization.

The gating logic !deferTokenManagement || tokenListState.initialized will never activate token management if token list initialization fails. When a non-CanceredError occurs during fetch, the exception is rethrown (line 449) and the component crashes instead of gracefully degrading. Even for CanceledError, there's no fallback to set initialized: true. Consider catching errors and either setting initialized: true as a fallback after retries, or implementing a timeout to prevent indefinite waits on slow networks.

packages/kit/src/views/Home/components/EarnListView/EarnListView.tsx (1)

20-28: LGTM! Disabling fetch on Home card makes sense.

Passing enableFetch={false} to the Recommended component on the Home card aligns with the performance goal by using cached data only. The Earn tab will still fetch when truly active.

packages/kit/src/views/Home/components/TokenListBlock/TokenListBlock.tsx (1)

2009-2009: LGTM! Consistent deferral for All Networks.

Deferring token management when network?.isAllNetworks is true aligns with the performance strategy. The boolean coercion is safe.

packages/kit/src/views/Home/pages/TokenListContainer.tsx (1)

1935-1983: LGTM! Consistent deferral pattern.

Adding deferTokenManagement={!!network?.isAllNetworks} matches the usage in TokenListBlock.tsx. This ensures consistent behavior across All Networks views.

packages/kit/src/components/AccountSelector/hooks/useAutoSelectAccount.tsx (2)

35-50: Cancellation logic looks sound.

The cancelled flag prevents heavy work from executing if the component unmounts during the deferral. The cleanup function correctly sets cancelled = true. The pattern is correct.


37-40: Confirm deferral doesn't block critical auto-select.

Deferring autoSelectNextAccount on the home scene improves first-screen performance. However, verify that this doesn't delay account selection too long, causing the user to see an empty or incorrect account state.

packages/kit/src/views/Earn/hooks/useEarnPortfolio.ts (2)

536-536: LGTM! Early return when inactive is efficient.

Returning early when !isActive prevents unnecessary fetch work. This aligns with the performance goal.


764-764: LGTM! Focus override respects active state.

The overrideIsFocused: (isFocused) => isFocused && isActive ensures polling only runs when the Earn tab is both focused and active. This is the correct logic.

packages/kit/src/views/Earn/components/Recommended.tsx (3)

223-231: LGTM! Prop default preserves existing behavior.

The enableFetch prop defaults to true, ensuring existing usage isn't broken. The destructuring is clean.


249-251: LGTM! Early return when fetch disabled is correct.

Returning cached recommendedTokens when enableFetch is false avoids unnecessary fetches while still showing cached data. This is the right approach.


277-277: LGTM! Focus override respects enableFetch.

The overrideIsFocused: (isFocused) => isFocused && enableFetch ensures fetching only occurs when both focused and fetch is enabled. Correct logic.

packages/kit/src/views/Earn/EarnHome.tsx (1)

66-66: Focus state initialization is correct and causes no delay.

Initializing isEarnTabFocused to false prevents unnecessary data fetching for invisible tabs. When the Earn tab becomes active, useListenTabFocusState updates the focus state immediately, which triggers data fetching right away through usePromiseResult. The current implementation has no artificial delays.

packages/kit/src/views/Home/pages/HomePageView.tsx (3)

6-6: LGTM: Clean imports for deferred work pattern.

The CanceledError import and deferHeavyWorkUntilUIIdle utility align with the PR's strategy to defer heavy work until UI idle.

Also applies to: 46-46


87-90: Good: Ref tracks approval state across renders.

This ref correctly maintains the latest hasRiskApprovals value for the async fetch closure, avoiding stale-closure issues.


161-209: Verify: Red-dot reset may cause brief flicker.

The eager reset at line 166 clears hasRiskApprovals immediately on account/network switch, even if the new context also has risk approvals. This could cause a brief red-dot disappearance followed by reappearance.

Consider keeping the previous state until the new fetch completes, or add a loading flag to distinguish "fetching" from "confirmed no risk."

Do you want the red-dot to briefly disappear during switches, or should it persist until the new fetch confirms absence of risk?

packages/kit/src/views/AssetList/hooks/useTokenManagement.ts (1)

27-27: LGTM: Clean feature-flag for heavy token loading.

The enabled parameter correctly gates the expensive token aggregation logic. When false, it short-circuits early (lines 44-50) and returns empty collections, avoiding heavy DB queries and API calls. The dependency array (line 201) includes enabled to re-run when toggled.

Also applies to: 32-32, 44-50, 201-201

packages/kit/src/utils/deferHeavyWork.ts (2)

3-14: LGTM: Robust frame deferral with fallback.

The nextFrame implementation correctly prefers requestAnimationFrame and falls back to setTimeout(0) when RAF is unavailable. The fallback ensures compatibility across environments.


24-45: Good: Platform-guarded dynamic import prevents bundle bloat.

The platform check (line 25) and dynamic require (line 31) correctly avoid importing React Native's InteractionManager on web/extension. The try-catch (lines 28-44) gracefully handles environments where the module is unavailable.

packages/kit/src/components/AccountSelector/AllNetworksManagerTrigger.tsx (3)

45-68: Good: Deferred compat query with cancellation guard.

The shouldEnableCompatQuery flag (lines 45-48) correctly identifies when deferment is needed (All Networks + non-Others wallet). The effect (lines 53-68) defers heavy work using deferHeavyWorkUntilUIIdle and updates isDeferredReady with proper cancellation handling via the cancelled flag.


70-75: Clever: Empty string gates expensive hook until UI idle.

The compatQueryWalletId memo returns an empty string until isDeferredReady is true. This effectively disables the expensive useEnabledNetworksCompatibleWithWalletIdInAllNetworks hook during initial render, deferring the heavy compat query.


89-119: Skeleton correctly displays during deferment.

The refresh handlers (lines 89-99) properly skip when deferment is active. The hook returns an empty compatibleNetworks array when walletId is empty (line 497-498 in useAllNetwork.ts), which triggers the skeleton condition at line 151 (enabledNetworksCompatibleWithWalletId.length === 0). The deferment flow is sound: while isDeferredReady is false, compatQueryWalletId remains empty, the hook returns an empty array, and the skeleton displays as intended.

packages/kit/src/hooks/useAllNetwork.ts (7)

30-43: LGTM: Type-safe result structure.

The IEnabledNetworksCompatResult type and getEmptyEnabledNetworksResult helper provide a consistent structure for enabled networks data. This improves type safety and makes default initialization cleaner.


164-164: Good: Smart debouncing—immediate first run, debounced thereafter.

The runCountRef pattern (lines 164, 171, 214) tracks consecutive runs. First invocation (runCountRef === 0) skips the debounce (lines 185-195), ensuring fast initial paint. Subsequent runs wait POLLING_DEBOUNCE_INTERVAL to reduce thrashing. The ref resets on account/network/wallet changes (line 171), treating each context switch as a new "first run."

Also applies to: 171-181, 185-195, 214-214


217-283: Excellent: Parallel execution of onStarted and account fetch.

The refactor runs onStarted (line 227) and getAllNetworkAccounts (line 238) in parallel rather than sequentially. This reduces blocking time during the critical first-screen window. Error capture via onStartedError (line 232) and deferred throw (line 281) ensures errors don't silently disappear while still allowing the accounts fetch to complete.


441-443: Critical: Finally block ensures isFetching cleanup.

The finally block (line 441-443) guarantees that isFetching.current resets even if errors occur. This prevents the hook from getting stuck in a "fetching" state that blocks subsequent runs.


484-484: Good: deferMs parameter enables controllable compat query delay.

The new deferMs parameter (lines 484, 491) allows callers to defer heavy compat network queries. Lines 518-520 wait for the specified duration after fetching enabled/disabled networks but before computing compatibility. This aligns with the PR's strategy to defer non-critical work until UI idle.

Also applies to: 491-491, 518-520


464-464: Observe: Debounce disabled via debounced: 0.

Line 464 sets debounced: 0, disabling usePromiseResult's built-in debouncing. The custom debounce logic (lines 185-195) using runCountRef replaces it. This is intentional and allows the first run to execute immediately.


285-330: Remove this comment—the early-exit concern is incorrect.

perf.done() at line 315 is just a performance marker, not a return statement. The code continues normally to line 337 where it executes the main requests block. The function doesn't exit early. Additionally, allNetworkDataInit.current is properly reset to false in the useEffect (line 170) when dependencies change, so fresh data fetches do occur on subsequent calls.

Likely an incorrect or invalid review comment.

@originalix
Copy link
Collaborator

@coderabbitai resolve

@originalix originalix enabled auto-merge (squash) December 30, 2025 14:55
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 30, 2025

✅ Actions performed

Comments resolved.

@originalix
Copy link
Collaborator

@codex review

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
packages/kit/src/views/Home/pages/HomePageView.tsx (1)

190-238: Solid deferred approval fetching pattern.

The cancellation logic is correct. The flow properly resets stale state, defers heavy work, then guards against races with cancelled checks.

One minor note: the catch block re-throws without logging. This was flagged in a past review.

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9812129 and 9be205d.

📒 Files selected for processing (2)
  • packages/kit/src/utils/deferHeavyWork.ts
  • packages/kit/src/views/Home/pages/HomePageView.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
packages/kit/src/views/Home/pages/HomePageView.tsx (1)
packages/kit/src/utils/deferHeavyWork.ts (1)
  • deferHeavyWorkUntilUIIdle (35-49)
packages/kit/src/utils/deferHeavyWork.ts (1)
packages/shared/src/polyfills/polyfillsPlatform.js (1)
  • platformEnv (19-19)
🔇 Additional comments (5)
packages/kit/src/views/Home/pages/HomePageView.tsx (2)

5-6: LGTM!

Imports are clean and used appropriately in the new deferred approval logic.

Also applies to: 45-45


116-119: LGTM!

Good use of a ref to track state without re-triggering the effect. This avoids stale closures.

packages/kit/src/utils/deferHeavyWork.ts (3)

1-2: LGTM!

Clean imports. Both are used appropriately.


4-26: LGTM!

The RAF-with-fallback pattern is solid. Math.max(0, frames) guards against negative counts. Simple and effective.


35-49: LGTM!

The sequencing is clear: wait for frames, optionally wait for interaction settling, then one more frame for layout flush. Callers handle cancellation via their own flags. This works well.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

auto-merge was automatically disabled December 30, 2025 15:26

Head branch was pushed to by a user without write access

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/kit/src/hooks/useAllNetwork.ts (1)

207-207: Remove debug console.log before merging.

This debug statement will clutter production logs. Remove or wrap in a debug/development check.

🔎 Proposed fix
-      console.log('useAllNetworkRequests >>>>>>>>>>>>>>');
+      // console.log('useAllNetworkRequests >>>>>>>>>>>>>>');

Or better, remove entirely if not needed for debugging.

♻️ Duplicate comments (1)
packages/kit/src/hooks/useAllNetwork.ts (1)

341-404: Add clarifying comments for these branching paths.

These two paths (cache warm vs. cache cold) handle different scenarios. Brief inline comments would help future readers understand:

  • Line 341: Cache is warm → parallel fetch all networks
  • Line 364: Cache is cold → fetch indexed backends first, then non-indexed
📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9be205d and 5a4634e.

📒 Files selected for processing (1)
  • packages/kit/src/hooks/useAllNetwork.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/kit/src/hooks/useAllNetwork.ts (5)
packages/kit-bg/src/vaults/types.ts (1)
  • IAccountDeriveTypes (136-140)
packages/shared/types/index.ts (1)
  • IServerNetwork (49-71)
packages/shared/src/consts/walletConsts.ts (1)
  • POLLING_DEBOUNCE_INTERVAL (10-10)
packages/kit-bg/src/services/ServiceAllNetwork/ServiceAllNetwork.ts (1)
  • IAllNetworkAccountInfo (29-41)
packages/shared/src/utils/promiseUtils.ts (1)
  • promiseAllSettledEnhanced (141-153)
🔇 Additional comments (7)
packages/kit/src/hooks/useAllNetwork.ts (7)

30-43: Clean extraction of result type and factory function.

Good move. The getEmptyEnabledNetworksResult factory ensures fresh objects each call. This avoids shared-reference bugs.


164-181: Solid approach for first-run bypass of debounce.

Resetting runCountRef on context changes ensures the first fetch is immediate. Subsequent fetches wait for POLLING_DEBOUNCE_INTERVAL. The expanded dependency array is correct.


217-234: Good parallel execution, but check failure semantics.

Running onStarted and getAllNetworkAccounts in parallel is a smart optimization. The error capture + re-throw pattern works.

One concern: if onStarted fails, clearAllNetworkData (line 219) has already run. The user sees cleared data until the next successful fetch. Confirm this is acceptable UX.


468-471: Correct: disable built-in debounce for manual control.

Setting debounced: 0 and handling it manually (lines 193-195) allows the first request to be immediate. Good alignment with the PR goals.


522-524: Deferral placed after initial fetches, before heavy processing.

The deferMs wait happens after network state is fetched but before the heavy compatibility processing. This is a sensible split - UI can paint while expensive work waits.


497-510: Stable initial result with correct early-exit handling.

The memoized initResult provides a stable reference for the initial state. Early returns create fresh empty objects, which is fine since those paths indicate invalid inputs.


185-195: Debounce logic is safe from state-update leaks but waits aren't cancellable.

timerUtils.wait() can't be cancelled if the component unmounts mid-wait. However, usePromiseResult prevents state updates after unmount through its isEffectValid flag and shouldSetState() checks (which verify isMountedRef). This prevents memory leaks, though the promise itself still resolves. If you want to optimize further, consider using AbortController for the timer to avoid unnecessary promise completions after unmount.

@originalix originalix enabled auto-merge (squash) December 30, 2025 15:40
@originalix originalix merged commit 2b5a846 into OneKeyHQ:x Dec 31, 2025
7 checks passed
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.

5 participants