-
Notifications
You must be signed in to change notification settings - Fork 392
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: replace the old version with a more maintainable Network Reachability test. #6392
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
概述演练这组变更主要涉及网络状态管理和异步操作处理的重构。项目移除了对 变更
序列图sequenceDiagram
participant App
participant NetInfo
participant EventBus
App->>NetInfo: 请求网络状态
NetInfo->>NetInfo: 检查网络连接
NetInfo-->>App: 返回网络状态
App->>EventBus: 触发网络刷新事件
EventBus->>NetInfo: 通知刷新网络状态
NetInfo->>NetInfo: 重新检查网络
NetInfo-->>App: 更新网络状态
这个序列图展示了新的网络状态管理流程,包括状态检查、事件通知和状态更新的交互过程。 Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (2)packages/shared/src/request/fetchInterceptor.ts (2)Line range hint 网络错误处理和日志记录的实现完整且合理:
建议优化重复的头部初始化代码 这里的头部初始化与之前的代码重复,建议合并到一处进行初始化。 建议如下修改: - options.headers = options.headers || {};
+ // 在函数开始时已经初始化过 headers,此处可以删除
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 10
🔭 Outside diff range comments (9)
packages/shared/src/request/axiosInterceptor.ts (1)
Line range hint
118-127
: 403 状态特殊处理可行,建议监控错误率。若 403 频发,或许需要更详细的信息反馈给用户或运维端。
packages/kit/src/components/Spotlight/index.tsx (2)
Line range hint
75-82
: useLayoutEffect 中对 triggerRef 的测量,最好在组件卸载时清理引用。虽然当前场景影响不大,但养成良好资源释放习惯更佳。
Line range hint
124-145
: SpotlightContent 通过 inline 样式精准定位遮罩内容。若后期有更多布局需求,可抽取为可配置的容器组件,减少重复样式配置。
packages/kit/src/hooks/usePromiseResult.ts (1)
Line range hint
200-214
: isEmptyResultRef 结合 isNative 模式判断首次结果,或可改用更兼容的空值检测方式。如果 result 是复杂结构,用 isEmpty 可能不够精准,需要视需求细化判断。
packages/kit/src/views/Market/components/TokenDetailTabs.tsx (2)
Line range hint
39-44
: 建议优化 defer 的处理逻辑使用固定的 100ms 延迟来解析 defer promise 可能会导致不必要的等待。建议根据实际数据加载状态来解析 promise。
useEffect(() => { - setTimeout(() => { - defer.resolve(null); - }, 100); + if (token) { + defer.resolve(null); + } }, [defer]);
Line range hint
124-134
: 建议重构滚动行为管理逻辑当前的滚动行为管理比较分散,建议将相关逻辑封装到一个自定义 hook 中。
+ const useTabScrollBehavior = (tabRef: MutableRefObject<ITabInstance | null>) => { + const changeTabVerticalScrollEnabled = useCallback( + ({ enabled }: { enabled: boolean }) => { + tabRef?.current?.setVerticalScrollEnabled(enabled); + }, + [], + ); + return { changeTabVerticalScrollEnabled }; + };packages/kit/src/views/Market/components/TokenPriceChart.tsx (1)
Line range hint
71-84
: 建议统一移动端和桌面端的加载状态处理当前移动端和桌面端的 defer 处理逻辑不一致,可能导致加载状态不同步。建议统一处理方式。
const init = useCallback(async () => { setIsLoading(true); const response = await backgroundApiProxy.serviceMarket.fetchTokenChart( coinGeckoId, days, ); - if (md) { - setTimeout(() => { - defer.resolve(null); - }, 100); - } else { - await defer.promise; - } + defer.resolve(null); setPoints(response); setIsLoading(false); }, [coinGeckoId, days, defer, md]);packages/kit/src/views/Market/MarketDetail.tsx (2)
Line range hint
155-165
: 建议增强数据刷新机制的错误处理当前的刷新机制缺少错误处理和加载状态的完整管理。建议添加错误处理和超时机制。
const onRefresh = useCallback(async () => { setIsRefreshing(true); + try { await fetchMarketTokenDetail(); + } catch (error) { + console.error('Failed to refresh market details:', error); + } finally { setIsRefreshing(false); + } }, [fetchMarketTokenDetail]);
Line range hint
171-182
: 建议增加 URL 构建的验证和错误处理URL 构建函数缺少参数验证和错误处理机制,建议添加相关逻辑以提高健壮性。
const buildDeepLinkUrl = useCallback( - () => - uriUtils.buildDeepLinkUrl({ + () => { + if (!coinGeckoId) { + throw new Error('Invalid coinGeckoId'); + } + return uriUtils.buildDeepLinkUrl({ path: EOneKeyDeepLinkPath.market_detail, query: { coinGeckoId, }, + }); }), [coinGeckoId], );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/mobile/ios/Podfile.lock
is excluded by!**/*.lock
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (17)
apps/mobile/package.json
(0 hunks)packages/components/src/hooks/index.tsx
(1 hunks)packages/components/src/hooks/useDeferredPromise.ts
(1 hunks)packages/components/src/hooks/useNetInfo.ts
(1 hunks)packages/kit/src/components/NetworkAlert.tsx
(1 hunks)packages/kit/src/components/Spotlight/index.tsx
(1 hunks)packages/kit/src/hooks/useDeferredPromise.ts
(0 hunks)packages/kit/src/hooks/usePromiseResult.ts
(1 hunks)packages/kit/src/provider/Container/NetworkReachabilityTracker.tsx
(1 hunks)packages/kit/src/views/Market/MarketDetail.tsx
(2 hunks)packages/kit/src/views/Market/components/TokenDetailTabs.tsx
(1 hunks)packages/kit/src/views/Market/components/TokenPriceChart.tsx
(1 hunks)packages/kit/src/views/Setting/pages/List/DevSettingsSection/NetInfo.tsx
(2 hunks)packages/shared/src/eventBus/appEventBus.ts
(2 hunks)packages/shared/src/modules3rdParty/@react-native-community/netinfo/index.ts
(0 hunks)packages/shared/src/request/axiosInterceptor.ts
(2 hunks)packages/shared/src/request/fetchInterceptor.ts
(1 hunks)
💤 Files with no reviewable changes (3)
- apps/mobile/package.json
- packages/shared/src/modules3rdParty/@react-native-community/netinfo/index.ts
- packages/kit/src/hooks/useDeferredPromise.ts
🔇 Additional comments (19)
packages/components/src/hooks/useNetInfo.ts (2)
90-93
: 在 fetch 开始前增加 isFetching 判定有助避免重复请求。
这里逻辑易懂且高效,避免了并发重复。值得保留。
161-183
: useNetInfo Hook 实现完备,re-render 逻辑简单易懂。
继续保持清晰,能快速让组件感知网络变化。
packages/shared/src/request/axiosInterceptor.ts (3)
9-12
: 添加了 RefreshNetInfo 事件通知,能优雅地替换旧的刷新方式。
这种基于 EventBus 的网络检测方式更灵活,推荐保留。
30-31
: debounce 优雅防抖刷新事件,频繁网络故障时可避免连续触发。
对性能有帮助,逻辑合理。
Line range hint 138-153
: 覆盖了 axios.create 的默认超时配置并附带全局拦截器,集成度高。
实现不错,但注意维护与升级时要保持同步更新拦截器逻辑。
packages/kit/src/components/Spotlight/index.tsx (2)
30-33
: useDeferredPromise 从组件库引入,统一异步调用。
此 Hook 通用性高,示例用法明确且容易跟踪。
Line range hint 193-211
: Spotlight 组件在 delayMs 后再渲染,逻辑易懂。
结合 isLocked 状态,精准控制显示时机。保持这种清晰性可读性不错。
packages/kit/src/hooks/usePromiseResult.ts (3)
8-9
: 替换旧 useNetInfo,改用新的 useNetInfo Hook。
逻辑更统一,减少依赖分散,维护成本更低。
Line range hint 40-58
: 利用 defer 在页面可见时再执行一次异步逻辑,思路巧妙。
但要注意复杂场景中,对可见性的判断或需多重条件确认。
Line range hint 155-162
: 在 revalidateOnReconnect 选项激活时,网络恢复后立即刷新请求。
确保用户获得最新数据,体验友好。此处设计不错。
packages/components/src/hooks/index.tsx (1)
2-2
: 导出声明看起来不错!
新增的两个 hook 导出遵循了统一的命名规范,保持了良好的代码组织结构。
Also applies to: 6-6
packages/kit/src/components/NetworkAlert.tsx (1)
5-5
: 请确认网络状态检查的变更
移除了 isRawInternetReachable
的使用,仅依赖 isInternetReachable
来判断网络状态。请验证这个改动是否会影响离线检测的准确性。
Also applies to: 9-9
✅ Verification successful
Let me analyze the results to verify if removing isRawInternetReachable is safe and if there are any other dependencies on this property.
移除 isRawInternetReachable 是安全的变更
NetworkAlert 组件只使用 isInternetReachable 来检测网络状态,这个改动是安全的。原因如下:
- NetworkAlert 组件本身从未使用过 isRawInternetReachable
- usePromiseResult 中的网络重连检测已经将 isRawInternetReachable 重命名为 isInternetReachable
- 开发者设置页面(DevSettingsSection)仍保留了 isRawInternetReachable 用于调试目的
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 搜索其他文件中是否还有使用 isRawInternetReachable 的地方
rg "isRawInternetReachable" --type tsx --type ts
Length of output: 76
Script:
#!/bin/bash
# Search for isRawInternetReachable usage in all files
rg "isRawInternetReachable"
# Check the NetworkAlert component implementation
ast-grep --pattern 'const NetworkAlert = () => {
$$$
}'
Length of output: 862
Script:
#!/bin/bash
# Check NetworkAlert implementation to understand network state usage
cat packages/kit/src/components/NetworkAlert.tsx
# Check usePromiseResult implementation since it uses isRawInternetReachable
cat packages/kit/src/hooks/usePromiseResult.ts
Length of output: 10904
packages/components/src/hooks/useDeferredPromise.ts (3)
3-7
: 枚举定义清晰简洁
枚举值的命名和状态定义符合最佳实践。
8-14
: 类型定义完整且实用
接口定义包含了所有必要的方法和属性,类型安全性好。
45-48
: Hook 实现简洁有效
使用 useMemo 确保了 Promise 实例的稳定性,符合 React Hooks 的最佳实践。
packages/kit/src/provider/Container/NetworkReachabilityTracker.tsx (1)
26-39
: 事件监听器清理实现正确
useEffect 中的事件监听器注册和清理实现得当,符合 React 最佳实践。
packages/shared/src/request/fetchInterceptor.ts (1)
Line range hint 82-111
: 错误处理实现完善
网络请求的错误处理和日志记录实现得当,包含了必要的错误信息。
packages/shared/src/eventBus/appEventBus.ts (2)
88-88
: 新增的事件名称符合规范!
RefreshNetInfo
事件名称简洁明了,清晰地表达了其用途。
261-261
: 事件负载类型定义正确!
将 RefreshNetInfo
事件的负载类型定义为 undefined
是合适的,因为刷新网络状态不需要额外参数。
Summary by CodeRabbit