From 9a4d4c45352ee6d88ef5fee4ff8bddd311f3bef2 Mon Sep 17 00:00:00 2001 From: Tu Shaokun <2801884530@qq.com> Date: Sat, 29 Nov 2025 13:39:43 +0800 Subject: [PATCH] =?UTF-8?q?refactor:=20=E6=A8=A1=E5=9D=97=E5=8C=96=20skill?= =?UTF-8?q?=20=E7=BB=93=E6=9E=84=EF=BC=8C=E6=94=AF=E6=8C=81=E6=8C=89?= =?UTF-8?q?=E9=9C=80=E5=8A=A0=E8=BD=BD?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 将 SKILL.md 从 1774 行精简到 ~180 行 - 新增 references/react.md:React 19、RSC、TanStack Query v5 - 新增 references/vue.md:Vue 3 Composition API - 新增 references/rust.md:所有权、unsafe、异步 - 新增 references/typescript.md:类型安全、async/await - 新增 references/python.md:常见陷阱 - 更新 README.md:说明按需加载机制 优化效果: - 初始加载:~40K tokens → ~4K tokens - 支持 Progressive Disclosure 按需加载 --- README.md | 105 ++- SKILL.md | 1685 +------------------------------------- references/python.md | 77 ++ references/react.md | 799 ++++++++++++++++++ references/rust.md | 256 ++++++ references/typescript.md | 101 +++ references/vue.md | 240 ++++++ 7 files changed, 1589 insertions(+), 1674 deletions(-) create mode 100644 references/python.md create mode 100644 references/react.md create mode 100644 references/rust.md create mode 100644 references/typescript.md create mode 100644 references/vue.md diff --git a/README.md b/README.md index b7f5eb6..25a397f 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ ## English -> A comprehensive code review skill for Claude Code, covering React 19, Vue 3, Rust, TypeScript, and more. +> A modular code review skill for Claude Code, covering React 19, Vue 3, Rust, TypeScript, Python, and more. [![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](https://opensource.org/licenses/MIT) @@ -14,10 +14,11 @@ This is a Claude Code skill designed to help developers conduct effective code reviews. It provides: -- **Language-specific patterns** for React 19, Vue 3, Rust, TypeScript/JavaScript, Python, SQL +- **Language-specific patterns** for React 19, Vue 3, Rust, TypeScript/JavaScript, Python - **Modern framework support** including React Server Components, TanStack Query v5, Suspense & Streaming - **Comprehensive checklists** for security, performance, and code quality - **Best practices** for giving constructive feedback +- **Modular structure** for on-demand loading (reduces context usage) ### Features @@ -33,29 +34,36 @@ This is a Claude Code skill designed to help developers conduct effective code r #### Content Statistics -- **SKILL.md**: ~1,800 lines of review guidelines and code examples -- **common-bugs-checklist.md**: ~1,200 lines of bug patterns and checklists -- Additional reference files for security review, PR templates, and more +| File | Lines | Description | +|------|-------|-------------| +| **SKILL.md** | ~180 | Core principles + index (loads on skill activation) | +| **references/react.md** | ~650 | React/Next.js patterns (on-demand) | +| **references/vue.md** | ~200 | Vue 3 patterns (on-demand) | +| **references/rust.md** | ~200 | Rust patterns (on-demand) | +| **references/typescript.md** | ~100 | TypeScript/JS patterns (on-demand) | +| **references/python.md** | ~60 | Python patterns (on-demand) | + +**Total: ~2,000+ lines** of review guidelines and code examples, loaded on-demand per language. ### Installation #### For Claude Code Users -Copy the skill to your Claude Code plugins directory: +Copy the skill to your Claude Code skills directory: ```bash # Clone the repository git clone https://github.com/tt-a1i/ai-code-review-guide.git # Copy to Claude Code skills directory -cp -r ai-code-review-guide ~/.claude/commands/code-review-excellence +cp -r ai-code-review-guide ~/.claude/skills/code-review-excellence ``` Or add to your existing Claude Code plugin: ```bash -cp -r ai-code-review-guide/SKILL.md ~/.claude/plugins/your-plugin/skills/code-review/ -cp -r ai-code-review-guide/references ~/.claude/plugins/your-plugin/skills/code-review/ +# Copy the entire directory structure +cp -r ai-code-review-guide ~/.claude/plugins/your-plugin/skills/code-review/ ``` ### Usage @@ -72,11 +80,16 @@ Or reference it in your custom commands. ``` ai-code-review-guide/ -├── SKILL.md # Main skill definition -├── README.md # This file -├── LICENSE # MIT License -├── CONTRIBUTING.md # Contribution guidelines +├── SKILL.md # Core skill (loads immediately) +├── README.md # This file +├── LICENSE # MIT License +├── CONTRIBUTING.md # Contribution guidelines ├── references/ +│ ├── react.md # React/Next.js patterns (on-demand) +│ ├── vue.md # Vue 3 patterns (on-demand) +│ ├── rust.md # Rust patterns (on-demand) +│ ├── typescript.md # TypeScript/JS patterns (on-demand) +│ ├── python.md # Python patterns (on-demand) │ ├── common-bugs-checklist.md # Language-specific bug patterns │ ├── security-review-guide.md # Security review checklist │ └── code-review-best-practices.md @@ -87,6 +100,16 @@ ai-code-review-guide/ └── pr-analyzer.py # PR complexity analyzer ``` +### On-Demand Loading + +This skill uses **Progressive Disclosure** to minimize context usage: + +1. **SKILL.md** (~180 lines) loads when the skill is activated +2. **Language-specific files** load only when reviewing that language +3. **Reference files** load only when explicitly needed + +This means reviewing a React PR only loads SKILL.md + react.md, not Vue/Rust/Python content. + ### Key Topics Covered #### React 19 @@ -143,7 +166,7 @@ This project is licensed under the MIT License - see the [LICENSE](LICENSE) file ## 中文 -> 一个全面的 Claude Code 代码审查技能,覆盖 React 19、Vue 3、Rust、TypeScript 等。 +> 一个模块化的 Claude Code 代码审查技能,覆盖 React 19、Vue 3、Rust、TypeScript、Python 等。 [![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](https://opensource.org/licenses/MIT) @@ -151,10 +174,11 @@ This project is licensed under the MIT License - see the [LICENSE](LICENSE) file 这是一个为 Claude Code 设计的代码审查技能,旨在帮助开发者进行高效的代码审查。它提供: -- **语言特定模式**:覆盖 React 19、Vue 3、Rust、TypeScript/JavaScript、Python、SQL +- **语言特定模式**:覆盖 React 19、Vue 3、Rust、TypeScript/JavaScript、Python - **现代框架支持**:包括 React Server Components、TanStack Query v5、Suspense & Streaming - **全面的检查清单**:安全、性能和代码质量检查 - **最佳实践**:如何提供建设性的反馈 +- **模块化结构**:按需加载,减少上下文占用 ### 特性 @@ -170,29 +194,36 @@ This project is licensed under the MIT License - see the [LICENSE](LICENSE) file #### 内容统计 -- **SKILL.md**:约 1,800 行审查指南和代码示例 -- **common-bugs-checklist.md**:约 1,200 行错误模式和检查清单 -- 额外的安全审查、PR 模板等参考文件 +| 文件 | 行数 | 描述 | +|------|------|------| +| **SKILL.md** | ~180 | 核心原则 + 索引(技能激活时加载)| +| **references/react.md** | ~650 | React/Next.js 模式(按需加载)| +| **references/vue.md** | ~200 | Vue 3 模式(按需加载)| +| **references/rust.md** | ~200 | Rust 模式(按需加载)| +| **references/typescript.md** | ~100 | TypeScript/JS 模式(按需加载)| +| **references/python.md** | ~60 | Python 模式(按需加载)| + +**总计:2,000+ 行**审查指南和代码示例,按语言按需加载。 ### 安装 #### Claude Code 用户 -将技能复制到 Claude Code 插件目录: +将技能复制到 Claude Code skills 目录: ```bash # 克隆仓库 git clone https://github.com/tt-a1i/ai-code-review-guide.git # 复制到 Claude Code skills 目录 -cp -r ai-code-review-guide ~/.claude/commands/code-review-excellence +cp -r ai-code-review-guide ~/.claude/skills/code-review-excellence ``` 或添加到现有的 Claude Code 插件: ```bash -cp -r ai-code-review-guide/SKILL.md ~/.claude/plugins/your-plugin/skills/code-review/ -cp -r ai-code-review-guide/references ~/.claude/plugins/your-plugin/skills/code-review/ +# 复制整个目录结构 +cp -r ai-code-review-guide ~/.claude/plugins/your-plugin/skills/code-review/ ``` ### 使用方法 @@ -209,11 +240,16 @@ cp -r ai-code-review-guide/references ~/.claude/plugins/your-plugin/skills/code- ``` ai-code-review-guide/ -├── SKILL.md # 主技能定义 -├── README.md # 本文件 -├── LICENSE # MIT 许可证 -├── CONTRIBUTING.md # 贡献指南 +├── SKILL.md # 核心技能(立即加载) +├── README.md # 本文件 +├── LICENSE # MIT 许可证 +├── CONTRIBUTING.md # 贡献指南 ├── references/ +│ ├── react.md # React/Next.js 模式(按需加载) +│ ├── vue.md # Vue 3 模式(按需加载) +│ ├── rust.md # Rust 模式(按需加载) +│ ├── typescript.md # TypeScript/JS 模式(按需加载) +│ ├── python.md # Python 模式(按需加载) │ ├── common-bugs-checklist.md # 语言特定的错误模式 │ ├── security-review-guide.md # 安全审查清单 │ └── code-review-best-practices.md @@ -224,6 +260,16 @@ ai-code-review-guide/ └── pr-analyzer.py # PR 复杂度分析器 ``` +### 按需加载机制 + +此技能使用 **Progressive Disclosure(渐进式披露)** 来最小化上下文占用: + +1. **SKILL.md**(~180 行)在技能激活时加载 +2. **语言特定文件** 仅在审查该语言时加载 +3. **参考文件** 仅在明确需要时加载 + +这意味着审查 React PR 时只加载 SKILL.md + react.md,不会加载 Vue/Rust/Python 内容。 + ### 核心内容 #### React 19 @@ -269,13 +315,6 @@ ai-code-review-guide/ 本项目采用 MIT 许可证 - 详见 [LICENSE](LICENSE) 文件。 -### 致谢 - -- 灵感来自软件工程社区的代码审查最佳实践 -- React 19 特性来自 React 官方文档 -- TanStack Query 文档 -- TkDodo 的 React Query 最佳实践博客 - ### 参考资料 - [React v19 官方文档](https://react.dev/blog/2024/12/05/react-19) diff --git a/SKILL.md b/SKILL.md index 344e2eb..730b8e8 100644 --- a/SKILL.md +++ b/SKILL.md @@ -1,6 +1,9 @@ --- name: code-review-excellence -description: Master effective code review practices to provide constructive feedback, catch bugs early, and foster knowledge sharing while maintaining team morale. Use when reviewing pull requests, establishing review standards, or mentoring developers. +description: | + Master effective code review practices to provide constructive feedback, catch bugs early, and foster knowledge sharing while maintaining team morale. Use when reviewing pull requests, establishing review standards, or mentoring developers. + Supports: React 19, Vue 3, Rust, TypeScript, Python. + Triggers: "review", "PR", "pull request", "code review" --- # Code Review Excellence @@ -81,69 +84,29 @@ Transform code reviews from gatekeeping to knowledge sharing through constructiv ### Phase 1: Context Gathering (2-3 minutes) -```markdown Before diving into code, understand: - 1. Read PR description and linked issue 2. Check PR size (>400 lines? Ask to split) 3. Review CI/CD status (tests passing?) 4. Understand the business requirement 5. Note any relevant architectural decisions -``` ### Phase 2: High-Level Review (5-10 minutes) -```markdown -1. **Architecture & Design** - - Does the solution fit the problem? - - Are there simpler approaches? - - Is it consistent with existing patterns? - - Will it scale? - -2. **File Organization** - - Are new files in the right places? - - Is code grouped logically? - - Are there duplicate files? - -3. **Testing Strategy** - - Are there tests? - - Do tests cover edge cases? - - Are tests readable? -``` +1. **Architecture & Design** - Does the solution fit the problem? +2. **File Organization** - Are new files in the right places? +3. **Testing Strategy** - Are there tests covering edge cases? ### Phase 3: Line-by-Line Review (10-20 minutes) -```markdown -For each file: - -1. **Logic & Correctness** - - Edge cases handled? - - Off-by-one errors? - - Null/undefined checks? - - Race conditions? - -2. **Security** - - Input validation? - - SQL injection risks? - - XSS vulnerabilities? - - Sensitive data exposure? - -3. **Performance** - - N+1 queries? - - Unnecessary loops? - - Memory leaks? - - Blocking operations? - -4. **Maintainability** - - Clear variable names? - - Functions doing one thing? - - Complex code commented? - - Magic numbers extracted? -``` +For each file, check: +- **Logic & Correctness** - Edge cases, off-by-one, null checks, race conditions +- **Security** - Input validation, injection risks, XSS, sensitive data +- **Performance** - N+1 queries, unnecessary loops, memory leaks +- **Maintainability** - Clear names, single responsibility, comments ### Phase 4: Summary & Decision (2-3 minutes) -```markdown 1. Summarize key concerns 2. Highlight what you liked 3. Make clear decision: @@ -151,38 +114,16 @@ For each file: - 💬 Comment (minor suggestions) - 🔄 Request Changes (must address) 4. Offer to pair if complex -``` ## Review Techniques ### Technique 1: The Checklist Method -```markdown -## Security Checklist -- [ ] User input validated and sanitized -- [ ] SQL queries use parameterization -- [ ] Authentication/authorization checked -- [ ] Secrets not hardcoded -- [ ] Error messages don't leak info - -## Performance Checklist -- [ ] No N+1 queries -- [ ] Database queries indexed -- [ ] Large lists paginated -- [ ] Expensive operations cached -- [ ] No blocking I/O in hot paths - -## Testing Checklist -- [ ] Happy path tested -- [ ] Edge cases covered -- [ ] Error cases tested -- [ ] Test names are descriptive -- [ ] Tests are deterministic -``` +Use checklists for consistent reviews. See `references/security-review-guide.md` for comprehensive security checklist. ### Technique 2: The Question Approach -Instead of stating problems, ask questions to encourage thinking: +Instead of stating problems, ask questions: ```markdown ❌ "This will fail if the list is empty." @@ -190,1585 +131,47 @@ Instead of stating problems, ask questions to encourage thinking: ❌ "You need error handling here." ✅ "How should this behave if the API call fails?" - -❌ "This is inefficient." -✅ "I see this loops through all users. Have we considered - the performance impact with 100k users?" ``` ### Technique 3: Suggest, Don't Command -```markdown -## Use Collaborative Language +Use collaborative language: +```markdown ❌ "You must change this to use async/await" -✅ "Suggestion: async/await might make this more readable: - ```typescript - async function fetchUser(id: string) { - const user = await db.query('SELECT * FROM users WHERE id = ?', id); - return user; - } - ``` - What do you think?" +✅ "Suggestion: async/await might make this more readable. What do you think?" ❌ "Extract this into a function" -✅ "This logic appears in 3 places. Would it make sense to - extract it into a shared utility function?" +✅ "This logic appears in 3 places. Would it make sense to extract it?" ``` ### Technique 4: Differentiate Severity -```markdown Use labels to indicate priority: -🔴 [blocking] - Must fix before merge -🟡 [important] - Should fix, discuss if disagree -🟢 [nit] - Nice to have, not blocking -💡 [suggestion] - Alternative approach to consider -📚 [learning] - Educational comment, no action needed -🎉 [praise] - Good work, keep it up! - -Example: -"🔴 [blocking] This SQL query is vulnerable to injection. - Please use parameterized queries." - -"🟢 [nit] Consider renaming `data` to `userData` for clarity." - -"🎉 [praise] Excellent test coverage! This will catch edge cases." -``` - -## Language-Specific Patterns - -### Python Code Review - -```python -# Check for Python-specific issues - -# ❌ Mutable default arguments -def add_item(item, items=[]): # Bug! Shared across calls - items.append(item) - return items - -# ✅ Use None as default -def add_item(item, items=None): - if items is None: - items = [] - items.append(item) - return items - -# ❌ Catching too broad -try: - result = risky_operation() -except: # Catches everything, even KeyboardInterrupt! - pass - -# ✅ Catch specific exceptions -try: - result = risky_operation() -except ValueError as e: - logger.error(f"Invalid value: {e}") - raise - -# ❌ Using mutable class attributes -class User: - permissions = [] # Shared across all instances! - -# ✅ Initialize in __init__ -class User: - def __init__(self): - self.permissions = [] -``` - -### TypeScript/JavaScript Code Review - -```typescript -// Check for TypeScript-specific issues - -// ❌ Using any defeats type safety -function processData(data: any) { // Avoid any - return data.value; -} - -// ✅ Use proper types -interface DataPayload { - value: string; -} -function processData(data: DataPayload) { - return data.value; -} - -// ❌ Not handling async errors -async function fetchUser(id: string) { - const response = await fetch(`/api/users/${id}`); - return response.json(); // What if network fails? -} - -// ✅ Handle errors properly -async function fetchUser(id: string): Promise { - try { - const response = await fetch(`/api/users/${id}`); - if (!response.ok) { - throw new Error(`HTTP ${response.status}`); - } - return await response.json(); - } catch (error) { - console.error('Failed to fetch user:', error); - throw error; - } -} - -// ❌ Mutation of props -function UserProfile({ user }: Props) { - user.lastViewed = new Date(); // Mutating prop! - return
{user.name}
; -} - -// ✅ Don't mutate props -function UserProfile({ user, onView }: Props) { - useEffect(() => { - onView(user.id); // Notify parent to update - }, [user.id]); - return
{user.name}
; -} -``` - -### React Code Review - -React 审查重点:Hooks 规则、性能优化的适度性、组件设计、以及现代 React 19/RSC 模式。 - -```tsx -// === Hooks 规则 === - -// ❌ 条件调用 Hooks — 违反 Hooks 规则 -function BadComponent({ isLoggedIn }) { - if (isLoggedIn) { - const [user, setUser] = useState(null); // Error! - } - return
...
; -} - -// ✅ Hooks 必须在组件顶层调用 -function GoodComponent({ isLoggedIn }) { - const [user, setUser] = useState(null); - if (!isLoggedIn) return ; - return
{user?.name}
; -} - -// === useEffect 常见错误 === - -// ❌ 依赖数组缺失或不完整 -function BadEffect({ userId }) { - const [user, setUser] = useState(null); - useEffect(() => { - fetchUser(userId).then(setUser); - }, []); // 缺少 userId 依赖! -} - -// ✅ 完整的依赖数组 -function GoodEffect({ userId }) { - const [user, setUser] = useState(null); - useEffect(() => { - let cancelled = false; - fetchUser(userId).then(data => { - if (!cancelled) setUser(data); - }); - return () => { cancelled = true; }; // 清理函数 - }, [userId]); -} - -// ❌ useEffect 用于派生状态(反模式) -function BadDerived({ items }) { - const [filteredItems, setFilteredItems] = useState([]); - useEffect(() => { - setFilteredItems(items.filter(i => i.active)); - }, [items]); // 不必要的 effect + 额外渲染 - return ; -} - -// ✅ 直接在渲染时计算,或用 useMemo -function GoodDerived({ items }) { - const filteredItems = useMemo( - () => items.filter(i => i.active), - [items] - ); - return ; -} - -// ❌ useEffect 用于事件响应 -function BadEventEffect() { - const [query, setQuery] = useState(''); - useEffect(() => { - if (query) { - analytics.track('search', { query }); // 应该在事件处理器中 - } - }, [query]); -} - -// ✅ 在事件处理器中执行副作用 -function GoodEvent() { - const [query, setQuery] = useState(''); - const handleSearch = (q: string) => { - setQuery(q); - analytics.track('search', { query: q }); - }; -} - -// === useMemo / useCallback 过度使用 === - -// ❌ 过度优化 — 常量不需要 useMemo -function OverOptimized() { - const config = useMemo(() => ({ timeout: 5000 }), []); // 无意义 - const handleClick = useCallback(() => { - console.log('clicked'); - }, []); // 如果不传给 memo 组件,无意义 -} - -// ✅ 只在需要时优化 -function ProperlyOptimized() { - const config = { timeout: 5000 }; // 简单对象直接定义 - const handleClick = () => console.log('clicked'); -} - -// ❌ useCallback 依赖总是变化 -function BadCallback({ data }) { - // data 每次渲染都是新对象,useCallback 无效 - const process = useCallback(() => { - return data.map(transform); - }, [data]); -} - -// ✅ useMemo + useCallback 配合 React.memo 使用 -const MemoizedChild = React.memo(function Child({ onClick, items }) { - return
{items.length}
; -}); - -function Parent({ rawItems }) { - const items = useMemo(() => processItems(rawItems), [rawItems]); - const handleClick = useCallback(() => { - console.log(items.length); - }, [items]); - return ; -} - -// === 组件设计 === - -// ❌ 在组件内定义组件 — 每次渲染都创建新组件 -function BadParent() { - function ChildComponent() { // 每次渲染都是新函数! - return
child
; - } - return ; -} - -// ✅ 组件定义在外部 -function ChildComponent() { - return
child
; -} -function GoodParent() { - return ; -} - -// ❌ Props 总是新对象引用 -function BadProps() { - return ( - {}} // 每次渲染新函数 - /> - ); -} - -// ✅ 稳定的引用 -const style = { color: 'red' }; -function GoodProps() { - const handleClick = useCallback(() => {}, []); - return ; -} - -// === Error Boundaries & Suspense === - -// ❌ 没有错误边界 -function BadApp() { - return ( - }> - {/* 错误会导致整个应用崩溃 */} - - ); -} - -// ✅ Error Boundary 包裹 Suspense -function GoodApp() { - return ( - }> - }> - - - - ); -} - -// === React 19 / Server Components === - -// ❌ 在 Server Component 中使用客户端特性 -// app/page.tsx (Server Component by default) -function BadServerComponent() { - const [count, setCount] = useState(0); // Error! No hooks in RSC - return ; -} - -// ✅ 交互逻辑提取到 Client Component -// app/counter.tsx -'use client'; -function Counter() { - const [count, setCount] = useState(0); - return ; -} - -// app/page.tsx (Server Component) -function GoodServerComponent() { - const data = await fetchData(); // 可以直接 await - return ( -
-

{data.title}

- {/* 客户端组件 */} -
- ); -} - -// ❌ 'use client' 放置不当 — 整个树都变成客户端 -// layout.tsx -'use client'; // 这会让所有子组件都成为客户端组件 -export default function Layout({ children }) { ... } - -// ✅ 只在需要交互的组件使用 'use client' -// 将客户端逻辑隔离到叶子组件 -``` - -#### React Review Checklist - -```markdown -## Hooks 规则 - -- [ ] Hooks 在组件/自定义 Hook 顶层调用 -- [ ] 没有条件/循环中调用 Hooks -- [ ] useEffect 依赖数组完整 -- [ ] useEffect 有清理函数(订阅/定时器/请求) -- [ ] 没有用 useEffect 计算派生状态 - -## 性能优化(适度原则) - -- [ ] useMemo/useCallback 只用于真正需要的场景 -- [ ] React.memo 配合稳定的 props 引用 -- [ ] 没有在组件内定义子组件 -- [ ] 没有在 JSX 中创建新对象/函数(除非传给非 memo 组件) -- [ ] 长列表使用虚拟化(react-window/react-virtual) - -## 组件设计 - -- [ ] 组件职责单一,不超过 200 行 -- [ ] 逻辑与展示分离(Custom Hooks) -- [ ] Props 接口清晰,使用 TypeScript -- [ ] 避免 Props Drilling(考虑 Context 或组合) - -## 状态管理 - -- [ ] 状态就近原则(最小必要范围) -- [ ] 复杂状态用 useReducer -- [ ] 全局状态用 Context 或状态库 -- [ ] 避免不必要的状态(派生 > 存储) - -## 错误处理 - -- [ ] 关键区域有 Error Boundary -- [ ] Suspense 配合 Error Boundary 使用 -- [ ] 异步操作有错误处理 - -## Server Components (RSC) - -- [ ] 'use client' 只用于需要交互的组件 -- [ ] Server Component 不使用 Hooks/事件处理 -- [ ] 客户端组件尽量放在叶子节点 -- [ ] 数据获取在 Server Component 中进行 - -## 测试 - -- [ ] 使用 @testing-library/react -- [ ] 用 screen 查询元素 -- [ ] 用 userEvent 代替 fireEvent -- [ ] 优先使用 *ByRole 查询 -- [ ] 测试行为而非实现细节 -``` - -### React 19 Actions & Forms - -React 19 引入了 Actions 系统和新的表单处理 Hooks,简化异步操作和乐观更新。 - -```tsx -// === useActionState (替代 useFormState) === - -// ❌ 传统方式:多个状态变量 -function OldForm() { - const [isPending, setIsPending] = useState(false); - const [error, setError] = useState(null); - const [data, setData] = useState(null); - - const handleSubmit = async (formData: FormData) => { - setIsPending(true); - setError(null); - try { - const result = await submitForm(formData); - setData(result); - } catch (e) { - setError(e.message); - } finally { - setIsPending(false); - } - }; -} - -// ✅ React 19: useActionState 统一管理 -import { useActionState } from 'react'; - -function NewForm() { - const [state, formAction, isPending] = useActionState( - async (prevState, formData: FormData) => { - try { - const result = await submitForm(formData); - return { success: true, data: result }; - } catch (e) { - return { success: false, error: e.message }; - } - }, - { success: false, data: null, error: null } - ); - - return ( -
- - - {state.error &&

{state.error}

} -
- ); -} - -// === useFormStatus === - -// ❌ Props 透传表单状态 -function BadSubmitButton({ isSubmitting }) { - return ; -} - -// ✅ useFormStatus 访问父
状态(无需 props) -import { useFormStatus } from 'react-dom'; - -function SubmitButton() { - const { pending, data, method, action } = useFormStatus(); - // 注意:必须在 内部的子组件中使用 - return ( - - ); -} - -// ❌ useFormStatus 在 form 同级组件中调用——不工作 -function BadForm() { - const { pending } = useFormStatus(); // 这里无法获取状态! - return ( - - -
- ); -} - -// ✅ useFormStatus 必须在 form 的子组件中 -function GoodForm() { - return ( -
- {/* useFormStatus 在这里面调用 */} - - ); -} - -// === useOptimistic === - -// ❌ 等待服务器响应再更新 UI -function SlowLike({ postId, likes }) { - const [likeCount, setLikeCount] = useState(likes); - const [isPending, setIsPending] = useState(false); - - const handleLike = async () => { - setIsPending(true); - const newCount = await likePost(postId); // 等待... - setLikeCount(newCount); - setIsPending(false); - }; -} - -// ✅ useOptimistic 即时反馈,失败自动回滚 -import { useOptimistic } from 'react'; - -function FastLike({ postId, likes }) { - const [optimisticLikes, addOptimisticLike] = useOptimistic( - likes, - (currentLikes, increment: number) => currentLikes + increment - ); - - const handleLike = async () => { - addOptimisticLike(1); // 立即更新 UI - try { - await likePost(postId); // 后台同步 - } catch { - // React 自动回滚到 likes 原值 - } - }; - - return ; -} - -// === 配合 Server Actions (Next.js 15+) === - -// ❌ 客户端调用 API -'use client'; -function ClientForm() { - const handleSubmit = async (formData: FormData) => { - const res = await fetch('/api/submit', { - method: 'POST', - body: formData, - }); - // ... - }; -} - -// ✅ Server Action + useActionState -// actions.ts -'use server'; -export async function createPost(prevState: any, formData: FormData) { - const title = formData.get('title'); - await db.posts.create({ title }); - revalidatePath('/posts'); - return { success: true }; -} - -// form.tsx -'use client'; -import { createPost } from './actions'; - -function PostForm() { - const [state, formAction, isPending] = useActionState(createPost, null); - return ( -
- - - - ); -} -``` - -#### React 19 Forms Checklist - -```markdown -## useActionState -- [ ] 使用 useActionState 替代多个 useState(isPending/error/data) -- [ ] Action 函数返回新状态,不是直接 setState -- [ ] 正确处理错误状态和成功状态 -- [ ] 配合 Server Actions 时标记 'use server' - -## useFormStatus -- [ ] 在
的子组件中调用,不是同级 -- [ ] 不依赖 props 传递表单状态 -- [ ] 只用于读取状态,不用于触发提交 - -## useOptimistic -- [ ] 用于需要即时反馈的操作(点赞、收藏等) -- [ ] 提供合理的乐观更新逻辑 -- [ ] 服务端失败时 UI 会自动回滚 -- [ ] 不用于关键数据(如支付) -``` - -### Suspense & Streaming SSR - -Suspense 和 Streaming 是 React 18+ 的核心特性,在 2025 年的 Next.js 15 等框架中广泛使用。 - -```tsx -// === 基础 Suspense 用法 === - -// ❌ 传统加载状态管理 -function OldComponent() { - const [data, setData] = useState(null); - const [isLoading, setIsLoading] = useState(true); - - useEffect(() => { - fetchData().then(setData).finally(() => setIsLoading(false)); - }, []); - - if (isLoading) return ; - return ; -} - -// ✅ Suspense 声明式加载状态 -function NewComponent() { - return ( - }> - {/* 内部使用 use() 或支持 Suspense 的数据获取 */} - - ); -} - -// === 多个独立 Suspense 边界 === - -// ❌ 单一边界——所有内容一起加载 -function BadLayout() { - return ( - }> -
- {/* 慢 */} - {/* 快 */} - - ); -} - -// ✅ 独立边界——各部分独立流式传输 -function GoodLayout() { - return ( - <> -
{/* 立即显示 */} -
- }> - {/* 独立加载 */} - - }> - {/* 独立加载 */} - -
- - ); -} - -// === Error Boundary 配合 Suspense === - -// ❌ Suspense 没有错误处理 -function NoErrorHandling() { - return ( - }> - {/* 抛错会导致整个应用崩溃 */} - - ); -} - -// ✅ Error Boundary 包裹 Suspense -import { ErrorBoundary } from 'react-error-boundary'; - -function WithErrorHandling() { - return ( - } - onError={(error) => logError(error)} - > - }> - - - - ); -} - -// === Next.js 15 Streaming === - -// app/page.tsx - 自动 Streaming -export default async function Page() { - // 这个 await 不会阻塞整个页面 - const data = await fetchSlowData(); - return
{data}
; -} - -// app/loading.tsx - 自动 Suspense 边界 -export default function Loading() { - return ; -} - -// === 嵌套 Suspense 策略 === - -// ❌ 过深嵌套——难以调试 -function OverNested() { - return ( - - - - - - - - - - ); -} - -// ✅ 按用户体验需求分层 -function WellStructured() { - return ( - }> {/* 应用骨架 */} - - }> - {/* 导航优先 */} - - }> - {/* 主内容 */} - - - - ); -} - -// === use() Hook (React 19) === - -// ✅ 在组件中读取 Promise -import { use } from 'react'; - -function Comments({ commentsPromise }) { - const comments = use(commentsPromise); // 自动触发 Suspense - return ( -
    - {comments.map(c =>
  • {c.text}
  • )} -
- ); -} - -// 父组件创建 Promise,子组件消费 -function Post({ postId }) { - const commentsPromise = fetchComments(postId); // 不 await - return ( -
- - }> - - -
- ); -} -``` - -#### Suspense & Streaming Checklist - -```markdown -## Suspense 边界设计 -- [ ] 按用户体验需求划分 Suspense 边界 -- [ ] 独立内容使用独立的 Suspense 边界 -- [ ] 避免过深嵌套(2-3 层为宜) -- [ ] 提供有意义的 fallback(骨架屏 > 简单 Spinner) - -## 错误处理 -- [ ] 每个 Suspense 有对应的 Error Boundary -- [ ] Error Boundary 提供恢复机制(retry 按钮) -- [ ] 错误被正确记录和上报 - -## Streaming SSR (Next.js) -- [ ] 慢数据使用 Suspense 包裹,不阻塞整个页面 -- [ ] loading.tsx 用于路由级别加载状态 -- [ ] 理解 Server Component vs Client Component 边界 -- [ ] 避免在 layout 层级使用会触发全页 Suspense 的数据获取 - -## 性能考虑 -- [ ] 优先显示重要内容(Navigation, Header) -- [ ] 非关键内容延迟加载 -- [ ] 使用 startTransition 处理非紧急更新 -``` - -### TanStack Query (React Query) v5 - -TanStack Query 是 React 生态中最流行的数据获取库,v5 是当前稳定版本。 - -```tsx -// === 基础配置 === - -// ❌ 不正确的默认配置 -const queryClient = new QueryClient(); // 默认配置可能不适合 - -// ✅ 生产环境推荐配置 -const queryClient = new QueryClient({ - defaultOptions: { - queries: { - staleTime: 1000 * 60 * 5, // 5 分钟内数据视为新鲜 - gcTime: 1000 * 60 * 30, // 30 分钟后垃圾回收(v5 重命名) - retry: 3, - refetchOnWindowFocus: false, // 根据需求决定 - }, - }, -}); - -// === 使用 queryOptions (v5 新增) === - -// ❌ 重复定义 queryKey 和 queryFn -function Component1() { - const { data } = useQuery({ - queryKey: ['users', userId], - queryFn: () => fetchUser(userId), - }); -} - -function prefetchUser(queryClient, userId) { - queryClient.prefetchQuery({ - queryKey: ['users', userId], // 重复! - queryFn: () => fetchUser(userId), // 重复! - }); -} - -// ✅ queryOptions 统一定义,类型安全 -import { queryOptions } from '@tanstack/react-query'; - -const userQueryOptions = (userId: string) => - queryOptions({ - queryKey: ['users', userId], - queryFn: () => fetchUser(userId), - }); - -function Component1({ userId }) { - const { data } = useQuery(userQueryOptions(userId)); -} - -function prefetchUser(queryClient, userId) { - queryClient.prefetchQuery(userQueryOptions(userId)); -} - -// getQueryData 也是类型安全的 -const user = queryClient.getQueryData(userQueryOptions(userId).queryKey); - -// === 避免常见陷阱 === - -// ❌ staleTime 为 0 导致过度请求 -useQuery({ - queryKey: ['data'], - queryFn: fetchData, - // staleTime 默认为 0,每次组件挂载都会 refetch -}); - -// ✅ 设置合理的 staleTime -useQuery({ - queryKey: ['data'], - queryFn: fetchData, - staleTime: 1000 * 60, // 1 分钟内不会重新请求 -}); - -// ❌ 在 queryFn 中使用不稳定的引用 -function BadQuery({ filters }) { - useQuery({ - queryKey: ['items'], // queryKey 没有包含 filters! - queryFn: () => fetchItems(filters), // filters 变化不会触发重新请求 - }); -} - -// ✅ queryKey 包含所有影响数据的参数 -function GoodQuery({ filters }) { - useQuery({ - queryKey: ['items', filters], // filters 是 queryKey 的一部分 - queryFn: () => fetchItems(filters), - }); -} - -// === Suspense 模式 (v5 稳定) === - -// ❌ 使用 useQuery + enabled 实现条件查询 -function BadSuspenseQuery({ userId }) { - const { data } = useSuspenseQuery({ - queryKey: ['user', userId], - queryFn: () => fetchUser(userId), - enabled: !!userId, // useSuspenseQuery 不支持 enabled! - }); -} - -// ✅ 组件组合实现条件渲染 -function GoodSuspenseQuery({ userId }) { - // useSuspenseQuery 保证 data 是 T 不是 T | undefined - const { data } = useSuspenseQuery({ - queryKey: ['user', userId], - queryFn: () => fetchUser(userId), - }); - return ; -} - -function Parent({ userId }) { - if (!userId) return ; - return ( - }> - - - ); -} - -// === 乐观更新 (v5 简化) === - -// ❌ 手动管理缓存的乐观更新(复杂) -const mutation = useMutation({ - mutationFn: updateTodo, - onMutate: async (newTodo) => { - await queryClient.cancelQueries({ queryKey: ['todos'] }); - const previousTodos = queryClient.getQueryData(['todos']); - queryClient.setQueryData(['todos'], (old) => [...old, newTodo]); - return { previousTodos }; - }, - onError: (err, newTodo, context) => { - queryClient.setQueryData(['todos'], context.previousTodos); - }, - onSettled: () => { - queryClient.invalidateQueries({ queryKey: ['todos'] }); - }, -}); - -// ✅ v5 简化:使用 variables 进行乐观 UI -function TodoList() { - const { data: todos } = useQuery(todosQueryOptions); - const { mutate, variables, isPending } = useMutation({ - mutationFn: addTodo, - onSuccess: () => { - queryClient.invalidateQueries({ queryKey: ['todos'] }); - }, - }); - - return ( -
    - {todos?.map(todo => )} - {/* 乐观显示正在添加的 todo */} - {isPending && } -
- ); -} - -// === 状态字段变化 (v5) === - -// v4: isLoading 表示首次加载或后续获取 -// v5: isPending 表示没有数据,isLoading = isPending && isFetching - -const { data, isPending, isFetching, isLoading } = useQuery({...}); - -// isPending: 缓存中没有数据(首次加载) -// isFetching: 正在请求中(包括后台刷新) -// isLoading: isPending && isFetching(首次加载中) - -// ❌ v4 代码直接迁移 -if (isLoading) return ; // v5 中行为可能不同 - -// ✅ 明确意图 -if (isPending) return ; // 没有数据时显示加载 -// 或 -if (isLoading) return ; // 首次加载中 -``` - -#### TanStack Query Checklist - -```markdown -## 查询配置 -- [ ] 使用 queryOptions 统一定义查询 -- [ ] queryKey 包含所有影响数据的参数 -- [ ] 设置合理的 staleTime(不是默认的 0) -- [ ] gcTime(原 cacheTime)根据需求配置 - -## Suspense 模式 -- [ ] 使用 useSuspenseQuery(不是 useQuery + suspense: true) -- [ ] 不在 useSuspenseQuery 中使用 enabled -- [ ] 条件查询通过组件组合实现 - -## Mutations -- [ ] 成功后 invalidateQueries 相关查询 -- [ ] 使用 variables 实现简单乐观更新 -- [ ] 复杂场景使用 onMutate/onError/onSettled - -## 性能 -- [ ] 避免过多 useQuery 调用(>100 个查询可能影响性能) -- [ ] 大列表考虑分页或虚拟化 -- [ ] 使用 select 只订阅需要的数据 - -## v5 迁移 -- [ ] cacheTime → gcTime -- [ ] 理解 isPending vs isLoading 区别 -- [ ] useQuery 单一对象参数(移除重载) -``` - -### Vue 3 Code Review - -```vue - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -``` - -#### Vue 3 Review Checklist - -```markdown -## 响应性系统 -- [ ] ref 用于基本类型,reactive 用于对象 -- [ ] 没有解构 reactive 对象(或使用了 toRefs) -- [ ] props 传递给 composable 时保持了响应性 -- [ ] shallowRef/shallowReactive 用于大型对象优化 - -## Props & Emits -- [ ] defineProps 使用 TypeScript 类型声明 -- [ ] 复杂默认值使用 withDefaults + 工厂函数 -- [ ] defineEmits 有完整的类型定义 -- [ ] 没有直接修改 props - -## Watchers -- [ ] watch/watchEffect 有适当的清理函数 -- [ ] 异步 watch 处理了竞态条件 -- [ ] flush: 'post' 用于 DOM 操作的 watcher -- [ ] 避免过度使用 watcher(优先用 computed) - -## 模板 -- [ ] v-for 使用唯一且稳定的 key -- [ ] v-if 和 v-for 没有在同一元素上 -- [ ] 事件处理使用 kebab-case -- [ ] 大型列表使用虚拟滚动 - -## Composables -- [ ] 相关逻辑提取到 composables -- [ ] composables 返回响应式引用(不是 .value) -- [ ] 纯函数不要包装成 composable -- [ ] 副作用在组件卸载时清理 - -## 性能 -- [ ] 大型组件拆分为小组件 -- [ ] 使用 defineAsyncComponent 懒加载 -- [ ] 避免不必要的响应式转换 -- [ ] v-memo 用于昂贵的列表渲染 -``` - -### Rust Code Review - -Rust 审查的重点:编译器能捕获内存安全问题,但审查者需要关注编译器无法检测的问题——业务逻辑、API 设计、性能和可维护性。 - -```rust -// === 所有权与借用 === - -// ❌ clone() 是"Rust 的胶带"——用于绕过借用检查器 -fn bad_process(data: &Data) -> Result<()> { - let owned = data.clone(); // 为什么需要 clone? - expensive_operation(owned) -} - -// ✅ 审查时问:clone 是否必要?能否用借用? -fn good_process(data: &Data) -> Result<()> { - expensive_operation(data) // 传递引用 -} - -// ❌ Arc> 可能隐藏不必要的共享状态 -struct BadService { - cache: Arc>>, // 真的需要共享? -} - -// ✅ 考虑是否需要共享,或者设计可以避免 -struct GoodService { - cache: HashMap, // 单一所有者 -} - -// === Unsafe 代码审查(最关键!)=== - -// ❌ unsafe 没有安全文档——这是红旗 -unsafe fn bad_transmute(t: T) -> U { - std::mem::transmute(t) -} - -// ✅ 每个 unsafe 必须解释:为什么安全?什么不变量? -/// Transmutes `T` to `U`. -/// -/// # Safety -/// -/// - `T` and `U` must have the same size and alignment -/// - `T` must be a valid bit pattern for `U` -/// - The caller ensures no references to `t` exist after this call -unsafe fn documented_transmute(t: T) -> U { - // SAFETY: Caller guarantees size/alignment match and bit validity - std::mem::transmute(t) -} - -// === 异步代码 === - -// ❌ 在 async 上下文中阻塞——会饿死其他任务 -async fn bad_async() { - let data = std::fs::read_to_string("file.txt").unwrap(); // 阻塞! - std::thread::sleep(Duration::from_secs(1)); // 阻塞! -} - -// ✅ 使用异步 API -async fn good_async() { - let data = tokio::fs::read_to_string("file.txt").await?; - tokio::time::sleep(Duration::from_secs(1)).await; -} - -// ❌ 跨 .await 持有 std::sync::Mutex——可能死锁 -async fn bad_lock(mutex: &std::sync::Mutex) { - let guard = mutex.lock().unwrap(); - async_operation().await; // 持锁等待! - process(&guard); -} - -// ✅ 最小化锁范围,或使用 tokio::sync::Mutex -async fn good_lock(mutex: &std::sync::Mutex) { - let data = mutex.lock().unwrap().clone(); // 立即释放 - async_operation().await; - process(&data); -} - -// === 错误处理 === - -// ❌ 库代码用 anyhow——调用者无法 match 错误 -pub fn parse_config(s: &str) -> anyhow::Result { ... } - -// ✅ 库用 thiserror,应用用 anyhow -#[derive(Debug, thiserror::Error)] -pub enum ConfigError { - #[error("invalid syntax at line {line}: {message}")] - Syntax { line: usize, message: String }, - #[error("missing required field: {0}")] - MissingField(String), - #[error(transparent)] - Io(#[from] std::io::Error), -} - -pub fn parse_config(s: &str) -> Result { ... } - -// ❌ 吞掉错误上下文 -fn bad_error() -> Result<()> { - operation().map_err(|_| anyhow!("failed"))?; // 原始错误丢失 - Ok(()) -} - -// ✅ 使用 context 保留错误链 -fn good_error() -> Result<()> { - operation().context("failed to perform operation")?; - Ok(()) -} - -// === 性能 === - -// ❌ 不必要的 collect——中间分配 -fn bad_sum(items: &[i32]) -> i32 { - items.iter() - .filter(|x| **x > 0) - .collect::>() // 不必要! - .iter() - .sum() -} - -// ✅ 惰性迭代 -fn good_sum(items: &[i32]) -> i32 { - items.iter().filter(|x| **x > 0).sum() -} - -// ❌ 字符串拼接在循环中重复分配 -fn bad_concat(items: &[&str]) -> String { - let mut s = String::new(); - for item in items { - s = s + item; // 每次都重新分配! - } - s -} - -// ✅ 预分配或用 join -fn good_concat(items: &[&str]) -> String { - items.join("") // 或用 with_capacity -} - -// === Trait 设计 === - -// ❌ 过度抽象——不是 Java,不需要 Interface 一切 -trait Processor { fn process(&self); } -trait Handler { fn handle(&self); } -trait Manager { fn manage(&self); } // Trait 过多 - -// ✅ 只在需要多态时创建 trait -// 具体类型通常更简单、更快 -``` - -#### Rust Review Checklist - -```markdown -## 编译器不能捕获的问题 - -**业务逻辑正确性** -- [ ] 边界条件处理正确 -- [ ] 状态机转换完整 -- [ ] 并发场景下的竞态条件 - -**API 设计** -- [ ] 公共 API 难以误用 -- [ ] 类型签名清晰表达意图 -- [ ] 错误类型粒度合适 - -## 所有权与借用 - -- [ ] clone() 是有意为之,文档说明了原因 -- [ ] Arc> 真的需要共享状态吗? -- [ ] RefCell 的使用有正当理由 -- [ ] 生命周期不过度复杂 - -## Unsafe 代码(最重要) - -- [ ] 每个 unsafe 块有 SAFETY 注释 -- [ ] unsafe fn 有 # Safety 文档节 -- [ ] 解释了为什么是安全的,不只是做什么 -- [ ] 列出了必须维护的不变量 -- [ ] unsafe 边界尽可能小 -- [ ] 考虑过是否有 safe 替代方案 - -## 异步/并发 - -- [ ] 没有在 async 中阻塞(std::fs、thread::sleep) -- [ ] 没有跨 .await 持有 std::sync 锁 -- [ ] spawn 的任务满足 'static -- [ ] 锁的获取顺序一致 -- [ ] Channel 缓冲区大小合理 - -## 错误处理 - -- [ ] 库:thiserror 定义结构化错误 -- [ ] 应用:anyhow + context -- [ ] 没有生产代码 unwrap/expect -- [ ] 错误消息对调试有帮助 -- [ ] must_use 返回值被处理 - -## 性能 - -- [ ] 避免不必要的 collect() -- [ ] 大数据传引用 -- [ ] 字符串用 with_capacity 或 write! -- [ ] impl Trait vs Box 选择合理 -- [ ] 热路径避免分配 - -## 代码质量 - -- [ ] cargo clippy 零警告 -- [ ] cargo fmt 格式化 -- [ ] 文档注释完整 -- [ ] 测试覆盖边界条件 -- [ ] 公共 API 有文档示例 -``` - -## Advanced Review Patterns - -### Pattern 1: Architectural Review - -```markdown -When reviewing significant changes: - -1. **Design Document First** - - For large features, request design doc before code - - Review design with team before implementation - - Agree on approach to avoid rework - -2. **Review in Stages** - - First PR: Core abstractions and interfaces - - Second PR: Implementation - - Third PR: Integration and tests - - Easier to review, faster to iterate - -3. **Consider Alternatives** - - "Have we considered using [pattern/library]?" - - "What's the tradeoff vs. the simpler approach?" - - "How will this evolve as requirements change?" -``` - -### Pattern 2: Test Quality Review - -```typescript -// ❌ Poor test: Implementation detail testing -test('increments counter variable', () => { - const component = render(); - const button = component.getByRole('button'); - fireEvent.click(button); - expect(component.state.counter).toBe(1); // Testing internal state -}); - -// ✅ Good test: Behavior testing -test('displays incremented count when clicked', () => { - render(); - const button = screen.getByRole('button', { name: /increment/i }); - fireEvent.click(button); - expect(screen.getByText('Count: 1')).toBeInTheDocument(); -}); - -// Review questions for tests: -// - Do tests describe behavior, not implementation? -// - Are test names clear and descriptive? -// - Do tests cover edge cases? -// - Are tests independent (no shared state)? -// - Can tests run in any order? -``` - -### Pattern 3: Security Review - -```markdown -## Security Review Checklist - -### Authentication & Authorization -- [ ] Is authentication required where needed? -- [ ] Are authorization checks before every action? -- [ ] Is JWT validation proper (signature, expiry)? -- [ ] Are API keys/secrets properly secured? - -### Input Validation -- [ ] All user inputs validated? -- [ ] File uploads restricted (size, type)? -- [ ] SQL queries parameterized? -- [ ] XSS protection (escape output)? - -### Data Protection -- [ ] Passwords hashed (bcrypt/argon2)? -- [ ] Sensitive data encrypted at rest? -- [ ] HTTPS enforced for sensitive data? -- [ ] PII handled according to regulations? - -### Common Vulnerabilities -- [ ] No eval() or similar dynamic execution? -- [ ] No hardcoded secrets? -- [ ] CSRF protection for state-changing operations? -- [ ] Rate limiting on public endpoints? -``` - -## Giving Difficult Feedback - -### Pattern: The Sandwich Method (Modified) - -```markdown -Traditional: Praise + Criticism + Praise (feels fake) - -Better: Context + Specific Issue + Helpful Solution - -Example: -"I noticed the payment processing logic is inline in the -controller. This makes it harder to test and reuse. - -[Specific Issue] -The calculateTotal() function mixes tax calculation, -discount logic, and database queries, making it difficult -to unit test and reason about. - -[Helpful Solution] -Could we extract this into a PaymentService class? That -would make it testable and reusable. I can pair with you -on this if helpful." -``` - -### Handling Disagreements - -```markdown -When author disagrees with your feedback: - -1. **Seek to Understand** - "Help me understand your approach. What led you to - choose this pattern?" - -2. **Acknowledge Valid Points** - "That's a good point about X. I hadn't considered that." - -3. **Provide Data** - "I'm concerned about performance. Can we add a benchmark - to validate the approach?" - -4. **Escalate if Needed** - "Let's get [architect/senior dev] to weigh in on this." - -5. **Know When to Let Go** - If it's working and not a critical issue, approve it. - Perfection is the enemy of progress. -``` - -## Best Practices - -1. **Review Promptly**: Within 24 hours, ideally same day -2. **Limit PR Size**: 200-400 lines max for effective review -3. **Review in Time Blocks**: 60 minutes max, take breaks -4. **Use Review Tools**: GitHub, GitLab, or dedicated tools -5. **Automate What You Can**: Linters, formatters, security scans -6. **Build Rapport**: Emoji, praise, and empathy matter -7. **Be Available**: Offer to pair on complex issues -8. **Learn from Others**: Review others' review comments - -## Common Pitfalls - -- **Perfectionism**: Blocking PRs for minor style preferences -- **Scope Creep**: "While you're at it, can you also..." -- **Inconsistency**: Different standards for different people -- **Delayed Reviews**: Letting PRs sit for days -- **Ghosting**: Requesting changes then disappearing -- **Rubber Stamping**: Approving without actually reviewing -- **Bike Shedding**: Debating trivial details extensively - -## Templates - -### PR Review Comment Template - -```markdown -## Summary -[Brief overview of what was reviewed] - -## Strengths -- [What was done well] -- [Good patterns or approaches] - -## Required Changes -🔴 [Blocking issue 1] -🔴 [Blocking issue 2] - -## Suggestions -💡 [Improvement 1] -💡 [Improvement 2] - -## Questions -❓ [Clarification needed on X] -❓ [Alternative approach consideration] - -## Verdict -✅ Approve after addressing required changes -``` - -## Resources - -- **references/code-review-best-practices.md**: Comprehensive review guidelines -- **references/common-bugs-checklist.md**: Language-specific bugs to watch for -- **references/security-review-guide.md**: Security-focused review checklist -- **assets/pr-review-template.md**: Standard review comment template -- **assets/review-checklist.md**: Quick reference checklist -- **scripts/pr-analyzer.py**: Analyze PR complexity and suggest reviewers +- 🔴 `[blocking]` - Must fix before merge +- 🟡 `[important]` - Should fix, discuss if disagree +- 🟢 `[nit]` - Nice to have, not blocking +- 💡 `[suggestion]` - Alternative approach to consider +- 📚 `[learning]` - Educational comment, no action needed +- 🎉 `[praise]` - Good work, keep it up! + +## Language-Specific Guides + +根据审查的代码语言,查阅对应的详细指南: + +| Language/Framework | Reference File | Key Topics | +|-------------------|----------------|------------| +| **React** | `references/react.md` | Hooks, useEffect, React 19 Actions, RSC, Suspense, TanStack Query v5 | +| **Vue 3** | `references/vue.md` | Composition API, 响应性系统, Props/Emits, Watchers, Composables | +| **Rust** | `references/rust.md` | 所有权/借用, Unsafe 审查, 异步代码, 错误处理 | +| **TypeScript** | `references/typescript.md` | 类型安全, async/await, 不可变性 | +| **Python** | `references/python.md` | 可变默认参数, 异常处理, 类属性 | + +## Additional Resources + +- `references/common-bugs-checklist.md` - 按语言分类的常见错误清单 +- `references/security-review-guide.md` - 安全审查指南 +- `references/code-review-best-practices.md` - 代码审查最佳实践 +- `assets/pr-review-template.md` - PR 审查评论模板 +- `assets/review-checklist.md` - 快速参考清单 diff --git a/references/python.md b/references/python.md new file mode 100644 index 0000000..abcf22c --- /dev/null +++ b/references/python.md @@ -0,0 +1,77 @@ +# Python Code Review Guide + +> Python 代码审查指南,覆盖常见陷阱和最佳实践。 + +--- + +## 常见陷阱 + +### 可变默认参数 + +```python +# ❌ Mutable default arguments +def add_item(item, items=[]): # Bug! Shared across calls + items.append(item) + return items + +# ✅ Use None as default +def add_item(item, items=None): + if items is None: + items = [] + items.append(item) + return items +``` + +### 异常捕获过宽 + +```python +# ❌ Catching too broad +try: + result = risky_operation() +except: # Catches everything, even KeyboardInterrupt! + pass + +# ✅ Catch specific exceptions +try: + result = risky_operation() +except ValueError as e: + logger.error(f"Invalid value: {e}") + raise +``` + +### 可变类属性 + +```python +# ❌ Using mutable class attributes +class User: + permissions = [] # Shared across all instances! + +# ✅ Initialize in __init__ +class User: + def __init__(self): + self.permissions = [] +``` + +--- + +## Python Review Checklist + +### 数据结构 +- [ ] 没有使用可变默认参数(list、dict、set) +- [ ] 类属性不是可变对象 +- [ ] 理解浅拷贝和深拷贝的区别 + +### 异常处理 +- [ ] 捕获特定异常类型,不使用裸 `except:` +- [ ] 异常信息有意义,便于调试 +- [ ] 必要时重新抛出异常(`raise`) + +### 性能 +- [ ] 大数据集使用生成器而非列表 +- [ ] 避免循环中重复创建对象 +- [ ] 使用 `collections` 模块的高效数据结构 + +### 代码风格 +- [ ] 遵循 PEP 8 风格指南 +- [ ] 使用类型注解(type hints) +- [ ] 函数和类有 docstring diff --git a/references/react.md b/references/react.md new file mode 100644 index 0000000..f41bf31 --- /dev/null +++ b/references/react.md @@ -0,0 +1,799 @@ +# React Code Review Guide + +React 审查重点:Hooks 规则、性能优化的适度性、组件设计、以及现代 React 19/RSC 模式。 + +## 目录 + +- [基础 Hooks 规则](#基础-hooks-规则) +- [useEffect 模式](#useeffect-模式) +- [useMemo / useCallback](#usememo--usecallback) +- [组件设计](#组件设计) +- [Error Boundaries & Suspense](#error-boundaries--suspense) +- [Server Components (RSC)](#server-components-rsc) +- [React 19 Actions & Forms](#react-19-actions--forms) +- [Suspense & Streaming SSR](#suspense--streaming-ssr) +- [TanStack Query v5](#tanstack-query-v5) +- [Review Checklists](#review-checklists) + +--- + +## 基础 Hooks 规则 + +```tsx +// ❌ 条件调用 Hooks — 违反 Hooks 规则 +function BadComponent({ isLoggedIn }) { + if (isLoggedIn) { + const [user, setUser] = useState(null); // Error! + } + return
...
; +} + +// ✅ Hooks 必须在组件顶层调用 +function GoodComponent({ isLoggedIn }) { + const [user, setUser] = useState(null); + if (!isLoggedIn) return ; + return
{user?.name}
; +} +``` + +--- + +## useEffect 模式 + +```tsx +// ❌ 依赖数组缺失或不完整 +function BadEffect({ userId }) { + const [user, setUser] = useState(null); + useEffect(() => { + fetchUser(userId).then(setUser); + }, []); // 缺少 userId 依赖! +} + +// ✅ 完整的依赖数组 +function GoodEffect({ userId }) { + const [user, setUser] = useState(null); + useEffect(() => { + let cancelled = false; + fetchUser(userId).then(data => { + if (!cancelled) setUser(data); + }); + return () => { cancelled = true; }; // 清理函数 + }, [userId]); +} + +// ❌ useEffect 用于派生状态(反模式) +function BadDerived({ items }) { + const [filteredItems, setFilteredItems] = useState([]); + useEffect(() => { + setFilteredItems(items.filter(i => i.active)); + }, [items]); // 不必要的 effect + 额外渲染 + return ; +} + +// ✅ 直接在渲染时计算,或用 useMemo +function GoodDerived({ items }) { + const filteredItems = useMemo( + () => items.filter(i => i.active), + [items] + ); + return ; +} + +// ❌ useEffect 用于事件响应 +function BadEventEffect() { + const [query, setQuery] = useState(''); + useEffect(() => { + if (query) { + analytics.track('search', { query }); // 应该在事件处理器中 + } + }, [query]); +} + +// ✅ 在事件处理器中执行副作用 +function GoodEvent() { + const [query, setQuery] = useState(''); + const handleSearch = (q: string) => { + setQuery(q); + analytics.track('search', { query: q }); + }; +} +``` + +--- + +## useMemo / useCallback + +```tsx +// ❌ 过度优化 — 常量不需要 useMemo +function OverOptimized() { + const config = useMemo(() => ({ timeout: 5000 }), []); // 无意义 + const handleClick = useCallback(() => { + console.log('clicked'); + }, []); // 如果不传给 memo 组件,无意义 +} + +// ✅ 只在需要时优化 +function ProperlyOptimized() { + const config = { timeout: 5000 }; // 简单对象直接定义 + const handleClick = () => console.log('clicked'); +} + +// ❌ useCallback 依赖总是变化 +function BadCallback({ data }) { + // data 每次渲染都是新对象,useCallback 无效 + const process = useCallback(() => { + return data.map(transform); + }, [data]); +} + +// ✅ useMemo + useCallback 配合 React.memo 使用 +const MemoizedChild = React.memo(function Child({ onClick, items }) { + return
{items.length}
; +}); + +function Parent({ rawItems }) { + const items = useMemo(() => processItems(rawItems), [rawItems]); + const handleClick = useCallback(() => { + console.log(items.length); + }, [items]); + return ; +} +``` + +--- + +## 组件设计 + +```tsx +// ❌ 在组件内定义组件 — 每次渲染都创建新组件 +function BadParent() { + function ChildComponent() { // 每次渲染都是新函数! + return
child
; + } + return ; +} + +// ✅ 组件定义在外部 +function ChildComponent() { + return
child
; +} +function GoodParent() { + return ; +} + +// ❌ Props 总是新对象引用 +function BadProps() { + return ( + {}} // 每次渲染新函数 + /> + ); +} + +// ✅ 稳定的引用 +const style = { color: 'red' }; +function GoodProps() { + const handleClick = useCallback(() => {}, []); + return ; +} +``` + +--- + +## Error Boundaries & Suspense + +```tsx +// ❌ 没有错误边界 +function BadApp() { + return ( + }> + {/* 错误会导致整个应用崩溃 */} + + ); +} + +// ✅ Error Boundary 包裹 Suspense +function GoodApp() { + return ( + }> + }> + + + + ); +} +``` + +--- + +## Server Components (RSC) + +```tsx +// ❌ 在 Server Component 中使用客户端特性 +// app/page.tsx (Server Component by default) +function BadServerComponent() { + const [count, setCount] = useState(0); // Error! No hooks in RSC + return ; +} + +// ✅ 交互逻辑提取到 Client Component +// app/counter.tsx +'use client'; +function Counter() { + const [count, setCount] = useState(0); + return ; +} + +// app/page.tsx (Server Component) +function GoodServerComponent() { + const data = await fetchData(); // 可以直接 await + return ( +
+

{data.title}

+ {/* 客户端组件 */} +
+ ); +} + +// ❌ 'use client' 放置不当 — 整个树都变成客户端 +// layout.tsx +'use client'; // 这会让所有子组件都成为客户端组件 +export default function Layout({ children }) { ... } + +// ✅ 只在需要交互的组件使用 'use client' +// 将客户端逻辑隔离到叶子组件 +``` + +--- + +## React 19 Actions & Forms + +React 19 引入了 Actions 系统和新的表单处理 Hooks,简化异步操作和乐观更新。 + +### useActionState + +```tsx +// ❌ 传统方式:多个状态变量 +function OldForm() { + const [isPending, setIsPending] = useState(false); + const [error, setError] = useState(null); + const [data, setData] = useState(null); + + const handleSubmit = async (formData: FormData) => { + setIsPending(true); + setError(null); + try { + const result = await submitForm(formData); + setData(result); + } catch (e) { + setError(e.message); + } finally { + setIsPending(false); + } + }; +} + +// ✅ React 19: useActionState 统一管理 +import { useActionState } from 'react'; + +function NewForm() { + const [state, formAction, isPending] = useActionState( + async (prevState, formData: FormData) => { + try { + const result = await submitForm(formData); + return { success: true, data: result }; + } catch (e) { + return { success: false, error: e.message }; + } + }, + { success: false, data: null, error: null } + ); + + return ( + + + + {state.error &&

{state.error}

} + + ); +} +``` + +### useFormStatus + +```tsx +// ❌ Props 透传表单状态 +function BadSubmitButton({ isSubmitting }) { + return ; +} + +// ✅ useFormStatus 访问父
状态(无需 props) +import { useFormStatus } from 'react-dom'; + +function SubmitButton() { + const { pending, data, method, action } = useFormStatus(); + // 注意:必须在 内部的子组件中使用 + return ( + + ); +} + +// ❌ useFormStatus 在 form 同级组件中调用——不工作 +function BadForm() { + const { pending } = useFormStatus(); // 这里无法获取状态! + return ( + + +
+ ); +} + +// ✅ useFormStatus 必须在 form 的子组件中 +function GoodForm() { + return ( +
+ {/* useFormStatus 在这里面调用 */} + + ); +} +``` + +### useOptimistic + +```tsx +// ❌ 等待服务器响应再更新 UI +function SlowLike({ postId, likes }) { + const [likeCount, setLikeCount] = useState(likes); + const [isPending, setIsPending] = useState(false); + + const handleLike = async () => { + setIsPending(true); + const newCount = await likePost(postId); // 等待... + setLikeCount(newCount); + setIsPending(false); + }; +} + +// ✅ useOptimistic 即时反馈,失败自动回滚 +import { useOptimistic } from 'react'; + +function FastLike({ postId, likes }) { + const [optimisticLikes, addOptimisticLike] = useOptimistic( + likes, + (currentLikes, increment: number) => currentLikes + increment + ); + + const handleLike = async () => { + addOptimisticLike(1); // 立即更新 UI + try { + await likePost(postId); // 后台同步 + } catch { + // React 自动回滚到 likes 原值 + } + }; + + return ; +} +``` + +### Server Actions (Next.js 15+) + +```tsx +// ❌ 客户端调用 API +'use client'; +function ClientForm() { + const handleSubmit = async (formData: FormData) => { + const res = await fetch('/api/submit', { + method: 'POST', + body: formData, + }); + // ... + }; +} + +// ✅ Server Action + useActionState +// actions.ts +'use server'; +export async function createPost(prevState: any, formData: FormData) { + const title = formData.get('title'); + await db.posts.create({ title }); + revalidatePath('/posts'); + return { success: true }; +} + +// form.tsx +'use client'; +import { createPost } from './actions'; + +function PostForm() { + const [state, formAction, isPending] = useActionState(createPost, null); + return ( +
+ + + + ); +} +``` + +--- + +## Suspense & Streaming SSR + +Suspense 和 Streaming 是 React 18+ 的核心特性,在 2025 年的 Next.js 15 等框架中广泛使用。 + +### 基础 Suspense + +```tsx +// ❌ 传统加载状态管理 +function OldComponent() { + const [data, setData] = useState(null); + const [isLoading, setIsLoading] = useState(true); + + useEffect(() => { + fetchData().then(setData).finally(() => setIsLoading(false)); + }, []); + + if (isLoading) return ; + return ; +} + +// ✅ Suspense 声明式加载状态 +function NewComponent() { + return ( + }> + {/* 内部使用 use() 或支持 Suspense 的数据获取 */} + + ); +} +``` + +### 多个独立 Suspense 边界 + +```tsx +// ❌ 单一边界——所有内容一起加载 +function BadLayout() { + return ( + }> +
+ {/* 慢 */} + {/* 快 */} + + ); +} + +// ✅ 独立边界——各部分独立流式传输 +function GoodLayout() { + return ( + <> +
{/* 立即显示 */} +
+ }> + {/* 独立加载 */} + + }> + {/* 独立加载 */} + +
+ + ); +} +``` + +### Next.js 15 Streaming + +```tsx +// app/page.tsx - 自动 Streaming +export default async function Page() { + // 这个 await 不会阻塞整个页面 + const data = await fetchSlowData(); + return
{data}
; +} + +// app/loading.tsx - 自动 Suspense 边界 +export default function Loading() { + return ; +} +``` + +### use() Hook (React 19) + +```tsx +// ✅ 在组件中读取 Promise +import { use } from 'react'; + +function Comments({ commentsPromise }) { + const comments = use(commentsPromise); // 自动触发 Suspense + return ( +
    + {comments.map(c =>
  • {c.text}
  • )} +
+ ); +} + +// 父组件创建 Promise,子组件消费 +function Post({ postId }) { + const commentsPromise = fetchComments(postId); // 不 await + return ( +
+ + }> + + +
+ ); +} +``` + +--- + +## TanStack Query v5 + +TanStack Query 是 React 生态中最流行的数据获取库,v5 是当前稳定版本。 + +### 基础配置 + +```tsx +// ❌ 不正确的默认配置 +const queryClient = new QueryClient(); // 默认配置可能不适合 + +// ✅ 生产环境推荐配置 +const queryClient = new QueryClient({ + defaultOptions: { + queries: { + staleTime: 1000 * 60 * 5, // 5 分钟内数据视为新鲜 + gcTime: 1000 * 60 * 30, // 30 分钟后垃圾回收(v5 重命名) + retry: 3, + refetchOnWindowFocus: false, // 根据需求决定 + }, + }, +}); +``` + +### queryOptions (v5 新增) + +```tsx +// ❌ 重复定义 queryKey 和 queryFn +function Component1() { + const { data } = useQuery({ + queryKey: ['users', userId], + queryFn: () => fetchUser(userId), + }); +} + +function prefetchUser(queryClient, userId) { + queryClient.prefetchQuery({ + queryKey: ['users', userId], // 重复! + queryFn: () => fetchUser(userId), // 重复! + }); +} + +// ✅ queryOptions 统一定义,类型安全 +import { queryOptions } from '@tanstack/react-query'; + +const userQueryOptions = (userId: string) => + queryOptions({ + queryKey: ['users', userId], + queryFn: () => fetchUser(userId), + }); + +function Component1({ userId }) { + const { data } = useQuery(userQueryOptions(userId)); +} + +function prefetchUser(queryClient, userId) { + queryClient.prefetchQuery(userQueryOptions(userId)); +} + +// getQueryData 也是类型安全的 +const user = queryClient.getQueryData(userQueryOptions(userId).queryKey); +``` + +### 常见陷阱 + +```tsx +// ❌ staleTime 为 0 导致过度请求 +useQuery({ + queryKey: ['data'], + queryFn: fetchData, + // staleTime 默认为 0,每次组件挂载都会 refetch +}); + +// ✅ 设置合理的 staleTime +useQuery({ + queryKey: ['data'], + queryFn: fetchData, + staleTime: 1000 * 60, // 1 分钟内不会重新请求 +}); + +// ❌ 在 queryFn 中使用不稳定的引用 +function BadQuery({ filters }) { + useQuery({ + queryKey: ['items'], // queryKey 没有包含 filters! + queryFn: () => fetchItems(filters), // filters 变化不会触发重新请求 + }); +} + +// ✅ queryKey 包含所有影响数据的参数 +function GoodQuery({ filters }) { + useQuery({ + queryKey: ['items', filters], // filters 是 queryKey 的一部分 + queryFn: () => fetchItems(filters), + }); +} +``` + +### useSuspenseQuery + +```tsx +// ❌ 使用 useQuery + enabled 实现条件查询 +function BadSuspenseQuery({ userId }) { + const { data } = useSuspenseQuery({ + queryKey: ['user', userId], + queryFn: () => fetchUser(userId), + enabled: !!userId, // useSuspenseQuery 不支持 enabled! + }); +} + +// ✅ 组件组合实现条件渲染 +function GoodSuspenseQuery({ userId }) { + // useSuspenseQuery 保证 data 是 T 不是 T | undefined + const { data } = useSuspenseQuery({ + queryKey: ['user', userId], + queryFn: () => fetchUser(userId), + }); + return ; +} + +function Parent({ userId }) { + if (!userId) return ; + return ( + }> + + + ); +} +``` + +### 乐观更新 (v5 简化) + +```tsx +// ❌ 手动管理缓存的乐观更新(复杂) +const mutation = useMutation({ + mutationFn: updateTodo, + onMutate: async (newTodo) => { + await queryClient.cancelQueries({ queryKey: ['todos'] }); + const previousTodos = queryClient.getQueryData(['todos']); + queryClient.setQueryData(['todos'], (old) => [...old, newTodo]); + return { previousTodos }; + }, + onError: (err, newTodo, context) => { + queryClient.setQueryData(['todos'], context.previousTodos); + }, + onSettled: () => { + queryClient.invalidateQueries({ queryKey: ['todos'] }); + }, +}); + +// ✅ v5 简化:使用 variables 进行乐观 UI +function TodoList() { + const { data: todos } = useQuery(todosQueryOptions); + const { mutate, variables, isPending } = useMutation({ + mutationFn: addTodo, + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: ['todos'] }); + }, + }); + + return ( +
    + {todos?.map(todo => )} + {/* 乐观显示正在添加的 todo */} + {isPending && } +
+ ); +} +``` + +### v5 状态字段变化 + +```tsx +// v4: isLoading 表示首次加载或后续获取 +// v5: isPending 表示没有数据,isLoading = isPending && isFetching + +const { data, isPending, isFetching, isLoading } = useQuery({...}); + +// isPending: 缓存中没有数据(首次加载) +// isFetching: 正在请求中(包括后台刷新) +// isLoading: isPending && isFetching(首次加载中) + +// ❌ v4 代码直接迁移 +if (isLoading) return ; // v5 中行为可能不同 + +// ✅ 明确意图 +if (isPending) return ; // 没有数据时显示加载 +// 或 +if (isLoading) return ; // 首次加载中 +``` + +--- + +## Review Checklists + +### Hooks 规则 + +- [ ] Hooks 在组件/自定义 Hook 顶层调用 +- [ ] 没有条件/循环中调用 Hooks +- [ ] useEffect 依赖数组完整 +- [ ] useEffect 有清理函数(订阅/定时器/请求) +- [ ] 没有用 useEffect 计算派生状态 + +### 性能优化(适度原则) + +- [ ] useMemo/useCallback 只用于真正需要的场景 +- [ ] React.memo 配合稳定的 props 引用 +- [ ] 没有在组件内定义子组件 +- [ ] 没有在 JSX 中创建新对象/函数(除非传给非 memo 组件) +- [ ] 长列表使用虚拟化(react-window/react-virtual) + +### 组件设计 + +- [ ] 组件职责单一,不超过 200 行 +- [ ] 逻辑与展示分离(Custom Hooks) +- [ ] Props 接口清晰,使用 TypeScript +- [ ] 避免 Props Drilling(考虑 Context 或组合) + +### 状态管理 + +- [ ] 状态就近原则(最小必要范围) +- [ ] 复杂状态用 useReducer +- [ ] 全局状态用 Context 或状态库 +- [ ] 避免不必要的状态(派生 > 存储) + +### 错误处理 + +- [ ] 关键区域有 Error Boundary +- [ ] Suspense 配合 Error Boundary 使用 +- [ ] 异步操作有错误处理 + +### Server Components (RSC) + +- [ ] 'use client' 只用于需要交互的组件 +- [ ] Server Component 不使用 Hooks/事件处理 +- [ ] 客户端组件尽量放在叶子节点 +- [ ] 数据获取在 Server Component 中进行 + +### React 19 Forms + +- [ ] 使用 useActionState 替代多个 useState +- [ ] useFormStatus 在 form 子组件中调用 +- [ ] useOptimistic 不用于关键业务(支付等) +- [ ] Server Action 正确标记 'use server' + +### Suspense & Streaming + +- [ ] 按用户体验需求划分 Suspense 边界 +- [ ] 每个 Suspense 有对应的 Error Boundary +- [ ] 提供有意义的 fallback(骨架屏 > Spinner) +- [ ] 避免在 layout 层级 await 慢数据 + +### TanStack Query + +- [ ] queryKey 包含所有影响数据的参数 +- [ ] 设置合理的 staleTime(不是默认 0) +- [ ] useSuspenseQuery 不使用 enabled +- [ ] Mutation 成功后 invalidate 相关查询 +- [ ] 理解 isPending vs isLoading 区别 + +### 测试 + +- [ ] 使用 @testing-library/react +- [ ] 用 screen 查询元素 +- [ ] 用 userEvent 代替 fireEvent +- [ ] 优先使用 *ByRole 查询 +- [ ] 测试行为而非实现细节 diff --git a/references/rust.md b/references/rust.md new file mode 100644 index 0000000..a66f818 --- /dev/null +++ b/references/rust.md @@ -0,0 +1,256 @@ +# Rust Code Review Guide + +> Rust 代码审查指南。编译器能捕获内存安全问题,但审查者需要关注编译器无法检测的问题——业务逻辑、API 设计、性能和可维护性。 + +--- + +## 所有权与借用 + +### 避免不必要的 clone() + +```rust +// ❌ clone() 是"Rust 的胶带"——用于绕过借用检查器 +fn bad_process(data: &Data) -> Result<()> { + let owned = data.clone(); // 为什么需要 clone? + expensive_operation(owned) +} + +// ✅ 审查时问:clone 是否必要?能否用借用? +fn good_process(data: &Data) -> Result<()> { + expensive_operation(data) // 传递引用 +} +``` + +### Arc> 的使用 + +```rust +// ❌ Arc> 可能隐藏不必要的共享状态 +struct BadService { + cache: Arc>>, // 真的需要共享? +} + +// ✅ 考虑是否需要共享,或者设计可以避免 +struct GoodService { + cache: HashMap, // 单一所有者 +} +``` + +--- + +## Unsafe 代码审查(最关键!) + +```rust +// ❌ unsafe 没有安全文档——这是红旗 +unsafe fn bad_transmute(t: T) -> U { + std::mem::transmute(t) +} + +// ✅ 每个 unsafe 必须解释:为什么安全?什么不变量? +/// Transmutes `T` to `U`. +/// +/// # Safety +/// +/// - `T` and `U` must have the same size and alignment +/// - `T` must be a valid bit pattern for `U` +/// - The caller ensures no references to `t` exist after this call +unsafe fn documented_transmute(t: T) -> U { + // SAFETY: Caller guarantees size/alignment match and bit validity + std::mem::transmute(t) +} +``` + +--- + +## 异步代码 + +### 避免阻塞操作 + +```rust +// ❌ 在 async 上下文中阻塞——会饿死其他任务 +async fn bad_async() { + let data = std::fs::read_to_string("file.txt").unwrap(); // 阻塞! + std::thread::sleep(Duration::from_secs(1)); // 阻塞! +} + +// ✅ 使用异步 API +async fn good_async() { + let data = tokio::fs::read_to_string("file.txt").await?; + tokio::time::sleep(Duration::from_secs(1)).await; +} +``` + +### Mutex 和 .await + +```rust +// ❌ 跨 .await 持有 std::sync::Mutex——可能死锁 +async fn bad_lock(mutex: &std::sync::Mutex) { + let guard = mutex.lock().unwrap(); + async_operation().await; // 持锁等待! + process(&guard); +} + +// ✅ 最小化锁范围,或使用 tokio::sync::Mutex +async fn good_lock(mutex: &std::sync::Mutex) { + let data = mutex.lock().unwrap().clone(); // 立即释放 + async_operation().await; + process(&data); +} +``` + +--- + +## 错误处理 + +### 库 vs 应用的错误类型 + +```rust +// ❌ 库代码用 anyhow——调用者无法 match 错误 +pub fn parse_config(s: &str) -> anyhow::Result { ... } + +// ✅ 库用 thiserror,应用用 anyhow +#[derive(Debug, thiserror::Error)] +pub enum ConfigError { + #[error("invalid syntax at line {line}: {message}")] + Syntax { line: usize, message: String }, + #[error("missing required field: {0}")] + MissingField(String), + #[error(transparent)] + Io(#[from] std::io::Error), +} + +pub fn parse_config(s: &str) -> Result { ... } +``` + +### 保留错误上下文 + +```rust +// ❌ 吞掉错误上下文 +fn bad_error() -> Result<()> { + operation().map_err(|_| anyhow!("failed"))?; // 原始错误丢失 + Ok(()) +} + +// ✅ 使用 context 保留错误链 +fn good_error() -> Result<()> { + operation().context("failed to perform operation")?; + Ok(()) +} +``` + +--- + +## 性能 + +### 避免不必要的 collect() + +```rust +// ❌ 不必要的 collect——中间分配 +fn bad_sum(items: &[i32]) -> i32 { + items.iter() + .filter(|x| **x > 0) + .collect::>() // 不必要! + .iter() + .sum() +} + +// ✅ 惰性迭代 +fn good_sum(items: &[i32]) -> i32 { + items.iter().filter(|x| **x > 0).sum() +} +``` + +### 字符串拼接 + +```rust +// ❌ 字符串拼接在循环中重复分配 +fn bad_concat(items: &[&str]) -> String { + let mut s = String::new(); + for item in items { + s = s + item; // 每次都重新分配! + } + s +} + +// ✅ 预分配或用 join +fn good_concat(items: &[&str]) -> String { + items.join("") // 或用 with_capacity +} +``` + +--- + +## Trait 设计 + +```rust +// ❌ 过度抽象——不是 Java,不需要 Interface 一切 +trait Processor { fn process(&self); } +trait Handler { fn handle(&self); } +trait Manager { fn manage(&self); } // Trait 过多 + +// ✅ 只在需要多态时创建 trait +// 具体类型通常更简单、更快 +``` + +--- + +## Rust Review Checklist + +### 编译器不能捕获的问题 + +**业务逻辑正确性** +- [ ] 边界条件处理正确 +- [ ] 状态机转换完整 +- [ ] 并发场景下的竞态条件 + +**API 设计** +- [ ] 公共 API 难以误用 +- [ ] 类型签名清晰表达意图 +- [ ] 错误类型粒度合适 + +### 所有权与借用 + +- [ ] clone() 是有意为之,文档说明了原因 +- [ ] Arc> 真的需要共享状态吗? +- [ ] RefCell 的使用有正当理由 +- [ ] 生命周期不过度复杂 + +### Unsafe 代码(最重要) + +- [ ] 每个 unsafe 块有 SAFETY 注释 +- [ ] unsafe fn 有 # Safety 文档节 +- [ ] 解释了为什么是安全的,不只是做什么 +- [ ] 列出了必须维护的不变量 +- [ ] unsafe 边界尽可能小 +- [ ] 考虑过是否有 safe 替代方案 + +### 异步/并发 + +- [ ] 没有在 async 中阻塞(std::fs、thread::sleep) +- [ ] 没有跨 .await 持有 std::sync 锁 +- [ ] spawn 的任务满足 'static +- [ ] 锁的获取顺序一致 +- [ ] Channel 缓冲区大小合理 + +### 错误处理 + +- [ ] 库:thiserror 定义结构化错误 +- [ ] 应用:anyhow + context +- [ ] 没有生产代码 unwrap/expect +- [ ] 错误消息对调试有帮助 +- [ ] must_use 返回值被处理 + +### 性能 + +- [ ] 避免不必要的 collect() +- [ ] 大数据传引用 +- [ ] 字符串用 with_capacity 或 write! +- [ ] impl Trait vs Box 选择合理 +- [ ] 热路径避免分配 + +### 代码质量 + +- [ ] cargo clippy 零警告 +- [ ] cargo fmt 格式化 +- [ ] 文档注释完整 +- [ ] 测试覆盖边界条件 +- [ ] 公共 API 有文档示例 diff --git a/references/typescript.md b/references/typescript.md new file mode 100644 index 0000000..9d204d4 --- /dev/null +++ b/references/typescript.md @@ -0,0 +1,101 @@ +# TypeScript/JavaScript Code Review Guide + +> TypeScript/JavaScript 通用代码审查指南,覆盖类型安全、异步处理、常见陷阱。 + +--- + +## 类型安全 + +### 避免使用 any + +```typescript +// ❌ Using any defeats type safety +function processData(data: any) { // Avoid any + return data.value; +} + +// ✅ Use proper types +interface DataPayload { + value: string; +} +function processData(data: DataPayload) { + return data.value; +} +``` + +--- + +## 异步处理 + +### 错误处理 + +```typescript +// ❌ Not handling async errors +async function fetchUser(id: string) { + const response = await fetch(`/api/users/${id}`); + return response.json(); // What if network fails? +} + +// ✅ Handle errors properly +async function fetchUser(id: string): Promise { + try { + const response = await fetch(`/api/users/${id}`); + if (!response.ok) { + throw new Error(`HTTP ${response.status}`); + } + return await response.json(); + } catch (error) { + console.error('Failed to fetch user:', error); + throw error; + } +} +``` + +--- + +## 不可变性 + +### 避免直接修改 Props/参数 + +```typescript +// ❌ Mutation of props +function UserProfile({ user }: Props) { + user.lastViewed = new Date(); // Mutating prop! + return
{user.name}
; +} + +// ✅ Don't mutate props +function UserProfile({ user, onView }: Props) { + useEffect(() => { + onView(user.id); // Notify parent to update + }, [user.id]); + return
{user.name}
; +} +``` + +--- + +## TypeScript Review Checklist + +### 类型系统 +- [ ] 没有使用 `any`(使用 `unknown` 代替未知类型) +- [ ] 接口和类型定义完整 +- [ ] 使用泛型提高代码复用性 +- [ ] 联合类型有正确的类型收窄 + +### 异步代码 +- [ ] async 函数有错误处理 +- [ ] Promise rejection 被正确处理 +- [ ] 避免 callback hell,使用 async/await +- [ ] 并发请求使用 `Promise.all` 或 `Promise.allSettled` + +### 不可变性 +- [ ] 不直接修改函数参数 +- [ ] 使用 spread 操作符创建新对象/数组 +- [ ] 考虑使用 `readonly` 修饰符 + +### 代码质量 +- [ ] 启用 strict 模式 +- [ ] ESLint/TSLint 无警告 +- [ ] 函数有返回类型注解 +- [ ] 避免类型断言(`as`),除非确实必要 diff --git a/references/vue.md b/references/vue.md new file mode 100644 index 0000000..3b3f71e --- /dev/null +++ b/references/vue.md @@ -0,0 +1,240 @@ +# Vue 3 Code Review Guide + +> Vue 3 Composition API 代码审查指南,覆盖响应性系统、Props/Emits、Watchers、Composables 等核心主题。 + +--- + +## 响应性系统 + +### 解构 reactive 对象 + +```vue + + + + + +``` + +### computed 副作用 + +```vue + + + + + +``` + +--- + +## Props & Emits + +### 直接修改 props + +```vue + + + + + +``` + +### defineProps 类型声明 + +```vue + + + + + +``` + +--- + +## Watchers + +### watch 清理函数 + +```vue + + + + + +``` + +--- + +## 模板最佳实践 + +### v-for 的 key + +```vue + + + + + +``` + +### v-if 和 v-for 优先级 + +```vue + + + + + + +``` + +--- + +## Composables + +### Props 传递给 composable + +```vue + + + + + +``` + +--- + +## Vue 3 Review Checklist + +### 响应性系统 +- [ ] ref 用于基本类型,reactive 用于对象 +- [ ] 没有解构 reactive 对象(或使用了 toRefs) +- [ ] props 传递给 composable 时保持了响应性 +- [ ] shallowRef/shallowReactive 用于大型对象优化 + +### Props & Emits +- [ ] defineProps 使用 TypeScript 类型声明 +- [ ] 复杂默认值使用 withDefaults + 工厂函数 +- [ ] defineEmits 有完整的类型定义 +- [ ] 没有直接修改 props + +### Watchers +- [ ] watch/watchEffect 有适当的清理函数 +- [ ] 异步 watch 处理了竞态条件 +- [ ] flush: 'post' 用于 DOM 操作的 watcher +- [ ] 避免过度使用 watcher(优先用 computed) + +### 模板 +- [ ] v-for 使用唯一且稳定的 key +- [ ] v-if 和 v-for 没有在同一元素上 +- [ ] 事件处理使用 kebab-case +- [ ] 大型列表使用虚拟滚动 + +### Composables +- [ ] 相关逻辑提取到 composables +- [ ] composables 返回响应式引用(不是 .value) +- [ ] 纯函数不要包装成 composable +- [ ] 副作用在组件卸载时清理 + +### 性能 +- [ ] 大型组件拆分为小组件 +- [ ] 使用 defineAsyncComponent 懒加载 +- [ ] 避免不必要的响应式转换 +- [ ] v-memo 用于昂贵的列表渲染