feat(pages): show live npm downloads in highlights stats - #794
feat(pages): show live npm downloads in highlights stats#794lizhengfeng101 wants to merge 3 commits into
Conversation
Add a useNpmDownloads hook that fetches real monthly download counts from the npm registry API at runtime, and surface them in the Highlights section, falling back to a static i18n value while loading or on error. Also refresh the stats row copy and ordering, and sync the label/caption/value changes across en, zh, ja and ru. The hook encodes the (scoped) package name into the request URL and aborts the fetch after a timeout so the UI degrades promptly on a slow or unresponsive network.
|
🔍 OpenCodeReview found 7 issue(s) in this PR.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| // 弱网或 API 无响应时，超时后 abort 请求并降级，避免界面长期卡在 loading | ||
| const controller = new AbortController(); | ||
| const timeout = setTimeout(() => controller.abort(), 8000); |
There was a problem hiding this comment.
[bug · medium]
Resource leak: setTimeout not cleared on fetch completion. The 8-second timeout is only cleared in the effect cleanup function. If the fetch resolves or rejects well before the timeout, the timer keeps running and will call controller.abort() on an already-finished request. While functionally harmless, it's a minor resource leak. Clear the timeout in both success and error paths (or in a finally block).
| const timeout = setTimeout(() => controller.abort(), 8000); | ||
|
|
||
| // pkg 可能是 scoped 包名（含 `/`），编码后再插值以保证 URL 路径合法 | ||
| fetch(`https://api.npmjs.org/downloads/point/${period}/${encodeURIComponent(pkg)}`, { |
There was a problem hiding this comment.
[maintainability · medium]
Hardcoded API URL. The npm API base URL https://api.npmjs.org/downloads/point/ is hardcoded directly in the fetch call. Per project coding standards, hardcoded URL paths (especially for API endpoints) should be extracted into a configuration constant (e.g., const NPM_API_BASE = '...') or environment variable. This makes the endpoint easier to swap for testing, staging, or if the URL changes.
Suggestion:
| fetch(`https://api.npmjs.org/downloads/point/${period}/${encodeURIComponent(pkg)}`, { | |
| fetch(`${NPM_API_BASE}/${period}/${encodeURIComponent(pkg)}`, { |
| fetch(`https://api.npmjs.org/downloads/point/${period}/${encodeURIComponent(pkg)}`, { | ||
| signal: controller.signal, | ||
| }) | ||
| .then((r) => { | ||
| if (!r.ok) throw new Error(`npm downloads API responded ${r.status}`); | ||
| return r.json(); | ||
| }) | ||
| .then((data: { downloads?: number }) => { | ||
| if (cancelled) return; | ||
| if (typeof data.downloads !== 'number') throw new Error('unexpected payload'); | ||
| setState({ downloads: data.downloads, loading: false, error: false }); | ||
| }) | ||
| .catch(() => { | ||
| if (cancelled) return; | ||
| setState({ downloads: null, loading: false, error: true }); | ||
| }); |
There was a problem hiding this comment.
[maintainability · medium]
Prefer async/await over Promise chains. The project coding standards require preferring async/await over raw .then()/.catch() chains. Other async code in this project (e.g., HeroSection.tsx, MarkdownRenderer.tsx) consistently uses async/await. Refactoring to an async IIFE with try/catch would improve readability and align with codebase conventions.
Suggestion:
| fetch(`https://api.npmjs.org/downloads/point/${period}/${encodeURIComponent(pkg)}`, { | |
| signal: controller.signal, | |
| }) | |
| .then((r) => { | |
| if (!r.ok) throw new Error(`npm downloads API responded ${r.status}`); | |
| return r.json(); | |
| }) | |
| .then((data: { downloads?: number }) => { | |
| if (cancelled) return; | |
| if (typeof data.downloads !== 'number') throw new Error('unexpected payload'); | |
| setState({ downloads: data.downloads, loading: false, error: false }); | |
| }) | |
| .catch(() => { | |
| if (cancelled) return; | |
| setState({ downloads: null, loading: false, error: true }); | |
| }); | |
| (async () => { | |
| try { | |
| const r = await fetch(`https://api.npmjs.org/downloads/point/${period}/${encodeURIComponent(pkg)}`, { | |
| signal: controller.signal, | |
| }); | |
| if (!r.ok) throw new Error(`npm downloads API responded ${r.status}`); | |
| const data: { downloads?: number } = await r.json(); | |
| if (cancelled) return; | |
| if (typeof data.downloads !== 'number') throw new Error('unexpected payload'); | |
| setState({ downloads: data.downloads, loading: false, error: false }); | |
| } catch { | |
| if (cancelled) return; | |
| setState({ downloads: null, loading: false, error: true }); | |
| } finally { | |
| if (!cancelled) clearTimeout(timeout); | |
| } | |
| })(); |
| 'highlights.stat3Value': '3M+', | ||
| 'highlights.stat3Label': 'REAL-WORLD TASKS', | ||
| 'highlights.stat3Caption': 'Code review tasks executed', | ||
| 'highlights.stat3Caption': 'Battle-tested inside Alibaba Group', |
There was a problem hiding this comment.
[maintainability · medium]
The caption for stat3 is now "Battle-tested inside Alibaba Group", which is identical to stat1Caption. Having two stats display the exact same caption text on the page will look like a copy-paste mistake to users. Consider giving stat3 a distinct caption that reflects what "REAL-WORLD TASKS" measures (e.g., something about code review tasks executed, or a different description of scale).
| 'highlights.stat1Caption': 'Alibabaグループ内で実戦検証済み', | ||
| 'highlights.stat2Value': '> 30%', | ||
| 'highlights.stat2Label': '採用率', | ||
| 'highlights.stat2Caption': 'Alibabaグループ内で実戦検証済み', | ||
| 'highlights.stat3Value': '1M+', | ||
| 'highlights.stat2Value': '150K+', | ||
| 'highlights.stat2Label': 'NPM コミュニティダウンロード', | ||
| 'highlights.stat2Caption': 'npm 過去30日の実ダウンロード数', | ||
| 'highlights.stat3Value': '3M+', | ||
| 'highlights.stat3Label': '実タスク', | ||
| 'highlights.stat3Caption': '実行されたコードレビュータスク', | ||
| 'highlights.stat3Caption': 'Alibabaグループ内で実戦検証済み', |
There was a problem hiding this comment.
[maintainability · low]
The display order of stats (stat1, stat3, stat2, stat4, stat5) is now different from the key numbering (stat1, stat2, stat3, stat4, stat5). This can be confusing for future maintainers who may expect the keys to correspond to display order. Consider either:
- Renumbering the i18n keys to match the display order, or
- Adding a comment explaining why stat2 (npm downloads) is intentionally placed after stat3.
Also, note that highlights.stat1Caption and highlights.stat3Caption now have identical values ("Alibabaグループ内で実戦検証済み" / "Battle-tested inside Alibaba Group"). If this is intentional, it's fine, but it could also be an opportunity to deduplicate into a single key or give stat3 a more specific caption.
| 'highlights.stat2Label': 'ЗАГРУЗКИ СООБЩЕСТВА NPM', | ||
| 'highlights.stat2Caption': 'реальные загрузки npm · 30 дней', | ||
| 'highlights.stat3Value': '3M+', | ||
| 'highlights.stat3Label': 'ЗАДАЧ КОД-РЕВЬЮ', |
There was a problem hiding this comment.
[other · medium]
Translation inconsistency: 'ЗАДАЧ КОД-РЕВЬЮ' means 'CODE REVIEW TASKS' in Russian, but the English source is 'REAL-WORLD TASKS' (zh: '真实任务', ja: '実タスク'). Consider updating to something like 'РЕАЛЬНЫХ ЗАДАЧ' to be consistent with the other languages.
Suggestion:
| 'highlights.stat3Label': 'ЗАДАЧ КОД-РЕВЬЮ', | |
| 'highlights.stat3Label': 'РЕАЛЬНЫХ ЗАДАЧ', |
| 'highlights.stat3Value': '3M+', | ||
| 'highlights.stat3Label': '真实任务', | ||
| 'highlights.stat3Caption': '已执行的代码审查任务', | ||
| 'highlights.stat3Caption': '经阿里巴巴集团内部实战验证', |
There was a problem hiding this comment.
[bug · medium]
Potential copy-paste issue: stat3Caption is now identical to stat1Caption ("经阿里巴巴集团内部实战验证"). This means the same caption text will appear twice on the page — once under "内部活跃用户" (stat1) and again under "真实任务" (stat3).
Previously, stat3Caption had a unique value ("已执行的代码审查任务") that described the stat. It seems likely that stat3Caption should have its own distinct caption rather than duplicating stat1Caption. Please verify this is intentional, or provide a unique caption for stat3.
…hook - Give highlights stat3 its own caption instead of duplicating stat1's 'battle-tested' text across all four locales - Correct the Russian stat3 label to match 'real-world tasks' in the other locales - Refactor the npm-downloads fetch to async/await and clear the timeout in a finally block so it no longer lingers after the request settles
1 participant
What
Why
The download figure was previously a hardcoded static value that drifted from reality. Fetching it live from the npm registry keeps the landing page honest and self-updating, while the static i18n fallback guarantees the section still renders on a slow or blocked network.
Notes
Verification