Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis pull request refactors the version synchronization feature by extracting version comparison logic into a dedicated React hook ( Changes
Sequence DiagramsequenceDiagram
participant SyncAlert as SyncAlert Component
participant Hook as useNpmVersionCompare Hook
participant SWR as SWR Library
participant Registry as NPM Registry
SyncAlert->>Hook: useNpmVersionCompare(pkg)
activate Hook
Hook->>Hook: Check if private package
alt Private Package
Hook-->>SyncAlert: needSync: false, isLoading: false
else Public Package
Hook->>SWR: Fetch registry data
activate SWR
SWR->>Registry: GET registry metadata
activate Registry
Registry-->>SWR: Package metadata
deactivate Registry
SWR-->>Hook: Data or error
deactivate SWR
Hook->>Hook: Compute missingVersions & tagsDiff
Hook-->>SyncAlert: needSync, isLoading, missingVersions, tagsDiff
end
deactivate Hook
SyncAlert->>SyncAlert: Render based on isLoading & needSync
alt Loading State
SyncAlert->>SyncAlert: Show Spin with message
else Sync Needed
SyncAlert->>SyncAlert: Show warning with diffs
else No Sync Needed
SyncAlert->>SyncAlert: No alert
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 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 |
Summary of ChangesHello @elrrrrrrr, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求引入了一项新功能，允许用户将其本地包的版本和分发标签与官方 npm 注册表进行在线比较。这有助于用户快速识别任何不一致之处，并了解是否需要同步其包。通过将复杂的比较逻辑封装在一个新的 Hook 中，并改进了用户界面以清晰地显示同步状态和详细差异，提升了用户体验和系统的健壮性。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
此拉取请求引入了一个很棒的新功能：在线将包版本和标签与官方 npm 仓库进行比较。实现的核心是一个新的自定义钩子 useNpmVersionCompare，它封装了比较逻辑，整体实现很可靠。
我的审查主要集中在以下几个方面以求改进：
- 正确性：在
useNpmVersionCompare中，useSwr钩子的闭包存在一个潜在的数据过时问题，这可能导致错误的比较结果。 - 代码质量与可维护性：我发现了一些简化代码的机会，例如移除冗余逻辑和使用更现代的 API 功能（如
fetch的cache选项）。
总的来说，这是一个很有价值的补充。解决这些问题将使实现更加健壮和易于维护。
| const { data, isLoading, error } = useSwr( | ||
| pkgName && !isPrivate ? `npm-compare:${pkgName}` : null, | ||
| async () => { | ||
| const res = await fetch(`${NPM_REGISTRY}/${pkgName}?t=${Date.now()}`); |
There was a problem hiding this comment.
使用 ?t=${Date.now()} 来绕过缓存是一种常见但略显粗糙的方法。它能有效防止浏览器和某些代理的缓存。然而，fetch API 提供了更具声明性的方式来控制缓存行为，即通过 cache 选项。使用 cache 选项能让代码意图更清晰。
建议使用 { cache: 'no-cache' } 来确保获取到最新的数据，而不是通过修改 URL。
| const res = await fetch(`${NPM_REGISTRY}/${pkgName}?t=${Date.now()}`); | |
| const res = await fetch(`${NPM_REGISTRY}/${pkgName}`, { cache: 'no-cache' }); |
| // 私有包不需要同步 | ||
| if (isPrivate) { | ||
| return { | ||
| needSync: false, | ||
| isLoading: false, | ||
| missingVersions: [], | ||
| tagsDiff: [], | ||
| }; | ||
| } |
There was a problem hiding this comment.
这个 if (isPrivate) 代码块是多余的。对于私有包，useSwr hook 已经通过在第 21 行传递 null 作为 key 来禁用了。当被禁用时，useSwr 返回 isLoading: false 和 data: undefined。最终的 return 语句（第 78-84 行）将为私有包正确计算出期望的结果（例如 needSync: false）。
移除这个代码块可以简化代码，同时保持功能不变。
There was a problem hiding this comment.
Pull request overview
This PR adds functionality to automatically compare package versions with the upstream npm registry (registry.npmjs.org) and displays sync status to users. The comparison is performed client-side using a new custom React hook that fetches and compares version metadata.
- Introduces real-time version comparison with npm registry using SWR for data fetching
- Displays detailed sync status including missing versions and tag differences
- Shows sync button by default to ensure users can always trigger a refresh
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/hooks/useNpmVersionCompare.ts |
New hook that fetches npm registry data and compares versions/tags with local package manifest |
src/components/SyncAlert.tsx |
Updated to use the new comparison hook and display detailed sync information with loading states |
src/slugs/versions/index.tsx |
Simplified by removing needSync parameter since it's now computed internally by the hook |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const { data, isLoading, error } = useSwr( | ||
| pkgName && !isPrivate ? `npm-compare:${pkgName}` : null, | ||
| async () => { | ||
| const res = await fetch(`${NPM_REGISTRY}/${pkgName}?t=${Date.now()}`); |
There was a problem hiding this comment.
Using Date.now() in the fetch URL defeats the purpose of SWR's caching and the dedupingInterval configuration. This will force a fresh request every time the hook is called, bypassing SWR's cache. Remove the timestamp parameter to allow SWR to handle caching properly, or if cache-busting is intentionally needed, consider using SWR's revalidation options instead.
| const res = await fetch(`${NPM_REGISTRY}/${pkgName}?t=${Date.now()}`); | |
| const res = await fetch(`${NPM_REGISTRY}/${pkgName}`); |
| return { | ||
| needSync: data?.needSync ?? false, | ||
| isLoading, | ||
| error, | ||
| missingVersions: data?.missingVersions ?? [], | ||
| tagsDiff: data?.tagsDiff ?? [], | ||
| }; |
There was a problem hiding this comment.
The error from SWR is retrieved but never exposed or handled in the component. When an error occurs during the fetch, users won't see any feedback. Consider adding error handling in the UI, such as displaying an error state in the Alert component when error is present.
| export default function SyncAlert({ pkg }: SyncAlertProps) { | ||
| const registry = pkg?._source_registry_name; | ||
| // const { needSync, isLoading } = useNeedSync(pkg!, registry); | ||
| const { needSync, isLoading, missingVersions, tagsDiff } = useNpmVersionCompare(pkg); |
There was a problem hiding this comment.
The error field returned from useNpmVersionCompare is destructured but never used. If an error occurs during version comparison, users won't see any feedback. Consider handling the error state in the component, for example by displaying an error alert when error is present.
| if (isLoading) { | ||
| type = 'info'; | ||
| message = ( | ||
| <Space> | ||
| <Spin size="small" /> | ||
| <span>正在比对上游版本...</span> | ||
| </Space> | ||
| ); |
There was a problem hiding this comment.
When isLoading is true, the description variable is set in lines 29-34 but then never cleared or updated in the loading state handler (lines 36-43). This means the loading alert will still show the description text like "为私有包，发布流程在..." or "为公网包，目前会从..." along with the loading spinner. Consider clearing or updating the description during the loading state for a cleaner UI.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/hooks/useNpmVersionCompare.ts (1)
20-66: Consider including pkg data in the SWR key.The fetcher captures
pkgfrom the closure (lines 35, 40) to access local versions and tags. If thepkgobject is updated butpkgNameremains unchanged, SWR won't re-run the fetcher since the key only includespkgName. This could result in comparing stale local data against fresh npm data.While this may not be a practical issue if the package manifest remains stable during a page view, consider including a version identifier or hash of the relevant pkg fields in the SWR key to ensure accurate comparisons when the local data changes.
🔎 Example approach
+ const versionsHash = pkg ? JSON.stringify(Object.keys(pkg.versions || {})) : ''; + const tagsHash = pkg ? JSON.stringify(pkg['dist-tags'] || {}) : ''; const { data, isLoading, error } = useSwr( - pkgName && !isPrivate ? `npm-compare:${pkgName}` : null, + pkgName && !isPrivate ? `npm-compare:${pkgName}:${versionsHash}:${tagsHash}` : null, async () => {Note: This increases key complexity, so only implement if pkg updates during the same page view are expected.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/SyncAlert.tsxsrc/hooks/useNpmVersionCompare.tssrc/slugs/versions/index.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
src/slugs/versions/index.tsx (2)
src/pages/package/[...slug]/index.tsx (1)
PageProps(22-26)src/components/SyncAlert.tsx (1)
SyncAlert(20-85)
src/hooks/useNpmVersionCompare.ts (1)
src/hooks/useManifest.ts (1)
PackageManifest(32-47)
src/components/SyncAlert.tsx (1)
src/hooks/useNpmVersionCompare.ts (1)
useNpmVersionCompare(16-85)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Agent
- GitHub Check: Node.js / Test (ubuntu-latest, 24)
- GitHub Check: Node.js / Test (ubuntu-latest, 22)
🔇 Additional comments (3)
src/hooks/useNpmVersionCompare.ts (1)
68-85: LGTM!The private package handling and safe default values using the nullish coalescing operator are well-implemented. The hook properly returns all fields defined in the
NpmVersionCompareResultinterface.src/slugs/versions/index.tsx (1)
192-208: LGTM!The refactoring simplifies
ReadOnlyVersionsby removing theadditionalInfo/needSyncparameter and prop passing. The component now delegates synchronization logic entirely toSyncAlert, which improves encapsulation.src/components/SyncAlert.tsx (1)
48-64: LGTM!The version difference detail building is well-implemented. The truncation of missing versions to the first 3 items (with a count) when there are more provides a good user experience, and the tag difference formatting clearly shows both local and upstream values.
| if (isLoading) { | ||
| type = 'info'; | ||
| message = ( | ||
| <Space> | ||
| <Spin size="small" /> | ||
| <span>正在比对上游版本...</span> | ||
| </Space> | ||
| ); | ||
| } else if (needSync) { | ||
| type = 'warning'; | ||
| message = '版本和上游信息不一致'; | ||
| } | ||
|
|
||
| if (needSync === undefined) { | ||
| type = 'info'; | ||
| message = '目前暂不支持自动版本比对，需访问源站进行确认'; | ||
| const details: string[] = []; | ||
| if (missingVersions.length > 0) { | ||
| const displayVersions = | ||
| missingVersions.length > 3 | ||
| ? `${missingVersions.slice(0, 3).join(', ')} 等 ${missingVersions.length} 个版本` | ||
| : missingVersions.join(', '); | ||
| details.push(`缺失版本: ${displayVersions}`); | ||
| } | ||
| if (tagsDiff.length > 0) { | ||
| const tagDetails = tagsDiff | ||
| .map((d) => `${d.tag}: 本地 ${d.local || '无'} / 上游 ${d.npm || '无'}`) | ||
| .join('; '); | ||
| details.push(`Tags 差异: ${tagDetails}`); | ||
| } | ||
| if (details.length > 0) { | ||
| description = details.join(' | '); | ||
| } | ||
| } |
There was a problem hiding this comment.
Handle the error state from the hook.
The error field is destructured from useNpmVersionCompare (line 22) but never displayed to the user. If the npm registry fetch fails (non-404 error), the component will show the success message "所有版本均已同步" because needSync defaults to false when data is unavailable. This misleads users into thinking all versions are synchronized when the comparison couldn't be performed.
🔎 Proposed fix to display error state
if (isLoading) {
type = 'info';
message = (
<Space>
<Spin size="small" />
<span>正在比对上游版本...</span>
</Space>
);
+ } else if (error) {
+ type = 'error';
+ message = '无法获取上游版本信息';
+ description = `获取 registry.npmjs.org 版本信息失败，请稍后重试`;
} else if (needSync) {
type = 'warning';
message = '版本和上游信息不一致';📝 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.
| if (isLoading) { | |
| type = 'info'; | |
| message = ( | |
| <Space> | |
| <Spin size="small" /> | |
| <span>正在比对上游版本...</span> | |
| </Space> | |
| ); | |
| } else if (needSync) { | |
| type = 'warning'; | |
| message = '版本和上游信息不一致'; | |
| } | |
| if (needSync === undefined) { | |
| type = 'info'; | |
| message = '目前暂不支持自动版本比对，需访问源站进行确认'; | |
| const details: string[] = []; | |
| if (missingVersions.length > 0) { | |
| const displayVersions = | |
| missingVersions.length > 3 | |
| ? `${missingVersions.slice(0, 3).join(', ')} 等 ${missingVersions.length} 个版本` | |
| : missingVersions.join(', '); | |
| details.push(`缺失版本: ${displayVersions}`); | |
| } | |
| if (tagsDiff.length > 0) { | |
| const tagDetails = tagsDiff | |
| .map((d) => `${d.tag}: 本地 ${d.local || '无'} / 上游 ${d.npm || '无'}`) | |
| .join('; '); | |
| details.push(`Tags 差异: ${tagDetails}`); | |
| } | |
| if (details.length > 0) { | |
| description = details.join(' | '); | |
| } | |
| } | |
| if (isLoading) { | |
| type = 'info'; | |
| message = ( | |
| <Space> | |
| <Spin size="small" /> | |
| <span>正在比对上游版本...</span> | |
| </Space> | |
| ); | |
| } else if (error) { | |
| type = 'error'; | |
| message = '无法获取上游版本信息'; | |
| description = `获取 registry.npmjs.org 版本信息失败，请稍后重试`; | |
| } else if (needSync) { | |
| type = 'warning'; | |
| message = '版本和上游信息不一致'; | |
| const details: string[] = []; | |
| if (missingVersions.length > 0) { | |
| const displayVersions = | |
| missingVersions.length > 3 | |
| ? `${missingVersions.slice(0, 3).join(', ')} 等 ${missingVersions.length} 个版本` | |
| : missingVersions.join(', '); | |
| details.push(`缺失版本: ${displayVersions}`); | |
| } | |
| if (tagsDiff.length > 0) { | |
| const tagDetails = tagsDiff | |
| .map((d) => `${d.tag}: 本地 ${d.local || '无'} / 上游 ${d.npm || '无'}`) | |
| .join('; '); | |
| details.push(`Tags 差异: ${tagDetails}`); | |
| } | |
| if (details.length > 0) { | |
| description = details.join(' | '); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/components/SyncAlert.tsx around lines 36 to 65, the error returned from
useNpmVersionCompare is not handled so failures render as "all synced"; detect
when error is truthy and set type='error' with a clear user-facing message (e.g.
'无法比对版本') and set description to the error message or a friendly fallback
(error.message || '比对时发生错误，请重试'), ensuring this branch runs before the
success/needSync branches so users see the failure instead of a false success.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.