Fix/pr90 whitescreen#93
Conversation
- 在tailwind配置中添加sm-md-lg-xl-2xl屏幕断点 - 更新Header组件导航栏的响应式断点从lg改为xl - 升级vite及相关插件版本
调整SubscriptionView组件中侧边栏的布局样式，添加sticky定位 移除构建过程中意外提交的dist/index.html文件
- 将订阅侧边栏、仓库卡片和开发者卡片中的emoji图标替换为lucide-react图标 - 为移动端添加可滑动的标签导航组件 - 优化平台图标的显示效果和间距 - 添加底部活动指示器和渐变遮罩效果
- 将订阅功能重构为发现频道，新增多种筛选和排序选项 - 添加AI分析辅助工具类，优化仓库分析逻辑 - 实现安全的剪贴板读写工具函数，增强兼容性 - 改进Markdown渲染器，添加代码复制和目录功能 - 优化仓库卡片组件，增加Star和AI分析操作 - 更新README模态框，支持字体大小调整和目录导航 - 移除旧的订阅相关组件和类型
- 在类型定义中添加 SubscriptionRepo 和 SubscriptionDev 等订阅相关类型 - 在应用状态中增加订阅相关的状态字段 - 实现订阅频道的初始化和迁移逻辑 - 移除未使用的 Category 类型导入
feat(ErrorBoundary): 增强错误边界组件功能 feat(DataManagementPanel): 扩展数据导入导出功能 style(index.css): 添加代码高亮和终端样式增强 feat(SubscriptionRepoCard): 改进Star操作逻辑和UI feat(ScrollToBottom): 新增滚动到底部组件 build: 添加highlight.js依赖
添加Inter字体文件，包括多种字重和字符集支持 在tailwind配置中设置Inter为默认无衬线字体 在index.html中引入字体样式文件
使用动态分块函数替代静态配置，提高代码灵活性 移除过时注释以保持配置简洁
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
refactor(DiscoveryView): 使用ref优化滚动位置存储 fix(DataManagementPanel): 修复主题和视图模式类型检查
refactor(pagination): 重构分页组件，支持服务端分页和本地分页 feat(store): 添加discoveryCurrentPage状态管理 style(tooltip): 改进排序算法提示框的定位和响应式设计 fix(scroll): 修复页面切换时的滚动位置问题 perf(discovery): 优化数据加载逻辑和性能
- 添加性能优化相关代码，包括虚拟列表、图片懒加载和性能监控 - 实现RSS趋势功能，支持从第三方源获取GitHub趋势数据 - 重构发现频道相关代码，优化类型定义和状态管理 - 添加代码分割和懒加载以提升首屏性能 - 优化排序算法提示弹窗的交互和样式 - 更新依赖项，添加esbuild用于构建优化
- 在RSS服务中添加基础URL常量并重构URL配置 - 优化RepositoryList组件的暂停/恢复和停止逻辑，使用useCallback提升性能 - 在应用状态管理中新增rssTimeRange字段并实现版本迁移
保留切换频道时的当前页码，而不是总是重置为1
- 重构项目发现模块，合并项目类型和时间范围为场景化时间范围 - 新增模态框可见性钩子，优化滚动按钮在模态框打开时的显示逻辑 - 修复暗黑模式下Markdown文本颜色问题 - 更新数据管理面板，支持更多状态的导入导出 - 调整发现页筛选器UI，优化用户体验
在 persist store 的迁移逻辑中添加了对 subscriptionRepos、subscriptionLastRefresh、subscriptionIsLoading 的完整性检查和修复，确保所有频道键（most-stars、most-forks、most-dev、trending）都存在。 🤖 Generated with [Claude Code](https://claude.com/claude-code)
📝 WalkthroughWalkthroughThis PR implements a comprehensive Discovery feature enabling repository exploration across multiple channels (trending, topic-based, search, and RSS feeds) with AI analysis capabilities. It includes major store refactoring transitioning from subscription to discovery terminology, enhanced markdown rendering with syntax highlighting and table-of-contents support, refactored component architecture with lazy-loading and error boundaries, updated build configuration (Vite ^8.0.0, added highlight.js), and new utilities for pagination, clipboard operations, and performance monitoring. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DiscoveryView
participant GitHubAPI
participant AIService
participant Store
User->>DiscoveryView: Select channel (trending/topic/search/rss)
DiscoveryView->>Store: setSelectedDiscoveryChannel()
DiscoveryView->>DiscoveryView: refreshChannel()
alt Channel Type
DiscoveryView->>GitHubAPI: getTrendingRepositories()
GitHubAPI-->>DiscoveryView: repos (without analysis)
else Topic
DiscoveryView->>GitHubAPI: getTopicRepositories()
GitHubAPI-->>DiscoveryView: repos
else Search
DiscoveryView->>GitHubAPI: searchRepositories()
GitHubAPI-->>DiscoveryView: repos
else RSS Trending
DiscoveryView->>RSSTrendingService: fetchRSSTrending()
RSSTrendingService->>GitHubAPI: getRepository() (batched)
GitHubAPI-->>RSSTrendingService: enriched repos
RSSTrendingService-->>DiscoveryView: repos
end
DiscoveryView->>Store: setDiscoveryRepos(repos)
DiscoveryView->>Store: setDiscoveryLastRefresh()
DiscoveryView-->>User: Display paginated repos
User->>DiscoveryView: Click "Analyze All" on page
DiscoveryView->>AIService: analyzeRepository() [per unanalyzed repo]
AIService->>GitHubAPI: getRepositoryReadme()
GitHubAPI-->>AIService: readme content
AIService-->>DiscoveryView: { summary, tags, platforms, category }
DiscoveryView->>Store: updateDiscoveryRepo(repoId, analysis)
DiscoveryView-->>User: Display analysis results + completion alert
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~65 minutes The pull request spans 40+ heterogeneous files across multiple architectural concerns: type system overhaul, major store refactoring with migration logic, two large new feature components (1800+ and 1200+ lines), enhanced rendering pipeline with syntax highlighting and zoom UI, new service layer for RSS/AI orchestration, and updated build infrastructure. While individual changes are comprehensible, the breadth of integration points, state management complexity, and new feature interplay require careful review across store selectors, discovery flow logic, component lifecycle handling, and backward-compatibility considerations. Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/components/Modal.tsx (1)
20-39:⚠️ Potential issue | 🟠 MajorModal lifecycle dispatches spurious
gsm:modal-closeevents and resets global body overflow unconditionally.Two concrete problems stem from this effect:
- Cleanup always fires
gsm:modal-close, even whenisOpenwasfalsethe whole time. A parent that mounts<Modal isOpen={false}>and later unmounts it will emit a close event that flipsuseModalVisibilitytofalse— even if a different modal (e.g.ReadmeModal) is currently open. This desynchronizesBackToTop/ScrollToBottomvisibility relative to actual modal state.onCloseidentity changes re-run the effect whileisOpenis stilltrue, producing aclose→openburst on the event bus and a straybody.style.overflow = 'unset'in between. If the consumer passes an inline() => ..., this happens on every render.body.style.overflow = 'unset'is applied on every cleanup regardless of whether another modal is still mounted open; overlapping modals can lose their scroll lock.Guard both dispatches and the overflow reset on the actual open state of this modal, and ideally switch
useModalVisibilityto a counter (see comment on that file).🛠️ Proposed fix
useEffect(() => { const handleEscape = (event: KeyboardEvent) => { if (event.key === 'Escape') { onClose(); } }; - if (isOpen) { - document.addEventListener('keydown', handleEscape); - // Prevent body scroll when modal is open - document.body.style.overflow = 'hidden'; - window.dispatchEvent(new CustomEvent('gsm:modal-open')); - } - - return () => { - document.removeEventListener('keydown', handleEscape); - document.body.style.overflow = 'unset'; - window.dispatchEvent(new CustomEvent('gsm:modal-close')); - }; + if (!isOpen) return; + + const previousOverflow = document.body.style.overflow; + document.addEventListener('keydown', handleEscape); + document.body.style.overflow = 'hidden'; + window.dispatchEvent(new CustomEvent('gsm:modal-open')); + + return () => { + document.removeEventListener('keydown', handleEscape); + document.body.style.overflow = previousOverflow; + window.dispatchEvent(new CustomEvent('gsm:modal-close')); + }; }, [isOpen, onClose]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Modal.tsx` around lines 20 - 39, The effect in Modal.tsx currently adds/removes listeners and mutates document.body and dispatches gsm:modal-open/gsm:modal-close unconditionally, causing spurious close events and overflow resets when this modal was never open or when onClose identity changes; update the useEffect (the handleEscape listener registration and cleanup) so that all side-effects (document.addEventListener('keydown', handleEscape), document.body.style.overflow = 'hidden', window.dispatchEvent('gsm:modal-open')) only occur when isOpen is true and, on cleanup, only dispatch gsm:modal-close and reset document.body.style.overflow for this modal if isOpen was true for this instance (i.e., guard the cleanup by the modal's open state or a ref that tracks "this modal opened"), avoid depending on changing onClose identity (stabilize with useCallback or useRef) and consider switching useModalVisibility to a counter to track multiple open modals to prevent clearing overflow when other modals remain open.src/services/aiService.ts (1)
89-256:⚠️ Potential issue | 🟠 Major
signalis not forwarded to thebackend.proxyAIRequestcalls.The abort signal is only attached to direct
fetch(...)calls (lines 129–137, 185–195, 241–250). Whenbackend.isAvailable && this.config.id, requests pass throughbackend.proxyAIRequest(id, body)without the signal, so aborting an analysis (e.g., on unmount or user cancellation) will not cancel in-flight backend-proxied requests. These can still resolve and mutate store state after the user has navigated away.The
proxyAIRequestmethod signature currently accepts only(configId: string, body: object)and needs to be extended to accept anAbortSignalparameter, or the caller should short-circuit before invoking the backend proxy:+ if (options.signal?.aborted) { + throw new DOMException('Aborted', 'AbortError'); + } if (backend.isAvailable && this.config.id) { - data = await backend.proxyAIRequest(this.config.id, requestBody) as Record<string, unknown>; + data = await backend.proxyAIRequest(this.config.id, requestBody, options.signal) as Record<string, unknown>;Apply the same fix to the Claude and Gemini branches (lines 182–183 and 237–238).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/aiService.ts` around lines 89 - 256, The backend proxy calls in requestText are not passing the AbortSignal, so update backend.proxyAIRequest to accept an optional third parameter (signal?: AbortSignal) and make its implementation forward that signal into its fetch/HTTP client; then modify all call sites inside requestText (the three places calling backend.proxyAIRequest in the OpenAI/OpenAI-responses branch, the Claude branch, and the Gemini branch) to pass options.signal as the new argument (e.g., backend.proxyAIRequest(this.config.id, requestBody, options.signal)); keep the signature backward-compatible (optional) and ensure proxyAIRequest aborts/throws when the signal is aborted.src/components/Header.tsx (1)
212-222:⚠️ Potential issue | 🟡 MinorA11y: Explore button in desktop nav has no accessible name when icon-only.
In the desktop (
xl:flex) nav, the Repositories/Releases/Settings buttons all addaria-labelandtitlewhenisTextWrappedis true (so the icon-only state remains announceable). The Explore/subscription button is the only one missing both attributes, so screen readers will announce it as an unlabeled button whenever the nav collapses to icons at ≥1300px wrap.♿ Proposed fix
<button onClick={() => setCurrentView('subscription')} + aria-label={isTextWrapped ? t('探索', 'Explore') : undefined} + title={isTextWrapped ? t('探索', 'Explore') : undefined} className={`${isTextWrapped ? 'p-2.5' : 'px-4 py-2'} rounded-lg font-medium transition-colors ${ currentView === 'subscription' ? 'bg-blue-100 text-blue-700 dark:bg-blue-900 dark:text-blue-300' : 'text-gray-700 dark:text-gray-300 hover:bg-gray-100 dark:hover:bg-gray-700' }`} > <TrendingUp className={`${isTextWrapped ? 'w-5 h-5' : 'w-4 h-4'} ${isTextWrapped ? '' : 'inline mr-2'}`} /> {!isTextWrapped && t('探索', 'Explore')} </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Header.tsx` around lines 212 - 222, Explore/subscription button in Header.tsx is missing accessible labels when icon-only; update the button rendered with setCurrentView/currentView/isTextWrapped/TrendingUp to include aria-label and title (use the same translated string from t('探索','Explore') ) when isTextWrapped is true so screen readers announce the icon-only control; implement the same conditional pattern used by the other nav buttons (add aria-label={t(...)} and title={t(...)} when isTextWrapped).src/components/ReadmeModal.tsx (1)
189-214:⚠️ Potential issue | 🟡 MinorUnbalanced event dispatches are harmless to
useModalVisibility, but good practice to fix anyway.The effect only dispatches
gsm:modal-openinside theif (isOpen)branch, but cleanup unconditionally dispatchesgsm:modal-close. TheuseModalVisibilityhook uses a simple boolean state (setIsModalOpen(true/false)), not a counter, so unbalanced closes have no practical effect—settingfalsewhen alreadyfalseis idempotent. Still, the dispatch imbalance is noisy and could surprise listeners expecting paired events, so the fix remains worthwhile for maintainability.🔧 Proposed fix
useEffect(() => { const handleEscape = (event: KeyboardEvent) => { if (event.key === 'Escape') { onClose(); } }; if (isOpen) { previousFocusRef.current = document.activeElement as HTMLElement; document.addEventListener('keydown', handleEscape); document.body.style.overflow = 'hidden'; window.dispatchEvent(new CustomEvent('gsm:modal-open')); setTimeout(() => { modalRef.current?.focus(); }, 0); } return () => { - document.removeEventListener('keydown', handleEscape); - if (document.body.style.overflow === 'hidden') { - document.body.style.overflow = 'unset'; - } - window.dispatchEvent(new CustomEvent('gsm:modal-close')); - previousFocusRef.current?.focus(); + if (isOpen) { + document.removeEventListener('keydown', handleEscape); + if (document.body.style.overflow === 'hidden') { + document.body.style.overflow = 'unset'; + } + window.dispatchEvent(new CustomEvent('gsm:modal-close')); + previousFocusRef.current?.focus(); + } }; }, [isOpen, onClose]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ReadmeModal.tsx` around lines 189 - 214, The effect dispatches 'gsm:modal-open' only when isOpen is true but always dispatches 'gsm:modal-close' in cleanup; update the useEffect in ReadmeModal to track whether the modal was actually opened (e.g., set a local boolean opened = false, set opened = true inside the if (isOpen) branch) and then only dispatch window.dispatchEvent(new CustomEvent('gsm:modal-close')) in the cleanup when opened is true; ensure the same opened flag governs restoring document.body.style.overflow and focusing previousFocusRef so all open-related teardown runs only if the modal was opened (references: useEffect, handleEscape, previousFocusRef, modalRef, isOpen, onClose, useModalVisibility).src/store/useAppStore.ts (1)
1165-1242:⚠️ Potential issue | 🟠 MajorBump the persisted store version to match the migration target.
Line 1167 still configures Zustand persist with
version: 6, whilemigratetargets version 8. Users already on version 6 won’t run the version 7/8 migration path, leaving stale discovery/trending state that can break the new UI.🐛 Proposed fix
+const CURRENT_STORE_VERSION = 8; + const store = create<AppState & AppActions>()( persist( (set) => ({ // Initial state @@ { name: 'github-stars-manager', - version: 6, + version: CURRENT_STORE_VERSION, @@ migrate: (persistedState, fromVersion) => { - const CURRENT_STORE_VERSION = 8; const state = persistedState as PersistedAppState | undefined;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/store/useAppStore.ts` around lines 1165 - 1242, The persist configuration for the store (name 'github-stars-manager') still sets version: 6 while the migrate function and CURRENT_STORE_VERSION constant target version 8; update the persist config's version field to 8 so Zustand will run the v7/v8 migration path (look for the persist block containing version, createJSONStorage, partialize and the migrate function with CURRENT_STORE_VERSION = 8) to ensure discovery/trending state migrations run for existing users.src/services/githubApi.ts (2)
391-393:⚠️ Potential issue | 🟡 MinorKeep
forksin sync when enriching RSS trending repos.The initial object sets both
forks_countandforks, but the API enrichment only updatesforks_count. Cards readingforkscan still show0.🐛 Proposed fix
r.stargazers_count = data.stargazers_count ?? r.stargazers_count; r.forks_count = data.forks_count ?? r.forks_count; + r.forks = data.forks_count ?? r.forks; r.language = data.language;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/githubApi.ts` around lines 391 - 393, The enrichment updates r.stargazers_count, r.forks_count and r.language but forgets to keep the duplicate r.forks field in sync; update the enrichment logic (the lines touching r.stargazers_count / r.forks_count / r.language) to also set r.forks = data.forks_count ?? r.forks so the UI that reads r.forks will reflect the API value (ensure any type conversion needed matches existing repo object shape).
153-155:⚠️ Potential issue | 🟠 MajorPropagate aborts instead of converting them to an empty README.
Returning
''forAbortErrormakes callers continue as if README fetch simply failed, which weakens the new analysis cancellation flow.🐛 Proposed fix
} catch (error) { + if ((error as Error)?.name === 'AbortError') { + throw error; + } console.warn(`Failed to fetch README for ${owner}/${repo}:`, error); return ''; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/githubApi.ts` around lines 153 - 155, In the catch block that currently logs "Failed to fetch README for ${owner}/${repo}" and returns '', detect AbortError and re-throw it instead of returning an empty string; for example check error.name === 'AbortError' (or use a helper isAbortError) inside the catch in the README-fetching function in src/services/githubApi.ts and if true do throw error, otherwise keep the existing warn and return ''. This preserves cancellation flow while still handling non-abort failures.
🟡 Minor comments (13)
src/components/VirtualList.tsx-112-124 (1)
112-124:⚠️ Potential issue | 🟡 MinorInfinite-load trigger fires on short lists and may spam
onLoadMore.When
totalHeight - fixedItemHeight * 2is less thancontainerHeight, the threshold is satisfied atscrollTop=0. Because this runs insidehandleScroll, the first user scroll (of any amount, even a single-pixel overscroll) will immediately fireonLoadMore, and each subsequent scroll event keeps re-firing if the new batch is also short. TheloadingRefguard prevents re-entry during one in-flight load, but not repeated loads after each resolves.Also,
thresholdshould probably be computed from the bottom margin (e.g.totalHeight - containerHeight - buffer) rather thantotalHeight - 2 * fixedItemHeight, andfixedItemHeightdefaults to50whenitemHeightis a function, which makes the threshold meaningless for variable-height lists.Consider:
const threshold = Math.max(0, totalHeight - containerHeight - overscan * fixedItemHeight);plus a check thattotalHeight > containerHeightbefore auto-loading, to avoid triggering on initially-short lists.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/VirtualList.tsx` around lines 112 - 124, The infinite-load trigger in handleScroll fires on initially-short lists because threshold = totalHeight - fixedItemHeight * 2 can be <= containerHeight; update handleScroll to first check that totalHeight > containerHeight before attempting auto-load, and compute threshold as Math.max(0, totalHeight - containerHeight - overscan * fixedItemHeight) (or equivalent using your overscan variable) so the load only triggers when the user scrolls near the bottom of a longer list; keep the loadingRef and setIsLoading guards but ensure you only call onLoadMore when totalHeight > containerHeight and scrollBottom >= threshold to avoid repeated immediate loads for short result sets (affecting symbols: handleScroll, loadingRef, setIsLoading, onLoadMore, totalHeight, containerHeight, fixedItemHeight, overscan).src/components/ErrorBoundary.tsx-28-29 (1)
28-29:⚠️ Potential issue | 🟡 MinorCopy button has no user feedback —
strings.copiedis defined but never rendered.
handleCopyErrorresolves silently on both success and failure, and the localizedcopiedstring (line 29) is declared but never used. Users clicking “Copy Error Info” get zero confirmation that anything happened, which is particularly bad in the error-boundary context where reliability signals matter. Additionally, on failure (e.g., insecure context / permission denied) the error is only logged to console, so the user cannot tell the copy didn't work.Consider wiring
strings.copiedinto a transient state and also surfacing a fallback message on failure (or reusingsafeWriteTextfromsrc/utils/clipboardUtils.tswhich this PR introduces).💡 Suggested wiring
- interface State { + interface State { hasError: boolean; error: Error | null; errorInfo: React.ErrorInfo | null; showDetails: boolean; + copyStatus: 'idle' | 'copied' | 'failed'; } @@ - this.state = { hasError: false, error: null, errorInfo: null, showDetails: false }; + this.state = { hasError: false, error: null, errorInfo: null, showDetails: false, copyStatus: 'idle' }; @@ handleCopyError = async () => { const { error, errorInfo } = this.state; const errorText = [ /* ... */ ].join('\n'); try { await navigator.clipboard.writeText(errorText); + this.setState({ copyStatus: 'copied' }); + setTimeout(() => this.setState({ copyStatus: 'idle' }), 2000); } catch (e) { console.error('Failed to copy:', e); + this.setState({ copyStatus: 'failed' }); + setTimeout(() => this.setState({ copyStatus: 'idle' }), 2000); } }; @@ - {strings.copyError} + {this.state.copyStatus === 'copied' ? strings.copied : strings.copyError}Also applies to: 61-78, 104-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ErrorBoundary.tsx` around lines 28 - 29, The copy button currently calls handleCopyError but never renders the localized strings.copied or indicates failure; update the ErrorBoundary component to use a transient piece of state (e.g., isCopied / copyError) and wire strings.copied into the UI so a success toast/inline message appears briefly after handleCopyError resolves; change handleCopyError to call the new safeWriteText from src/utils/clipboardUtils.ts (for robust failure handling), set isCopied=true on success and set a copyError message (using a localized fallback) on failure, start a timer to clear the transient state after a short delay, and ensure the copy button/area renders the success (strings.copied) or failure message accordingly (affecting the functions/methods named handleCopyError and the ErrorBoundary component state).src/utils/pagination.ts-20-32 (1)
20-32:⚠️ Potential issue | 🟡 Minor
hasNextPage/hasPreviousPageuse the unclampedcurrentPage.If a caller passes a currentPage outside
[1, totalPages](e.g. stale persisted state during rehydration), the returned state becomes inconsistent:currentPageis clamped while the boundary flags reflect the raw value (e.g.currentPage: 1, hasPreviousPage: truewhen input was5andtotalPagesis3). Compute the flags from the clamped value.): PaginationState { + const clamped = Math.max(1, Math.min(currentPage, totalPages || 1)); return { - currentPage: Math.max(1, Math.min(currentPage, totalPages || 1)), + currentPage: clamped, totalPages: Math.max(0, totalPages), totalCount: Math.max(0, totalCount), - hasNextPage: currentPage < totalPages, - hasPreviousPage: currentPage > 1, + hasNextPage: clamped < totalPages, + hasPreviousPage: clamped > 1, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/pagination.ts` around lines 20 - 32, The pagination flags are computed from the uncapped currentPage causing inconsistent state; in calculatePaginationState compute and reuse a clamped page value (e.g., clampedCurrentPage = Math.max(1, Math.min(currentPage, totalPages || 1))) and base hasNextPage and hasPreviousPage on that clampedCurrentPage instead of the raw currentPage, while keeping totalPages and totalCount clamped as shown.src/utils/usePagination.ts-164-194 (1)
164-194:⚠️ Potential issue | 🟡 MinorError messages are hard-coded in Chinese; app supports
zh|en.
validatePageInputreturns Chinese strings (e.g.请输入页码,页码不能超过${maxPage}) regardless of the current language. English users will see untranslated error text in the pagination jump-to input. Accept alanguage(or translated message bag) argument, or return an error code and let the caller translate.-export function validatePageInput( - input: string, - minPage: number, - maxPage: number -): { isValid: boolean; page: number | null; error: string | null } { +export type PageInputErrorCode = 'empty' | 'not_number' | 'nan' | 'below_min' | 'above_max'; +export function validatePageInput( + input: string, + minPage: number, + maxPage: number +): { isValid: boolean; page: number | null; error: PageInputErrorCode | null } { const trimmed = input.trim(); - if (!trimmed) return { isValid: false, page: null, error: '请输入页码' }; - if (!/^\d+$/.test(trimmed)) return { isValid: false, page: null, error: '请输入有效数字' }; + if (!trimmed) return { isValid: false, page: null, error: 'empty' }; + if (!/^\d+$/.test(trimmed)) return { isValid: false, page: null, error: 'not_number' }; const page = parseInt(trimmed, 10); - if (isNaN(page)) return { isValid: false, page: null, error: '请输入有效页码' }; - if (page < minPage) return { isValid: false, page: null, error: `页码不能小于${minPage}` }; - if (page > maxPage) return { isValid: false, page: null, error: `页码不能超过${maxPage}` }; + if (isNaN(page)) return { isValid: false, page: null, error: 'nan' }; + if (page < minPage) return { isValid: false, page: null, error: 'below_min' }; + if (page > maxPage) return { isValid: false, page: null, error: 'above_max' }; return { isValid: true, page, error: null }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/usePagination.ts` around lines 164 - 194, The function validatePageInput currently returns hard-coded Chinese error strings; update it to be locale-aware by either adding a language/locale parameter (e.g., language: 'zh' | 'en' or a messages bag) to validatePageInput or by changing its return shape to use error codes (e.g., errorCode: 'EMPTY' | 'INVALID_NUMBER' | 'OUT_OF_RANGE_MIN' | 'OUT_OF_RANGE_MAX') instead of human-readable text; ensure all existing string literals like '请输入页码', '请输入有效数字', '请输入有效页码', `页码不能小于${minPage}`, `页码不能超过${maxPage}` are replaced accordingly and update callers to map codes to localized messages or pass a messages object when invoking validatePageInput.src/components/SortAlgorithmTooltip.tsx-180-236 (1)
180-236:⚠️ Potential issue | 🟡 MinorAccessibility:
aria-modal="true"is inappropriate for a non-modal popover.This component behaves as an info popover—it doesn't trap focus, doesn't move focus on open, and is dismissed by an outside click or Escape key. Declaring
role="dialog" aria-modal="true"tells assistive technology that the rest of the page is inert, which is false and can confuse screen-reader users. Removearia-modal="true"and addaria-labelledbyto properly label the dialog.Suggested fix
role="dialog" - aria-modal="true" + aria-labelledby={`sort-algo-title-${channelId}`} >…and add
id={sort-algo-title-${channelId}}to the<h4>element.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SortAlgorithmTooltip.tsx` around lines 180 - 236, Remove the incorrect aria-modal usage and properly label the popover: in the createPortal block on the root div (where role="dialog" is set) delete aria-modal="true" and add aria-labelledby={`sort-algo-title-${channelId}`}; then give the <h4> the matching id prop id={`sort-algo-title-${channelId}`} (the title element rendered inside the SortAlgorithmTooltip createPortal block), ensuring you reference the existing channelId variable so assistive tech reads the title correctly.vite.config.ts-15-23 (1)
15-23:⚠️ Potential issue | 🟡 Minor
/api/rssproxy is unreachable — services bypass it with hardcoded absolute URLs.
src/services/rssTrendingService.ts:11andsrc/services/githubApi.ts:316both fetch directly fromhttps://mshibanami.github.io/GitHubTrendingRSSwith hardcoded URLs, so this proxy block is never invoked. Additionally,server.proxyonly applies to the Vite dev server, not production builds or Electron bundles.Either remove the unused proxy block, or refactor the service to use
/api/rssin dev for CORS mitigation:const RSS_BASE_URL = import.meta.env.DEV ? '/api/rss' : 'https://mshibanami.github.io/GitHubTrendingRSS';Then verify the production/Electron path still works without the proxy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vite.config.ts` around lines 15 - 23, The Vite dev proxy for '/api/rss' is never used because src/services/rssTrendingService.ts (line ~11) and src/services/githubApi.ts (line ~316) call the absolute 'https://mshibanami.github.io/GitHubTrendingRSS' URL directly and server.proxy only affects the Vite dev server; fix by updating those services to use a conditional base URL (e.g., RSS_BASE_URL) that resolves to '/api/rss' in dev (import.meta.env.DEV) and to the upstream URL in production/Electron, or delete the unused proxy block in vite.config.ts if you prefer not to support the dev-proxy path; after change, run dev and production/Electron builds to verify both routes work.public/fonts/inter.css-1-53 (1)
1-53:⚠️ Potential issue | 🟡 Minor
public/fonts/inter.cssis never loaded — dead asset.
index.htmllinks Inter only fromhttps://fonts.googleapis.com/css2?family=Inter:...with no reference to/fonts/inter.css, so these@font-facerules are never applied. Either:
- Add
<link rel="stylesheet" href="/fonts/inter.css" />inindex.htmlto self-host Inter and remove the Google Fonts link, or- Delete
public/fonts/inter.cssto avoid shipping unused CSS.Both
./inter-latin.woff2and./inter-latin-ext.woff2are present inpublic/fonts/, so self-hosting is feasible if intended by the Tailwindfont-sanschange.Stylelint also flags the
font-family: 'Inter'quotes on lines 2, 11, 20, 29, 38, 47 (rulefont-family-name-quotes). Drop the quotes or adjust the Stylelint config:- font-family: 'Inter'; + font-family: Inter;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@public/fonts/inter.css` around lines 1 - 53, The inter.css file defines `@font-face` for font-family Inter but is never loaded; either add a <link rel="stylesheet" href="/fonts/inter.css" /> to index.html and remove the Google Fonts link so the self-hosted src URLs (./inter-latin.woff2 and ./inter-latin-ext.woff2) are used, or delete public/fonts/inter.css to avoid shipping dead CSS; additionally, update the font-family declarations in inter.css from 'Inter' to Inter (remove quotes) to satisfy Stylelint's font-family-name-quotes rule (references: public/fonts/inter.css, font-family Inter, src entries inter-latin.woff2 and inter-latin-ext.woff2, index.html link).package.json-23-23 (1)
23-23:⚠️ Potential issue | 🟡 MinorRemove redundant
esbuilddevDependency and enforce Node.js floor viaenginesfield.Vite 8 ships with Rolldown as its bundler and includes esbuild internally, so the
"esbuild": "^0.28.0"devDependency added at line 43 is unused—esbuild is not imported or called anywhere in the codebase, only referenced as theminifystrategy string invite.config.tsline 36. Remove it from package.json.Additionally, add an
enginesfield to enforce Node.js 20.19+ or 22.12+ before install/build, as Vite 8 requires it. This ensures CI runners and the Electron build pipeline (scripts/build-desktop.js) fail fast rather than silently if a lower Node version is used.The
@vitejs/plugin-react@^6update is compatible—vite.config.tsline 8 callsreact()with no babel options, so no breakage from Babel removal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 23, Remove the unused "esbuild" devDependency from package.json (it's not imported anywhere and Vite 8 includes esbuild/uses Rollup internally—see the minify strategy string referenced in vite.config.ts) and add an "engines" field to package.json to enforce a Node.js floor (e.g., "node": ">=20.19.0") so CI and the Electron build script fail fast on older Node versions; update devDependencies to drop "esbuild" and add the engines object at the top-level of package.json.src/utils/performanceMonitor.ts-32-42 (1)
32-42:⚠️ Potential issue | 🟡 MinorRecord exported render measurements as
render, notcustom.
measureRender()currently goes throughstartMeasure(), which hardcodes the metric type tocustom, so render metrics won’t appear in the render summary/report.Proposed fix
- startMeasure(name: string): () => number { + startMeasure(name: string, type: MetricType = 'custom'): () => number { if (!this.enabled) { return () => 0; } const startTime = performance.now(); return () => { const duration = performance.now() - startTime; - this.addMetric(name, 'custom', duration); + this.addMetric(name, type, duration); return duration; }; }export const measureRender = (componentName: string) => { - return performanceMonitor.startMeasure(`render:${componentName}`); + return performanceMonitor.startMeasure(`render:${componentName}`, 'render'); };Also applies to: 151-153
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/performanceMonitor.ts` around lines 32 - 42, The startMeasure function currently hardcodes the metric type to 'custom', causing render timings to be miscategorized; update startMeasure(name: string) to accept an optional metricType: string = 'custom' (or overload) and use that metricType when calling this.addMetric(name, metricType, duration), then update measureRender (or any caller that records render metrics) to call startMeasure(name, 'render') so exported render measurements are recorded with type 'render' instead of 'custom'.src/components/SubscriptionRepoCard.tsx-390-398 (1)
390-398:⚠️ Potential issue | 🟡 MinorStop GitHub link clicks from opening the README modal.
This anchor is inside a clickable card; without stopping propagation, clicking “Open on GitHub” can also trigger
handleCardClick.Proposed fix
<a href={repo.html_url} target="_blank" rel="noopener noreferrer" + onClick={(e) => e.stopPropagation()} className="flex items-center justify-center w-8 h-8 rounded-lg bg-gray-100 text-gray-500 dark:bg-gray-700 dark:text-gray-400 hover:bg-gray-200 dark:hover:bg-gray-600 transition-colors" title={t('在GitHub打开', 'Open on GitHub')} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SubscriptionRepoCard.tsx` around lines 390 - 398, The GitHub anchor inside SubscriptionRepoCard.tsx is inside a clickable card and needs to stop the card's click handler (handleCardClick) from firing; update the anchor element (the one rendering ExternalLink) to call e.stopPropagation() in its onClick handler (and consider handling keyboard activation as appropriate) so clicks on the "Open on GitHub" link only open the external URL and do not also trigger handleCardClick.src/components/MarkdownRenderer.tsx-692-699 (1)
692-699:⚠️ Potential issue | 🟡 MinorExtract heading text recursively before looking up IDs.
children.join('')andString(children)turn inline elements like## Using `foo`into[object Object], so TOC/hash links won’t get matching IDs for formatted headings.♻️ Proposed fix
+ const extractTextContent = (node: React.ReactNode): string => { + return React.Children.toArray(node) + .map((child) => { + if (typeof child === 'string' || typeof child === 'number') { + return String(child); + } + if (React.isValidElement<{ children?: React.ReactNode }>(child)) { + return extractTextContent(child.props.children); + } + return ''; + }) + .join(''); + }; + const getHeadingId = (children: React.ReactNode): string | undefined => { - const text = typeof children === 'string' - ? children - : Array.isArray(children) - ? children.join('') - : String(children); + const text = extractTextContent(children); return headingIds?.get(text); };Also applies to: 709-720
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/MarkdownRenderer.tsx` around lines 692 - 699, The getHeadingId helper currently builds heading text using children.join('') and String(children), which yields "[object Object]" for inline React elements; replace that logic in getHeadingId (and the similar block at 709-720) with a recursive ReactNode-to-string extractor that walks elements, arrays, fragments and returns concatenated text content, then use that normalized text to look up headingIds.get(text); ensure it handles strings, numbers, booleans, null/undefined, arrays, and React elements with props.children so formatted inline nodes like <code> or <em> produce correct IDs.src/components/OptimizedImage.tsx-154-163 (1)
154-163:⚠️ Potential issue | 🟡 MinorCatch failed component prefetches.
importFn()returns a promise; without a catch, failed prefetches can surface as unhandled promise rejections.🛡️ Proposed fix
prefetchComponent: (importFn: () => Promise<unknown>): void => { + const runPrefetch = () => { + void importFn().catch((error) => { + console.warn('Failed to prefetch component:', error); + }); + }; + if (typeof window !== 'undefined' && 'requestIdleCallback' in window) { - requestIdleCallback(() => { - importFn(); - }); + requestIdleCallback(runPrefetch); } else { - setTimeout(() => { - importFn(); - }, 100); + setTimeout(runPrefetch, 100); } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/OptimizedImage.tsx` around lines 154 - 163, prefetchComponent currently calls importFn() without handling its returned promise, causing potential unhandled promise rejections; update the prefetchComponent implementation (the prefetchComponent arrow function that accepts importFn) to call importFn().catch(...) in both the requestIdleCallback and setTimeout branches and handle errors (e.g., swallow or log via console.warn or a provided logger) so failed prefetches are safely consumed.src/components/MarkdownRenderer.tsx-305-311 (1)
305-311:⚠️ Potential issue | 🟡 MinorPrevent duplicate opens for Ctrl/Cmd-clicked linked images.
Line 309 manually opens the link, but the enclosing anchor can still perform its default Ctrl/Cmd-click behavior. Add
preventDefault()andstopPropagation()beforewindow.open().🐛 Proposed fix
if (e.ctrlKey || e.metaKey) { + e.preventDefault(); + e.stopPropagation(); window.open(parentLinkHref, '_blank', 'noopener,noreferrer'); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/MarkdownRenderer.tsx` around lines 305 - 311, In handleImageClick (the React.useCallback handling image clicks) when isInsideLink and parentLinkHref are true, call e.preventDefault() and e.stopPropagation() before calling window.open(parentLinkHref, '_blank', 'noopener,noreferrer') so the enclosing anchor does not also perform its default Ctrl/Cmd-click behavior; ensure the sequence is: preventDefault, stopPropagation, then window.open, and return afterwards.
🧹 Nitpick comments (6)
src/services/updateService.ts (1)
8-11: RenameREPO_OWNERand harden URL parsing.The constant is named
REPO_OWNERbut actually containsowner/repo(AmintaCCCP/GithubStarsManager). More importantly,split('/').slice(-2).join('/')silently breaks if anyone later appends a trailing slash toPROJECT_REPO_URL(you'd getGithubStarsManager/and a 404 on update checks). Parsing viaURLis safer and self-documenting.♻️ Proposed fix
-import { PROJECT_REPO_URL } from '../constants/project'; - -const REPO_OWNER = PROJECT_REPO_URL.split('/').slice(-2).join('/'); -const VERSION_INFO_URL = `https://raw.githubusercontent.com/${REPO_OWNER}/main/versions/version-info.xml`; +import { PROJECT_REPO_URL } from '../constants/project'; + +const { pathname } = new URL(PROJECT_REPO_URL); +const OWNER_AND_REPO = pathname.replace(/^\/+|\/+$/g, ''); +const VERSION_INFO_URL = `https://raw.githubusercontent.com/${OWNER_AND_REPO}/main/versions/version-info.xml`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/updateService.ts` around lines 8 - 11, RENAME the misleading REPO_OWNER constant and harden parsing: replace REPO_OWNER with a clearly named constant like REPO_FULL_NAME (or REPO_PATH) and compute it by creating a new URL(PROJECT_REPO_URL). Use url.pathname.split('/').filter(Boolean).slice(-2).join('/') to reliably extract the "owner/repo" segments (this avoids trailing-slash bugs) and then build VERSION_INFO_URL using that sanitized REPO_FULL_NAME; reference PROJECT_REPO_URL, REPO_OWNER (old name), REPO_FULL_NAME (new name), and VERSION_INFO_URL when making the change.src/utils/clipboardUtils.ts (1)
76-116:execCommandfallback is skipped whenwriteTextthrows at runtime.The fallback only runs when
support.writeTextisfalse. Ifnavigator.clipboard.writeTextis supported but rejects (e.g., document not focused, permission denied, iframe withoutclipboard-write), the function returns a generic error without attempting the legacy fallback — defeating half of the “safe” promise. Also, the originalerris never logged, so the real failure reason is invisible in DevTools when debugging user reports.♻️ Try the fallback when the native API rejects
try { await navigator.clipboard.writeText(text); return { success: true }; } catch (err) { + console.warn('[clipboard] writeText failed, trying execCommand fallback:', err); + try { + const textarea = document.createElement('textarea'); + textarea.value = text; + textarea.style.position = 'fixed'; + textarea.style.left = '-999999px'; + textarea.style.top = '-999999px'; + document.body.appendChild(textarea); + textarea.focus(); + textarea.select(); + const ok = document.execCommand('copy'); + document.body.removeChild(textarea); + if (ok) return { success: true }; + } catch { /* fall through */ } return { success: false, error: getClipboardErrorMessage('write'), }; }Consider also logging
errinside the first fallback branch (line 97) for the same diagnostic reason.Note:
document.execCommand('copy')is deprecated but still widely supported as a fallback. Please confirm this is acceptable as a Chrome 80+/Firefox 75+ baseline (perErrorBoundarybrowser hint).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/clipboardUtils.ts` around lines 76 - 116, The safeWriteText function currently only tries the execCommand fallback when checkClipboardSupport().writeText is false, but must also attempt that fallback if navigator.clipboard.writeText exists but rejects; modify safeWriteText so that the try { await navigator.clipboard.writeText(text) } catch(err) block then calls the same execCommand fallback path (the textarea creation/select/execCommand/remove sequence used earlier) before returning failure, and include/log the caught err (and include its message in the returned error or pass it to processLogger/console) using identifiers safeWriteText, navigator.clipboard.writeText, execCommand fallback, and getClipboardErrorMessage so real runtime errors are visible and the legacy fallback is attempted when native API rejects.src/utils/pagination.ts (1)
50-118: Duplicated pagination helpers acrosspagination.tsandusePagination.ts.
getPageRangehere andgetPageNumbersinsrc/utils/usePagination.tsare near-identical (only thetotalPages <= 0/1edge differs), and the page-input validation exists in two flavors (isValidPageNumber+sanitizePageInputvsvalidatePageInput). This will drift over time. Consider havingusePaginationconsume these helpers (or vice versa), exporting a single source of truth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/pagination.ts` around lines 50 - 118, The duplicate pagination logic should be consolidated so there's a single source of truth: move/keep canonical helpers (getPageRange, isValidPageNumber, sanitizePageInput) in src/utils/pagination.ts and have usePagination.ts import and reuse them instead of reimplementing getPageNumbers/validatePageInput; reconcile the edge-case behavior (decide whether totalPages<=0 or <=1 returns [] and adjust getPageRange/getPageNumbers accordingly), export the helpers from pagination.ts, remove the duplicated implementations from usePagination.ts, and update any call sites to import the canonical functions.index.html (1)
10-12: Optional: self-host Inter for privacy/offline resilience.Loading Inter from
fonts.googleapis.comat runtime sends every page load (client IP + UA) to Google, which has been held to be a GDPR concern in some jurisdictions, and also makes the initial render dependent on Google's CDN. For a self-contained desktop-style app, consider bundling Inter via@fontsource/inter(or similar) so the fonts are served from the same origin.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@index.html` around lines 10 - 12, The page currently loads Inter via external <link> tags to fonts.googleapis.com and fonts.gstatic.com; replace that by self-hosting Inter: add the package (e.g., `@fontsource/inter`) to dependencies, import the desired weights in your app entry or global stylesheet (e.g., import "@fontsource/inter/400.css", etc.), remove the external <link rel="preconnect"> and Google Fonts <link> tags, and ensure your build/static assets include the font files so fonts are served from your origin for privacy/offline resilience.src/utils/usePagination.ts (1)
1-114: A few hygiene items in this hook.
- File location: this is a React hook (
useCallback,useMemo) but it lives insrc/utils/. By convention it belongs insrc/hooks/alongsideuseModalVisibility, and lint rules keyed offsrc/hooks/*for the hooks plugin will not apply here.console.logon everygoToNextPage/goToPreviousPage/goToPage/goToFirstPage/goToLastPageships to production and spams the console in paginated views. Use a gated logger or drop these.shouldLoadPage = pageItems.length === 0 && !isLoading(L66-68) is true whenever the current slice happens to be empty — including legitimate "no results" states (totalCount > 0but server returned fewer rows than expected). If a consumer drives auseEffectoff this flag it can refetch repeatedly. Consider gating ontotalCount === 0 ? false : pageItems.length === 0 && !isLoading, or tying it explicitly to "page index beyond what's been fetched".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/usePagination.ts` around lines 1 - 114, Move this hook file from src/utils to src/hooks so it lives alongside other React hooks (e.g., useModalVisibility) and picks up hooks-plugin linting; remove the console.log calls inside goToNextPage, goToPreviousPage, goToPage, goToFirstPage and goToLastPage (or replace them with a gated logger if you have an existing logging utility) so logs do not ship to production; and change the shouldLoadPage calculation in usePagination to not trigger when totalCount is zero — e.g., compute shouldLoadPage by first checking totalCount (return false when totalCount === 0) else evaluate pageItems.length === 0 && !isLoading so consumers don't repeatedly refetch on legitimate empty-page/no-results states (update usePagination, pageItems, and shouldLoadPage references accordingly).src/components/ScrollToBottom.tsx (1)
57-73: Effect attaches listeners only once; a late-mountingscrollContainerRef.currentsilently disables the button.
useEffectreadsscrollContainerRef.currentsynchronously at first run and bails out (if (!container) return;) when it'snull. Since refs aren't reactive, the effect never re-runs when the ref becomes populated, so the scroll listener is never attached andisVisiblestaysfalseforever.This works today only because
DiscoveryViewassigns the ref before this component mounts. It's fragile if the scroll container is ever rendered conditionally (e.g., behind a loading state or Suspense boundary).Two low-risk options:
- Attach listeners to
windowunconditionally and read the container insidecheckVisibility, or- Re-check on each render via a callback ref instead of a
RefObject.Also, the local
containerRefmirror is unnecessary — thecontainerbinding captured in the effect closure is already stable for cleanup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ScrollToBottom.tsx` around lines 57 - 73, The effect in ScrollToBottom reads scrollContainerRef.current once and returns early if null, so when the container mounts later the scroll listener is never attached and isVisible remains false; fix by removing the early return and either (a) always attach window listeners and have checkVisibility read scrollContainerRef.current each time, or (b) switch to a callback ref (instead of scrollContainerRef RefObject) so you can attach/detach the container listener when the DOM node is set; also drop the redundant containerRef mirror and ensure cleanup removes the listener from the actual container node captured at attach time and from window (use checkVisibility as the handler in all add/remove calls).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8d17565a-63a5-4dac-8abc-4686e72c5926
⛔ Files ignored due to path filters (4)
dist/index.htmlis excluded by!**/dist/**package-lock.jsonis excluded by!**/package-lock.jsonpublic/fonts/inter-latin-ext.woff2is excluded by!**/*.woff2public/fonts/inter-latin.woff2is excluded by!**/*.woff2
📒 Files selected for processing (42)
index.htmlpackage.jsonpublic/fonts/inter.csssrc/App.tsxsrc/components/BackToTop.tsxsrc/components/DiscoverySidebar.tsxsrc/components/DiscoveryView.tsxsrc/components/ErrorBoundary.tsxsrc/components/Header.tsxsrc/components/LoginScreen.tsxsrc/components/MarkdownRenderer.tsxsrc/components/Modal.tsxsrc/components/OptimizedImage.tsxsrc/components/ReadmeModal.tsxsrc/components/RepositoryCard.tsxsrc/components/RepositoryList.tsxsrc/components/ScrollToBottom.tsxsrc/components/SortAlgorithmTooltip.tsxsrc/components/SubscriptionDevCard.tsxsrc/components/SubscriptionRepoCard.tsxsrc/components/SubscriptionSidebar.tsxsrc/components/SubscriptionView.tsxsrc/components/VirtualList.tsxsrc/components/settings/DataManagementPanel.tsxsrc/components/settings/GeneralPanel.tsxsrc/constants/project.tssrc/hooks/useModalVisibility.tssrc/hooks/useStoreSelectors.tssrc/index.csssrc/services/aiAnalysisHelper.tssrc/services/aiService.tssrc/services/githubApi.tssrc/services/rssTrendingService.tssrc/services/updateService.tssrc/store/useAppStore.tssrc/types/index.tssrc/utils/clipboardUtils.tssrc/utils/pagination.tssrc/utils/performanceMonitor.tssrc/utils/usePagination.tstailwind.config.jsvite.config.ts
💤 Files with no reviewable changes (3)
- src/components/SubscriptionSidebar.tsx
- src/components/SubscriptionDevCard.tsx
- src/components/SubscriptionView.tsx
| const handleTrendingParamsChange = useCallback((params: Partial<TrendingParams>) => { | ||
| setTrendingParams(params); | ||
| refreshChannel('trending', 1, false); | ||
| }, [setTrendingParams, refreshChannel]); |
There was a problem hiding this comment.
Refresh with the next filter/query state, not the previous render’s closure.
These handlers update store state and immediately call refreshChannel, but refreshChannel reads memoized values from the previous render. For example, the first search after typing can run with the old discoverySearchQuery; sort/filter changes can fetch old params. Also reset discoveryCurrentPage when fetching page 1 to keep pagination UI aligned with the data.
🐛 Proposed direction
const handleSearch = useCallback(() => {
if (selectedDiscoveryChannel === 'search') {
setDiscoverySearchQuery(searchInput);
+ setDiscoveryCurrentPage('search', 1);
- refreshChannel('search', 1, false);
+ // Ensure refreshChannel reads latest values from the store, or pass `searchInput`
+ // as an explicit override into the request builder.
+ queueMicrotask(() => refreshChannel('search', 1, false));
}
- }, [selectedDiscoveryChannel, searchInput, setDiscoverySearchQuery, refreshChannel]);
+ }, [selectedDiscoveryChannel, searchInput, setDiscoverySearchQuery, setDiscoveryCurrentPage, refreshChannel]);A cleaner fix is to have refreshChannel read request parameters from useAppStore.getState() at request time, or accept explicit parameter overrides from each handler.
Also applies to: 1393-1398, 1550-1554, 1684-1688
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/DiscoveryView.tsx` around lines 1222 - 1225, The handlers like
handleTrendingParamsChange call setTrendingParams then immediately call
refreshChannel which reads stale memoized values; update refreshChannel to
derive its request params at call time from useAppStore.getState() (or add an
explicit overrides parameter so callers can pass the new params), and ensure
callers such as handleTrendingParamsChange, the handlers at lines ~1393 and
~1550 and ~1684, pass the correct overrides or let refreshChannel pull current
state; additionally, when fetching page 1 reset discoveryCurrentPage to 1 in the
store so pagination UI stays in sync (e.g., set discoveryCurrentPage = 1
before/when calling refreshChannel for page 1).
| code: ({ className, children, ...props }) => { | ||
| const isInline = !className; | ||
| const match = /language-(\w+)/.exec(className || ''); | ||
| const language = match ? match[1] : ''; | ||
|
|
||
| return isInline ? ( | ||
| <code className="px-1.5 py-0.5 bg-gray-100 dark:bg-gray-700 text-gray-800 dark:text-gray-200 rounded text-xs font-mono" {...props}> | ||
| {children} | ||
| </code> | ||
| ) : ( | ||
| <code className="text-xs font-mono text-gray-800 dark:text-gray-200" {...props}> | ||
| <CodeBlock className={className} language={language}> | ||
| {children} | ||
| </code> | ||
| </CodeBlock> | ||
| ); | ||
| }, | ||
| pre: ({ children }) => ( | ||
| <pre className="bg-gray-100 dark:bg-gray-800 p-3 rounded-lg overflow-x-auto my-3"> | ||
| {children} | ||
| </pre> | ||
| ), | ||
| pre: ({ children }) => <>{children}</>, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
react-markdown current version custom code component distinguish inline code from fenced code block without language
💡 Result:
The current version of react-markdown is 10.1.0 (released March 7, 2025). In versions 9.0.0+, the inline prop is no longer passed to custom code components (it was available in v8 and earlier). Fenced code blocks (with language or not) render as
...... directly in paragraphs. To distinguish in a custom code component without language info: 1. Check if children (stringified) contains newlines: String(children).includes('\n') — true for blocks, false for inline. 2. Check className: present (e.g., ['language-js']) for fenced blocks, undefined/null for inline. 3. Use node (HAST element): check if node.parent?.tagName === 'pre' for blocks. 4. For full control, use a rehype plugin with unist-util-visit to add a custom property (e.g., inline: true/false) to code nodes based on parent, then read it in the component. Example custom code component (common pattern): code({node, className, children, ...props}) { const match = /language-(\w+)/.exec(className?.[0] || ''); const isBlock = !!match || String(children).includes('\n'); if (isBlock && match) { return <SyntaxHighlighter language={match[1]} {...props}>{String(children).replace(/\n$/, '')}; } return <code className={className} {...props}>{children}; } Provide separate pre component for full block styling if needed. See GitHub issues #776, #834, #880 for discussions.
Citations:
- 1: https://github.com/remarkjs/react-markdown/releases
- 2: https://www.npmjs.com/package/react-markdown
- 3: https://github.com/remarkjs/react-markdown/
- 4: https://www.singlehanded.dev/blog/understanding-the-components-prop-in-react-markdown
- 5: https://strapi.io/blog/react-markdown-complete-guide-security-styling
- 6: Cannot run the code after upgrading the library: missing property
code.inlineand changed the type ofp.refremarkjs/react-markdown#776 - 7: https://github.com/remarkjs/react-markdown/blob/main/readme.md
- 8: Incorrect Languageless Code Block Rendering in Example remarkjs/react-markdown#834
- 9: Can't identify single line and multiline code block differently remarkjs/react-markdown#880
🏁 Script executed:
fd package.json | head -1 | xargs cat | jq '.dependencies."react-markdown", .devDependencies."react-markdown"' 2>/dev/null || find . -maxdepth 2 -name package.json -exec grep -A 2 "react-markdown" {} +Repository: AmintaCCCP/GithubStarsManager
Length of output: 87
🏁 Script executed:
wc -l src/components/MarkdownRenderer.tsx && sed -n '740,770p' src/components/MarkdownRenderer.tsxRepository: AmintaCCCP/GithubStarsManager
Length of output: 1746
🏁 Script executed:
# Search for any tests or documentation about markdown handling in the repo
rg -i "language-less|fenced code|``` " --type md --type ts --type tsx -A 2 -B 2 | head -50Repository: AmintaCCCP/GithubStarsManager
Length of output: 141
🌐 Web query:
react-markdown v10 language-less fenced code block className undefined
💡 Result:
In react-markdown v10, for fenced code blocks without a language specifier (language-less), the code component receives className as undefined. This is the expected behavior and matches inline code, where className is also undefined. Fenced code blocks ( content) render as
contentcontent. To distinguish them in a custom code component (e.g., for syntax highlighting), check for the presence of a pre parent via node.parent?.tagName === 'pre', or verify if children contains newlines, or handle both code and pre components separately. The official syntax highlighting example in the documentation uses: code(props) { const {children, className, node, ...rest} = props const match = /language-(\w+)/.exec(className || '') return match ? ( <SyntaxHighlighter language={match[1]} {...rest}>{children} ) : ( <code {...rest} className={className}>{children} ) } For language-less fenced blocks, since className is undefined and no regex match occurs, it falls back to plain , which appears inline unless wrapped in pre by react-markdown. Users reported this as rendering inline-like without additional checks (Issue #834). Note: Older versions passed an inline prop to code to distinguish block vs inline, but this was removed in v9+ (changelog). The top-level className prop on was also removed in v10.
Citations:
- 1: remarkjs/react-markdown@9.1.0...10.0.0
- 2: Incorrect Languageless Code Block Rendering in Example remarkjs/react-markdown#834
- 3: https://www.npmjs.com/package/react-markdown
- 4: https://github.com/remarkjs/react-markdown/blob/main/changelog.md
- 5: https://github.com/remarkjs/react-markdown/blob/main/readme.md
- 6: https://github.com/remarkjs/react-markdown/
Language-less fenced code blocks are incorrectly rendered as inline code.
With the !className check, fenced code blocks without a language specifier receive className={undefined} in react-markdown v10.1.0, causing them to render with inline <code> styling. The pre component then passes through unstyled, leaving them without block-level container styling.
Implement a block-aware fallback in the pre component to detect and properly style language-less code blocks:
Suggested fix
- pre: ({ children }) => <>{children}</>,
+ pre: ({ children }) => {
+ const childArray = React.Children.toArray(children);
+ const ownsBlockUi = childArray.some(
+ child => React.isValidElement(child) && child.type === CodeBlock
+ );
+
+ return ownsBlockUi ? (
+ <>{children}</>
+ ) : (
+ <pre className="my-3 rounded-xl overflow-x-auto bg-gray-50 dark:bg-[`#1e1e1e`] p-4">
+ {children}
+ </pre>
+ );
+ },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/MarkdownRenderer.tsx` around lines 746 - 761, The current code
renderer treats any node with a falsy className as inline, causing fenced blocks
without a language to be rendered inline; update the logic so block/inline is
determined from the actual child element in both the code and pre renderers: in
the code renderer (the "code: ({ className, children, ...props }) => { ... }"
function) continue to extract language from className but do not assume
!className === inline — instead treat inline only when the renderer is actually
given an inline code node (e.g., when children is a string or when the parent
pre is not present); implement a block-aware fallback in the pre renderer (the
"pre: ({ children }) => ..." function) that inspects children.props.className
(or children.props.mdxType/type === 'code') and when the child is a code element
with no language, render it through the existing CodeBlock component with
language='' (or appropriate empty-language handling) so fenced code blocks
without a language receive block-level styling, otherwise pass through children
unchanged.
| const actualSrc = isInView ? src : placeholder; | ||
| const showPlaceholder = !isLoaded && !hasError; | ||
|
|
||
| return ( | ||
| <div | ||
| className={`relative overflow-hidden ${className}`} | ||
| style={{ width, height }} | ||
| > | ||
| {showPlaceholder && ( | ||
| <div | ||
| className="absolute inset-0 bg-gray-200 dark:bg-gray-700 animate-pulse" | ||
| style={{ width, height }} | ||
| /> | ||
| )} | ||
| <img | ||
| ref={imgRef} | ||
| src={hasError ? placeholder : actualSrc} | ||
| alt={alt} | ||
| width={width} | ||
| height={height} | ||
| loading={lazy ? 'lazy' : 'eager'} | ||
| decoding="async" | ||
| onLoad={handleLoad} | ||
| onError={handleError} |
There was a problem hiding this comment.
Don’t let the placeholder mark the real image as loaded.
In lazy mode, the placeholder src fires onLoad, sets isLoaded, and calls the consumer callback before the real image loads. Also reset load/error state when src changes.
🐛 Proposed direction
+ useEffect(() => {
+ setIsLoaded(false);
+ setHasError(false);
+ setIsInView(!lazy);
+ }, [src, lazy]);
+
const actualSrc = isInView ? src : placeholder;
const showPlaceholder = !isLoaded && !hasError;
@@
<img
ref={imgRef}
src={hasError ? placeholder : actualSrc}
@@
- onLoad={handleLoad}
+ onLoad={() => {
+ if (isInView && !hasError) {
+ handleLoad();
+ }
+ }}Even better: observe the wrapper and don’t render the real <img src={src}> until isInView is true.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const actualSrc = isInView ? src : placeholder; | |
| const showPlaceholder = !isLoaded && !hasError; | |
| return ( | |
| <div | |
| className={`relative overflow-hidden ${className}`} | |
| style={{ width, height }} | |
| > | |
| {showPlaceholder && ( | |
| <div | |
| className="absolute inset-0 bg-gray-200 dark:bg-gray-700 animate-pulse" | |
| style={{ width, height }} | |
| /> | |
| )} | |
| <img | |
| ref={imgRef} | |
| src={hasError ? placeholder : actualSrc} | |
| alt={alt} | |
| width={width} | |
| height={height} | |
| loading={lazy ? 'lazy' : 'eager'} | |
| decoding="async" | |
| onLoad={handleLoad} | |
| onError={handleError} | |
| useEffect(() => { | |
| setIsLoaded(false); | |
| setHasError(false); | |
| setIsInView(!lazy); | |
| }, [src, lazy]); | |
| const actualSrc = isInView ? src : placeholder; | |
| const showPlaceholder = !isLoaded && !hasError; | |
| return ( | |
| <div | |
| className={`relative overflow-hidden ${className}`} | |
| style={{ width, height }} | |
| > | |
| {showPlaceholder && ( | |
| <div | |
| className="absolute inset-0 bg-gray-200 dark:bg-gray-700 animate-pulse" | |
| style={{ width, height }} | |
| /> | |
| )} | |
| <img | |
| ref={imgRef} | |
| src={hasError ? placeholder : actualSrc} | |
| alt={alt} | |
| width={width} | |
| height={height} | |
| loading={lazy ? 'lazy' : 'eager'} | |
| decoding="async" | |
| onLoad={() => { | |
| if (isInView && !hasError) { | |
| handleLoad(); | |
| } | |
| }} | |
| onError={handleError} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/OptimizedImage.tsx` around lines 67 - 90, The placeholder
image is firing onLoad and marking the real image as loaded; update
OptimizedImage so the real <img> src is not set to the placeholder in a way that
triggers handleLoad for the final image. Specifically: change logic in the img
render/actualSrc handling to only set src={src} when isInView (or when lazy is
false) so the placeholder never triggers the real-image onLoad, and in
handleLoad/handleError guard the event by checking event.currentTarget.src
matches the intended src (or compare against placeholder) before setting
isLoaded/hasError or calling the consumer callback; also add a useEffect
watching src to reset isLoaded and hasError when the source changes. Reference
identifiers: OptimizedImage component, imgRef, actualSrc, placeholder, isInView,
lazy, isLoaded, hasError, handleLoad, handleError.
| const updatedRepo = { | ||
| ...repository, | ||
| ai_summary: analysis.summary, | ||
| ai_tags: analysis.tags, | ||
| ai_platforms: analysis.platforms, | ||
| custom_category: resolvedCategory, | ||
| category_locked: shouldKeepLocked || wasCategoryLocked, | ||
| analyzed_at: new Date().toISOString(), | ||
| analysis_failed: false | ||
| ai_summary: result.summary, | ||
| ai_tags: result.tags, | ||
| ai_platforms: result.platforms, | ||
| custom_category: result.custom_category, | ||
| category_locked: result.category_locked, | ||
| analyzed_at: result.analyzed_at, | ||
| analysis_failed: result.analysis_failed | ||
| }; | ||
|
|
||
| updateRepository(updatedRepo); |
There was a problem hiding this comment.
Merge AI results onto the latest store snapshot.
updateRepository replaces the stored repo, so spreading the stale repository prop after a long AI request can overwrite edits or sync updates made while analysis was running.
Proposed fix
+ const getLatestRepository = () => {
+ const state = useAppStore.getState();
+ return (
+ state.repositories.find(r => r.id === repoId) ??
+ state.searchResults.find(r => r.id === repoId) ??
+ repository
+ );
+ };
+
const updatedRepo = {
- ...repository,
+ ...getLatestRepository(),
ai_summary: result.summary,
ai_tags: result.tags,
ai_platforms: result.platforms,
custom_category: result.custom_category,
category_locked: result.category_locked,
analyzed_at: result.analyzed_at,
analysis_failed: result.analysis_failed
}; const failedResult = createFailedAnalysisResult();
const failedRepo = {
- ...repository,
+ ...getLatestRepository(),
analyzed_at: failedResult.analyzed_at,
analysis_failed: failedResult.analysis_failed
};Also applies to: 333-340
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/RepositoryCard.tsx` around lines 311 - 322, The current code
builds updatedRepo by spreading the stale repository prop then calling
updateRepository, which can overwrite concurrent edits; instead call
updateRepository with an updater that reads the latest repo snapshot from the
store and merges only the AI-result fields (ai_summary, ai_tags, ai_platforms,
custom_category, category_locked, analyzed_at, analysis_failed) onto that latest
object so you preserve any intervening changes; do the same change for the other
similar block that updates with result fields.
| const EXPORT_DATA_TYPES = useMemo(() => [ | ||
| { key: 'repositories', label: t('仓库数据', 'Repositories') }, | ||
| { key: 'releases', label: t('Release数据', 'Releases') }, | ||
| { key: 'aiConfigs', label: t('AI配置', 'AI Configs') }, | ||
| { key: 'webdavConfigs', label: t('WebDAV配置', 'WebDAV Configs') }, | ||
| { key: 'customCategories', label: t('分类设置', 'Categories') }, | ||
| { key: 'assetFilters', label: t('资源过滤器', 'Asset Filters') }, | ||
| { key: 'discoveryRepos', label: t('发现页数据', 'Discovery Data') }, | ||
| { key: 'subscriptionRepos', label: t('订阅页数据', 'Subscription Data') }, | ||
| { key: 'releaseSubscriptions', label: t('Release订阅', 'Release Subscriptions') }, | ||
| { key: 'searchFilters', label: t('搜索过滤器', 'Search Filters') }, | ||
| { key: 'uiSettings', label: t('UI设置', 'UI Settings') }, | ||
| ], [t]); | ||
|
|
||
| const [exportSelectedTypes, setExportSelectedTypes] = useState<Set<string>>( | ||
| () => new Set(EXPORT_DATA_TYPES.map(item => item.key)) | ||
| ); |
There was a problem hiding this comment.
Do not export secret-bearing configs by default.
aiConfigs and webdavConfigs can contain API keys or credentials, and the current default selects them for plaintext JSON export. Make these explicit opt-in, or redact sensitive fields unless the user confirms.
Proposed fix
const EXPORT_DATA_TYPES = useMemo(() => [
{ key: 'repositories', label: t('仓库数据', 'Repositories') },
{ key: 'releases', label: t('Release数据', 'Releases') },
- { key: 'aiConfigs', label: t('AI配置', 'AI Configs') },
- { key: 'webdavConfigs', label: t('WebDAV配置', 'WebDAV Configs') },
+ { key: 'aiConfigs', label: t('AI配置（包含密钥）', 'AI Configs (contains secrets)'), sensitive: true },
+ { key: 'webdavConfigs', label: t('WebDAV配置（包含凭据）', 'WebDAV Configs (contains credentials)'), sensitive: true },
{ key: 'customCategories', label: t('分类设置', 'Categories') },
{ key: 'assetFilters', label: t('资源过滤器', 'Asset Filters') },
{ key: 'discoveryRepos', label: t('发现页数据', 'Discovery Data') },
@@
const [exportSelectedTypes, setExportSelectedTypes] = useState<Set<string>>(
- () => new Set(EXPORT_DATA_TYPES.map(item => item.key))
+ () => new Set(EXPORT_DATA_TYPES.filter(item => !item.sensitive).map(item => item.key))
);Also applies to: 543-548
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/settings/DataManagementPanel.tsx` around lines 192 - 208, The
export currently defaults to selecting sensitive types 'aiConfigs' and
'webdavConfigs' via EXPORT_DATA_TYPES and the initial exportSelectedTypes state;
change this so those two keys are not included by default (make them opt-in)
and/or implement a confirmation/redaction flow before including them: update
EXPORT_DATA_TYPES handling or the initial state creation (exportSelectedTypes /
setExportSelectedTypes) to omit 'aiConfigs' and 'webdavConfigs' from the new
Set(...), and add UI/flow to explicitly opt-in or to redact secret fields when
exporting these entries (refer to EXPORT_DATA_TYPES, exportSelectedTypes,
setExportSelectedTypes, aiConfigs, webdavConfigs).
| import { useState, useEffect } from 'react'; | ||
|
|
||
| export function useModalVisibility() { | ||
| const [isModalOpen, setIsModalOpen] = useState(false); | ||
|
|
||
| useEffect(() => { | ||
| const handleModalOpen = () => setIsModalOpen(true); | ||
| const handleModalClose = () => setIsModalOpen(false); | ||
|
|
||
| window.addEventListener('gsm:modal-open', handleModalOpen); | ||
| window.addEventListener('gsm:modal-close', handleModalClose); | ||
| return () => { | ||
| window.removeEventListener('gsm:modal-open', handleModalOpen); | ||
| window.removeEventListener('gsm:modal-close', handleModalClose); | ||
| }; | ||
| }, []); | ||
|
|
||
| return isModalOpen; | ||
| } |
There was a problem hiding this comment.
Use a ref-counter instead of a boolean to survive overlapping modals.
isModalOpen flips on any gsm:modal-close event, so with stacked/overlapping modals (a confirm dialog opened from inside another modal, or a Modal remount while another is open) the flag drops to false prematurely and BackToTop/ScrollToBottom reappear through the topmost modal. Combined with the unconditional close dispatch in Modal.tsx, this is observable in normal navigation.
🛠️ Proposed counter-based version
-import { useState, useEffect } from 'react';
-
-export function useModalVisibility() {
- const [isModalOpen, setIsModalOpen] = useState(false);
-
- useEffect(() => {
- const handleModalOpen = () => setIsModalOpen(true);
- const handleModalClose = () => setIsModalOpen(false);
-
- window.addEventListener('gsm:modal-open', handleModalOpen);
- window.addEventListener('gsm:modal-close', handleModalClose);
- return () => {
- window.removeEventListener('gsm:modal-open', handleModalOpen);
- window.removeEventListener('gsm:modal-close', handleModalClose);
- };
- }, []);
-
- return isModalOpen;
-}
+import { useState, useEffect, useRef } from 'react';
+
+export function useModalVisibility() {
+ const [isModalOpen, setIsModalOpen] = useState(false);
+ const countRef = useRef(0);
+
+ useEffect(() => {
+ const onOpen = () => {
+ countRef.current += 1;
+ setIsModalOpen(countRef.current > 0);
+ };
+ const onClose = () => {
+ countRef.current = Math.max(0, countRef.current - 1);
+ setIsModalOpen(countRef.current > 0);
+ };
+
+ window.addEventListener('gsm:modal-open', onOpen);
+ window.addEventListener('gsm:modal-close', onClose);
+ return () => {
+ window.removeEventListener('gsm:modal-open', onOpen);
+ window.removeEventListener('gsm:modal-close', onClose);
+ };
+ }, []);
+
+ return isModalOpen;
+}Note: this pairs with the Modal.tsx fix — without guarding the close dispatch on actual open state, even a counter can drift negative (clamped here, but still semantically off).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useModalVisibility.ts` around lines 1 - 19, useModalVisibility
currently uses a boolean that flips false on any gsm:modal-close, which breaks
when modals overlap; change useModalVisibility to track an integer ref counter
(e.g., modalCountRef) incremented in handleModalOpen and decremented (but
clamped to >=0) in handleModalClose, and derive the returned isModalOpen state
from modalCountRef.current > 0 using setIsModalOpen; keep the same event names
('gsm:modal-open'/'gsm:modal-close'), preserve adding/removing listeners in the
useEffect, and ensure the close handler never allows the counter to go negative
so the hook remains consistent with Modal.tsx’s close dispatch guard.
| color: #16a34a; | ||
| margin-right: 0.6em; | ||
| font-weight: 700; | ||
| font-family: 'Cascadia Code', 'Fira Code', 'JetBrains Mono', 'Consolas', monospace; |
There was a problem hiding this comment.
Fix the stylelint-blocking CSS violations.
The static analysis errors here will keep the stylesheet from passing lint: unquote Consolas, use kebab-case keyframe names, and lowercase optimizeSpeed.
Proposed fix
- font-family: 'Cascadia Code', 'Fira Code', 'JetBrains Mono', 'Consolas', monospace;
+ font-family: 'Cascadia Code', 'Fira Code', 'JetBrains Mono', Consolas, monospace;-@keyframes fadeIn {
+@keyframes fade-in {-.fade-in {
- animation: fadeIn 0.2s ease-out forwards;
+.fade-in {
+ animation: fade-in 0.2s ease-out forwards;
}
.stagger-fade-in > * {
- animation: fadeIn 0.2s ease-out forwards;
+ animation: fade-in 0.2s ease-out forwards;-@keyframes expandFade {
+@keyframes expand-fade { .animate-expand-fade {
- animation: expandFade 0.2s ease-out forwards;
+ animation: expand-fade 0.2s ease-out forwards; .text-optimize {
- text-rendering: optimizeSpeed;
+ text-rendering: optimizespeed;Also applies to: 432-432, 450-450, 617-617, 819-819, 831-836, 852-852, 866-866, 909-909
🧰 Tools
🪛 Stylelint (17.7.0)
[error] 412-412: Expected no quotes around "Consolas" (font-family-name-quotes)
(font-family-name-quotes)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/index.css` at line 412, The CSS has stylelint violations: remove the
unnecessary quotes around Consolas in the font-family declaration (the
font-family rule containing 'Cascadia Code', 'Fira Code', 'JetBrains Mono',
'Consolas', monospace), convert any camelCase keyframe names to kebab-case
(rename `@keyframes` declarations and their uses to kebab-case), and change any
occurrence of optimizeSpeed to lowercase optimize-speed (or the lowercase token
expected by your linter) across the stylesheet (the same fixes apply to the
similar font-family entries and keyframe definitions referenced in the review).
| onProgress?.('Fetching README...'); | ||
| const readmeContent = backend.isAvailable | ||
| ? await backend.getRepositoryReadme(owner, name) | ||
| : await githubApi.getRepositoryReadme(owner, name, signal); | ||
|
|
||
| const categoryNames = categories | ||
| .filter(cat => cat.id !== 'all') | ||
| .map(cat => cat.name); | ||
|
|
||
| onProgress?.('Analyzing with AI...'); | ||
| const analysis = await aiService.analyzeRepository(repository, readmeContent, categoryNames, signal); | ||
|
|
There was a problem hiding this comment.
Stop analysis when the abort signal fires during README loading.
The backend README path ignores signal, and the GitHub README helper currently catches aborts as normal failures. Check signal.aborted before starting the AI request so “Stop” doesn’t continue into analysis with empty/stale README content.
🐛 Proposed defensive check
const readmeContent = backend.isAvailable
? await backend.getRepositoryReadme(owner, name)
: await githubApi.getRepositoryReadme(owner, name, signal);
+
+ if (signal?.aborted) {
+ throw signal.reason instanceof Error
+ ? signal.reason
+ : new DOMException('Analysis aborted', 'AbortError');
+ }
const categoryNames = categories🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/aiAnalysisHelper.ts` around lines 37 - 48, The README fetch path
can proceed after an abort and allow analysis to run on stale/empty data; update
the flow around backend.getRepositoryReadme / githubApi.getRepositoryReadme and
aiService.analyzeRepository so we bail early on abort: (1) if
backend.getRepositoryReadme supports a signal, pass the signal through; (2)
ensure githubApi.getRepositoryReadme rethrows or surfaces AbortError instead of
treating it as a normal failure; and (3) immediately after awaiting the readme
(before calling aiService.analyzeRepository), check signal.aborted and
return/throw to stop the analysis and avoid calling aiService.analyzeRepository
with empty/stale readme content.
| private buildDiscoverySortParams(sortBy: DiscoverySortBy, sortOrder: DiscoverySortOrder): { sort: string; order: string } { | ||
| const sortMap: Record<DiscoverySortBy, string> = { | ||
| 'BestMatch': 'best-match', | ||
| 'Stars': 'stars', | ||
| 'Forks': 'forks', | ||
| 'Updated': 'updated', | ||
| 'Created': 'created', | ||
| 'HelpWanted': 'help-wanted-issues', | ||
| }; | ||
| const orderMap: Record<DiscoverySortOrder, string> = { | ||
| 'Desc': 'desc', | ||
| 'Asc': 'asc', | ||
| }; | ||
| return { | ||
| sort: sortMap[sortBy], | ||
| order: orderMap[sortOrder], | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GitHub REST API search repositories valid sort parameter values best match created
💡 Result:
The valid sort parameter values for the GitHub REST API endpoint GET /search/repositories are: stars, forks, help-wanted-issues, updated. The default is best match (no sort parameter needed).
Citations:
- 1: https://docs.github.com/rest/search/search
- 2: https://docs.github.com/en/rest/search/search
- 3: https://docs.github.com/en/free-pro-team@latest/rest/reference/search/
- 4: https://docs.github.com/en/rest/reference/search
Remove unsupported sort parameters from GitHub repository search.
The GitHub REST API for /search/repositories only supports stars, forks, help-wanted-issues, and updated as sort values. The default (best match) is achieved by omitting the sort parameter entirely. The current code incorrectly includes best-match and created, which are not valid values and will cause API validation failures.
Required fix
private buildDiscoverySortParams(sortBy: DiscoverySortBy, sortOrder: DiscoverySortOrder): { sort: string; order: string } {
const sortMap: Record<DiscoverySortBy, string> = {
- 'BestMatch': 'best-match',
+ 'BestMatch': '',
'Stars': 'stars',
'Forks': 'forks',
'Updated': 'updated',
- 'Created': 'created',
+ 'Created': '',
'HelpWanted': 'help-wanted-issues',
};Update the discovery sort UI to remove or conditionally handle the unsupported BestMatch and Created options, or ensure the caller properly omits empty sort values when building the API request.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/githubApi.ts` around lines 492 - 509, The
buildDiscoverySortParams function is returning unsupported GitHub search sort
values ('best-match' and 'created'); update buildDiscoverySortParams to not
return invalid sort values: remove the 'Created' entry from sortMap and make
'BestMatch' produce no sort (e.g., map to undefined or null) so callers can omit
the sort parameter; also update any caller that uses buildDiscoverySortParams to
skip adding the sort query parameter when the returned sort is falsy, ensuring
only 'stars', 'forks', 'help-wanted-issues', or 'updated' are sent to the
/search/repositories API.
| forks_count: number; | ||
| forks: number; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find sites that construct Repository objects without `forks`/`forks_count` set.
rg -nP --type=ts -C2 '\b(Repository|DiscoveryRepo|SubscriptionRepo)\b\s*(=|:|\[\]|\))' -g '!**/types/**'
rg -nP --type=ts -C1 '\bforks_count\b|\bforks\b' -g '!**/types/**'Repository: AmintaCCCP/GithubStarsManager
Length of output: 32708
🏁 Script executed:
sed -n '9,50p' src/utils/searchTestUtils.tsRepository: AmintaCCCP/GithubStarsManager
Length of output: 1540
🏁 Script executed:
sed -n '1,100p' src/types/index.ts | head -30Repository: AmintaCCCP/GithubStarsManager
Length of output: 764
🏁 Script executed:
cat -n src/utils/searchTestUtils.ts | tail -100Repository: AmintaCCCP/GithubStarsManager
Length of output: 3795
🏁 Script executed:
sed -n '9,120p' src/utils/searchTestUtils.tsRepository: AmintaCCCP/GithubStarsManager
Length of output: 3941
🏁 Script executed:
rg -n 'as any|as Repository|satisfies' src/utils/searchTestUtils.tsRepository: AmintaCCCP/GithubStarsManager
Length of output: 55
🏁 Script executed:
cat tsconfig.json | grep -A 5 '"strict"'Repository: AmintaCCCP/GithubStarsManager
Length of output: 55
🏁 Script executed:
cat tsconfig.jsonRepository: AmintaCCCP/GithubStarsManager
Length of output: 191
🏁 Script executed:
cat tsconfig.app.json | head -30Repository: AmintaCCCP/GithubStarsManager
Length of output: 624
🏁 Script executed:
rg -n 'ts-ignore|ts-nocheck' src/utils/searchTestUtils.tsRepository: AmintaCCCP/GithubStarsManager
Length of output: 55
Making both forks_count and forks required breaks test mock data and creates type violations.
The type definition adds both fields as required, but src/utils/searchTestUtils.ts defines mock Repository objects that omit both fields entirely. With strict TypeScript enabled, this violates the type contract. Additionally, real code shows defensive patterns like repo.forks_count ?? repo.forks ?? 0 in display logic, indicating the code was designed to handle cases where one or both might be absent. Consider making one of them optional to accommodate test data and defensive coding practices.
Suggested fix
stargazers_count: number;
- forks_count: number;
- forks: number;
+ forks_count: number;
+ /** Legacy alias mirrored from GitHub REST; prefer `forks_count`. */
+ forks?: number;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| forks_count: number; | |
| forks: number; | |
| forks_count: number; | |
| /** Legacy alias mirrored from GitHub REST; prefer `forks_count`. */ | |
| forks?: number; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/types/index.ts` around lines 8 - 9, The types currently require both
forks_count and forks which breaks test mocks and contradicts defensive usage
(repo.forks_count ?? repo.forks ?? 0); update the repository type declaration so
that at least forks is optional (e.g., make forks?: number) so test objects that
omit it are valid and runtime code can continue using the nullish-coalescing
pattern with forks_count and forks.
2 participants
Summary by CodeRabbit
Release Notes
New Features
Improvements