Skip to content
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

Merged
merged 21 commits into from
Dec 22, 2024

Conversation

huhuanming
Copy link
Contributor

@huhuanming huhuanming commented Dec 21, 2024

Summary by CodeRabbit

  • 新功能
    • 添加了网络可达性管理系统,允许组件监控网络状态。
    • 引入了延迟承诺的自定义钩子,改善异步操作管理。
    • 更新了多个组件以使用新的网络信息钩子和延迟承诺钩子。
  • Bug 修复
    • 更新了网络状态刷新逻辑,确保在网络连接恢复时重新验证。
  • 文档
    • 更新了事件总线以支持网络信息刷新事件。
  • 样式
    • 更新了组件以简化网络状态显示。
  • 重构
    • 精简了网络检查逻辑,提高了代码可读性和维护性。
    • 统一了延迟承诺的使用,增强了组件间的一致性。

Copy link

codesandbox bot commented Dec 21, 2024

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

Copy link
Contributor

coderabbitai bot commented Dec 21, 2024

概述

演练

这组变更主要涉及网络状态管理和异步操作处理的重构。项目移除了对 @react-native-community/netinfo 的依赖,并引入了自定义的网络信息和延迟 Promise 钩子。新的实现提供了更灵活的网络连接检测和异步操作管理机制,同时优化了组件间的状态传递和事件处理。

变更

文件 变更摘要
apps/mobile/package.json 移除 @react-native-community/netinfo 依赖
packages/components/src/hooks/index.tsx 新增 useDeferredPromiseuseNetInfo 导出
packages/components/src/hooks/useDeferredPromise.ts 新增延迟 Promise 实现
packages/components/src/hooks/useNetInfo.ts 新增网络可达性管理系统
packages/kit/src/components/NetworkAlert.tsx 更新网络状态钩子导入
packages/kit/src/components/Spotlight/index.tsx 集成 useDeferredPromise 钩子
packages/kit/src/hooks/usePromiseResult.ts 集成新的网络状态和延迟 Promise 钩子
packages/kit/src/provider/Container/NetworkReachabilityTracker.tsx 简化网络检查逻辑
packages/kit/src/views/Market/MarketDetail.tsx 更新 useDeferredPromise 导入
packages/kit/src/views/Market/components/TokenDetailTabs.tsx 更新组件以支持延迟 Promise
packages/kit/src/views/Market/components/TokenPriceChart.tsx 改进异步操作处理
packages/kit/src/views/Setting/pages/List/DevSettingsSection/NetInfo.tsx 更新网络状态处理逻辑
packages/shared/src/eventBus/appEventBus.ts 新增 RefreshNetInfo 事件
packages/shared/src/modules3rdParty/@react-native-community/netinfo/index.ts 删除自定义的 useNetInfo 钩子
packages/shared/src/request/axiosInterceptor.ts 增强错误处理和网络信息更新
packages/shared/src/request/fetchInterceptor.ts 改进请求头处理

序列图

sequenceDiagram
    participant App
    participant NetInfo
    participant EventBus
    
    App->>NetInfo: 请求网络状态
    NetInfo->>NetInfo: 检查网络连接
    NetInfo-->>App: 返回网络状态
    
    App->>EventBus: 触发网络刷新事件
    EventBus->>NetInfo: 通知刷新网络状态
    NetInfo->>NetInfo: 重新检查网络
    NetInfo-->>App: 更新网络状态
Loading

这个序列图展示了新的网络状态管理流程,包括状态检查、事件通知和状态更新的交互过程。

Tip

CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command @coderabbitai generate docstrings to have CodeRabbit automatically generate docstrings for your pull request. We would love to hear your feedback on Discord.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8524d9f and 69a0f7e.

📒 Files selected for processing (1)
  • packages/shared/src/request/fetchInterceptor.ts (1 hunks)
🔇 Additional comments (2)
packages/shared/src/request/fetchInterceptor.ts (2)

Line range hint 89-106: 错误处理和日志记录实现得当

网络错误处理和日志记录的实现完整且合理:

  • 捕获并记录了所有必要的错误信息
  • 包含了请求ID的追踪
  • 保持了错误的原始信息

61-61: 🧹 Nitpick (assertive)

建议优化重复的头部初始化代码

这里的头部初始化与之前的代码重复,建议合并到一处进行初始化。

建议如下修改:

-    options.headers = options.headers || {};
+    // 在函数开始时已经初始化过 headers,此处可以删除

Likely invalid or redundant comment.


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary or Summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@huhuanming huhuanming marked this pull request as ready for review December 22, 2024 04:48
@huhuanming huhuanming enabled auto-merge (squash) December 22, 2024 04:48
originalix
originalix previously approved these changes Dec 22, 2024
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: 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

📥 Commits

Reviewing files that changed from the base of the PR and between 033f85b and 8524d9f.

⛔ 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 是合适的,因为刷新网络状态不需要额外参数。

@huhuanming huhuanming merged commit 95f5391 into x Dec 22, 2024
8 checks passed
@huhuanming huhuanming deleted the netinfo branch December 22, 2024 15:35
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.

3 participants