refactor: use suspense for course page data loading state management #317
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the course page to use React Suspense and React Query patterns for better data loading management. The main changes move data fetching logic from component lifecycle methods into a dedicated hook and introduce proper error boundaries.
- Introduces
useCoursePageDatahook with React Query'suseSuspenseQueryfor centralized data fetching - Adds error boundary component for graceful error handling
- Refactors
CoursePagefrom a standalone component into a child component within the mainHomePage
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Adds packageManager field to specify Yarn version |
| hooks/useCourseDataSuspense.ts | New hook that centralizes course data loading with Suspense support |
| context/course-page.tsx | Updates context interface to include schedulesByDays and refetch function |
| components/course/course-error-boundary.tsx | New error boundary component for course page error handling |
| app/(tabs)/index.tsx | Major refactor to use Suspense pattern with nested CoursePage component |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // 设置较短的 staleTime，因为我们希望每次进入页面都能检查数据更新 | ||
| staleTime: 30000, // 30秒 |
There was a problem hiding this comment.
The comment states the staleTime is set to 30 seconds to check for data updates on each page entry, but this doesn't align with how React Query works. StaleTime of 30 seconds means the data won't be refetched for 30 seconds after being fetched, not that it checks every 30 seconds. If the goal is to check on page focus, consider setting refetchOnWindowFocus: true or reducing staleTime to 0. The current configuration and comment are misleading.
| // 设置较短的 staleTime，因为我们希望每次进入页面都能检查数据更新 | |
| staleTime: 30000, // 30秒 | |
| // 始终检查数据更新：每次进入页面或窗口聚焦时都会重新获取数据 | |
| staleTime: 0, | |
| refetchOnWindowFocus: true, |
| prevSettingsRef.current = currentSettingsStr; | ||
|
|
||
| // 使 React Query 缓存失效，触发重新加载 | ||
| await queryClient.invalidateQueries({ queryKey: ['course-page-all-data'] }); | ||
|
|
||
| // 重置组件（触发 Suspense 重新渲染） | ||
| setResetKey(prev => prev + 1); |
There was a problem hiding this comment.
Updating prevSettingsRef.current on line 267 before awaiting the async operations creates a timing issue. If the invalidation fails or the component unmounts before completion, the ref will already be updated, preventing retry logic on the next focus. Consider moving line 267 to after line 273, or better yet, use the result of successful invalidation to update the ref.
| prevSettingsRef.current = currentSettingsStr; | |
| // 使 React Query 缓存失效，触发重新加载 | |
| await queryClient.invalidateQueries({ queryKey: ['course-page-all-data'] }); | |
| // 重置组件（触发 Suspense 重新渲染） | |
| setResetKey(prev => prev + 1); | |
| // 使 React Query 缓存失效，触发重新加载 | |
| await queryClient.invalidateQueries({ queryKey: ['course-page-all-data'] }); | |
| // 重置组件（触发 Suspense 重新渲染） | |
| setResetKey(prev => prev + 1); | |
| // Only update the ref after successful invalidation and reset | |
| prevSettingsRef.current = currentSettingsStr; |
| }); | ||
|
|
||
| // 2. 定位当前日期 | ||
| const locateDateRes = await locateDate(); |
There was a problem hiding this comment.
这里是会抛异常出来的，是否要像原来那样捕捉一下抛出可读的错误信息
| <View className="flex-1 items-center justify-center bg-background p-6"> | ||
| <Text className="mb-4 text-xl font-semibold text-foreground">加载失败</Text> | ||
| <Text className="mb-6 text-center text-muted-foreground"> | ||
| {this.state.error?.message || '无法加载课表数据，请检查网络连接后重试'} | ||
| </Text> | ||
| <Button onPress={this.handleReset}> | ||
| <Text>重试</Text> | ||
| </Button> | ||
| </View> |
There was a problem hiding this comment.
这里的message是不是会从handleError走一遍以toast等形式出来，可以换成统一的ErrorView组件
<ErrorView refresh={this.handleReset} />There was a problem hiding this comment.
queryFn里抛出的异常不会toast出来，要不这里改成toast吧
| // 重置组件（触发 Suspense 重新渲染） | ||
| setResetKey(prev => prev + 1); |
There was a problem hiding this comment.
只要刷新了就重新渲染，即使课程没变化，会导致页面闪烁（重绘区域比之前大）
Closes #303