前端 Code Review 实践：从本地检查到团队协作

Code Review 的价值不只是“再找一个人看代码”。一套有效的审查流程，应该先让工具处理确定性问题，再让作者和 Reviewer 把有限的注意力放在逻辑、风险和长期维护成本上。

GitHub 通常把它叫 Pull Request，GitLab 通常叫 Merge Request。下文统一使用 PR，但方法并不依赖具体平台。

为什么 Code Review 经常失效

很多 Review 失败，并不是参与者不认真，而是流程没有为认真审查创造条件。

评论被格式问题淹没：缩进、引号和导入顺序反复占用讨论空间，真正的逻辑问题反而没人深挖。
改动过大：一个 PR 同时包含重构、功能开发和依赖升级，Reviewer 很难建立完整心智模型。
上下文缺失：描述只有“修复问题”，却没有解释目标、影响范围和验证方式，Reviewer 只能从 diff 猜意图。
直接点通过：检查结果正常不等于业务行为正确。没有风险判断的批准，只是在流程上盖章。

Review 的目标也不该是证明作者写错了什么。它更像一次合并前的共同决策：这次改动是否符合预期，已知风险是否被妥善处理，下一位维护者能否看懂。

先划清机器和人的边界

一个简单原则是：结果可重复、规则可明确表达的检查，优先自动化；需要业务语义和上下文判断的检查，交给人。

适合交给机器的检查

格式、基础语法和导入顺序：规则确定，不值得反复讨论。
TypeScript 类型检查：能稳定发现接口和调用不匹配。
单元测试和构建：适合在相同环境中重复执行。
依赖与静态安全扫描：适合先做广度筛查，再由人确认。

需要人来判断的检查

产品行为是否正确：需要理解需求、状态和用户路径。
边界条件与失败策略：工具不知道失败时系统应该怎样退化。
架构、命名和可维护性：需要结合代码库约定和未来变化判断。
权限边界与敏感数据流：自动扫描难以完整理解业务信任边界。

自动化并不能替代人工安全审查。OWASP 的 Secure Code Review Cheat Sheet 也把业务逻辑、数据流和上下文相关漏洞列为人工判断的重要范围。

一条可执行的审查流水线

可以把流程压缩为四个阶段，每一阶段只回答一类问题。

阶段一：本地检查

作者提交前运行格式化、Lint、类型检查和与改动相关的测试。Git hook 可以缩短反馈时间，但它只是便利设施；流程仍要假设本地检查可能被跳过或环境不一致。

阶段二：CI 检查

PR 创建或更新后，由 CI 在统一环境中执行完整检查，例如：

lint -> type-check -> test -> build -> security scan

并非所有项目都需要一次启用全部步骤，应从最稳定、误报最少的检查开始。对于关键分支，可以把可靠的检查设为合并条件。GitHub 的 Status checks 文档说明了检查结果与分支合并规则如何配合。

阶段三：人工 Review

人工阶段包含两个动作：作者先按最终 diff 自查，再由 Reviewer 审查。作者自查经常能发现调试代码、无关文件和描述遗漏；Reviewer 则从“另一位维护者”的视角检查逻辑、风险与设计。

发现问题后，作者更新改动，CI 重新运行，Reviewer 只需重点复核新的变化和受影响路径。平台上的评论、批准与请求修改只是状态表达，真正的完成条件应由团队规则定义。可以参考 GitHub 的 Pull request review 文档。

阶段四：合并

合并前确认检查通过、阻塞评论已解决、目标分支没有未处理的冲突。选择 merge、squash 或 rebase 应服从仓库已有约定；重点是让历史可追踪，而不是把某一种策略当成唯一答案。

作者提交前要提供什么

Reviewer 不应该靠猜测还原需求。一个可审查的 PR 描述至少包含：

目的：解决了什么问题，为什么现在要改。
影响范围：涉及哪些页面、组件、接口或共享能力。
验证证据：运行了哪些测试，手工覆盖了哪些关键路径。
UI 证据：视觉或交互变化附前后截图；多状态界面同时展示加载、空态、错误态等关键状态。
不确定点：主动指出希望 Reviewer 重点判断的设计和风险。

可以从这份短模板开始：

## 变更说明
<!-- 用几句话说明做了什么，以及为什么这样做 -->

## 影响范围
- 页面或组件：
- 接口或共享模块：

## 验证
- [ ] 自动化测试通过
- [ ] 手工验证关键路径
- [ ] UI 变化已附前后截图

## 请重点关注
<!-- 标出风险较高或尚不确定的部分 -->

描述不是提交后的行政工作。写不清楚“为什么”，通常意味着改动范围还没有收敛。

Reviewer 检查清单

清单用来降低遗漏概率，而不是代替思考。不同模块应按风险增删检查项。

正确性与边界

正常路径是否符合需求，旧行为是否意外改变。
空列表、缺失字段、零值、重复操作和极端输入是否有明确结果。
条件判断是否容易读反；复杂分支能否用有语义的变量说明原因。
状态转换是否完整，例如加载、成功、失败和重试是否互相冲突。

// The name explains why the branch exists.
const shouldRefreshProfile = profile.updatedAt < session.startedAt

if (shouldRefreshProfile) {
  await refreshProfile()
}

异步与错误处理

Promise 是否被正确等待或显式交给后台处理。
请求失败、超时、取消和竞态是否有可预期的行为。
错误是否既被记录，又给用户提供可执行的反馈。
重试是否有终止条件，组件卸载后是否仍可能更新状态。

try {
  const cart = await loadCart()
  renderCart(cart)
} catch (error) {
  reportError(error)
  showMessage('购物车加载失败，请稍后重试')
}

安全与隐私

外部输入是否经过校验，富文本或 HTML 输出是否按可信边界处理。
客户端权限判断是否被误当成服务端授权。
日志、错误信息、URL 和持久化数据是否暴露不必要的信息。
新依赖的来源、维护状态和权限范围是否合理。

性能

是否引入重复请求、不必要的重渲染或无法释放的监听器。
长列表、大资源和重计算是否出现在关键交互路径。
优化是否有测量依据，还是只增加了复杂度。
改动是否显著影响产物体积、启动时间或运行时内存。

可维护性

命名能否表达意图，注释是否说明“为什么”而非复述代码。
模块职责是否清楚，抽象是否真的减少重复或隔离变化。
测试是否覆盖容易回归的行为，而不只是覆盖实现细节。
TypeScript 类型是否收窄了状态空间，还是用宽泛类型绕过问题。

对于 Vue 3，还应检查副作用是否在组件卸载或停用时清理、响应式值是否在解构后仍保持预期行为，以及列表 key 是否稳定。它们不是“Vue 风格偏好”，而是会直接影响生命周期和渲染正确性的行为。

可访问性与跨平台

键盘能否完成核心操作，焦点顺序和焦点反馈是否合理。
表单控件是否有可访问名称，错误信息是否能和字段建立关联。
颜色是否是传递状态的唯一方式，文本与背景对比是否足够。
目标浏览器、屏幕尺寸、输入方式和深浅主题是否覆盖。
设备模拟结果是否在必要时经过真实环境验证。

如何写有用的评论

评论最好同时说明问题、影响和建议，并明确它是否阻塞合并。与其用抽象的 P0/P1 表达，不如采用四种直接标签：

blocking：不解决就不应合并，例如正确性、安全或数据一致性问题。

blocking: 请求失败后仍把通知标为已读，刷新页面会造成状态不一致。建议只在服务端确认成功后更新本地状态。
suggestion：当前方案可工作，但有明确的维护性改进。

suggestion: 这三处都在拼装相同的用户资料，是否可以放进一个适配函数，避免后续字段调整时遗漏？
question：Reviewer 缺少上下文，先确认意图，不预设作者写错。

question: 购物车为空时这里仍会发起结算请求，这是接口约定，还是需要在前端提前返回？
nit：不影响功能，也不值得阻塞合并的细节。

nit: data 可以改成 notificationList，读起来会更直接；不影响本次合并。

尽量评论代码，不评价写代码的人。存在多种合理方案时，把“事实上的缺陷”和“个人偏好”分开。线上来回几轮仍说不清的问题，直接进行一次短沟通，再把结论补回评论区，通常更高效。

关于改动大小和函数长度

“单次改动 400 行以内”或“函数不超过 80 行”可以作为团队观察信号，但不适合当作普遍真理。

行数没有表达风险：自动生成文件可能很长但容易确认，一个只有十几行的权限修改却可能影响巨大。更有用的问题是：

这次改动是否只有一个可描述的目标？
Reviewer 能否在一次专注时段内建立上下文？
重构与行为变化能否拆开，让 diff 更容易验证？
新增代码是否被大量机械变更或文件移动遮住？
函数是否承担多个变化原因，测试是否难以隔离？

团队可以根据历史 Review 时长和缺陷数据设置提醒阈值，并允许作者解释例外。阈值的作用是触发讨论，不是用数字替代判断。

PR 只有一个清晰目标，描述了影响范围和风险。
已检查最终 diff，没有调试残留和无关文件。
本地与 CI 检查通过，测试证据与改动风险匹配。
UI 变化附了关键状态的前后截图。
主动标出了最希望 Reviewer 关注的部分。

Reviewer

理解了改动目的，没有只看局部语法。
检查了正确性、边界、失败路径和状态变化。
检查了安全、隐私、性能、可访问性与兼容性风险。
评论说明了原因，并明确是否阻塞合并。
所有阻塞问题已解决，新的提交也经过必要复核。

好的 Code Review 不靠一份无限增长的规则维持。它依靠清晰上下文、稳定自动化和坦诚协作，让每次合并都成为一次可解释的工程判断。