1. 先聊清楚为什么前端团队需要一本Review指南1.1 open-code-review到底是什么我大概两年前开始折腾团队里的代码评审流程一开始大家Review全凭感觉看得快就多批两句赶上发版就草草点个Approve线上出了事才回头骂当初怎么没人看出来。后来我整理了整整一套前端Review的规范团队里管它叫open-code-review版——意思很简单评审的标准、流程、检查项全部开放出来不再收在某个人的脑子里也不依赖某位资深同事的神之直觉。这套东西拉平了团队成员之间的经验差让一个刚入职两周的新人拿到任何一份PR都知道该从哪看起、该重点关注什么、哪些问题必须拦下来。说实话open-code-review不是什么黑科技框架它更像一份活文档加一套配套检查流程。核心思想就是三件事第一把Review的检查项显式化做成清单而不是靠感觉第二把Review从事后挑毛病前置到提交之前就自检第三把机械化、重复性的检查交给工具和脚本让人力集中在真正需要判断力的地方比如设计合理性、业务边界、性能隐患。我见过太多团队把Review做成形式主义。代码写得稀烂CI是绿的Reviewer秒Approve技术债越滚越大最后只能靠重构来还。根本原因就是没有把Review当成一个工程问题来对待而是一直停留在找个人看看的原始阶段。open-code-review这套思路的价值不在于发明了新东西而在于把散落在各处的评审经验收敛成一套团队都能执行的标准动作并且把能自动化的部分全部自动化。1.2 前端代码Review和别的岗位不一样在哪不少后端出身的技术管理者会把Review的规范从Java/C团队直接搬过来用到了前端这边就会各种别扭。原因很简单前端的代码交付物不只是一堆业务逻辑还包含了UI状态管理、浏览器兼容性、构建产物体积、用户交互体验、埋点行为、错误上报这类非常前端特有的东西。后端Review会问这个接口的事务边界对不对而前端Review必须多问一句这几个状态到底该放Redux还是放组件内部State或者问你这样渲染列表数据量大一点会不会白屏。另一个显著差异是前端的可测试性通常比后端差一截这导致Review承担了比测试更多的质量职责。后端的单元测试覆盖率动不动能跑到80%前端的业务组件交互测试覆盖率经常惨不忍睹。在这种情况下代码评审就成了拦截缺陷的重要关卡。如果前端团队的Review只是盯着缩进和命名规范等于把最值钱的审查能力浪费在了机器能干的活上真正的业务风险和性能隐患全放过去了。所以你就能理解为什么需要一本专门针对前端的Review指南。它不是把后端那套套个壳而是真正从浏览器极限、用户体感、组件复用、包体积预算这些前端日常关心的点出发把评审的关注维度彻底重排。2. 制定团队Review秩序从流程约定到角色分工2.1 一套可落地的Review流程长什么样我们团队在推行open-code-review之后把Review流程收敛成四个阶段每个阶段都有明确的出入口标准。不搞复杂的审批流也不搞什么多级评审委员会只追求一件事每个PR在合并之前必须经过一次结构化的检查而不是随缘看一眼。第一步是提交阶段开发者自己跑一遍自检清单。我们把这个清单挂在仓库的PR模板里提交PR的时候要求逐项勾选。别看只是个形式动作它能逼着作者在提交前先过一遍自己的代码很多低级错误在这一步就拦掉了。第二步是自动检查阶段CI里挂了ESLint、Stylelint、TypeScript类型检查、单测和覆盖率报告。这些工具的作用是把格式对不对类型对不对基础规则有没有违反这类确定性问题全部接管。注意这里的执行顺序很关键自动检查必须在人工Review之前不然Reviewer一上来看到的全是缩进、分号、类型错误效率极低而且会消磨耐心。第三步是人工Review阶段要求至少一名团队成员做逐行的逻辑审查另一名成员做全局设计审查。分散Review和集中Review交替进行这样既能保证细节不遗漏又能保证整体设计的一致性。我们会刻意控制一次Review的代码量禁止一个PR塞进超过400行改动超过就拆。这招实测下来极其有效Review密度上去了讨论质量也上去了。第四步是合并后追溯阶段周五花半小时把本周合并的PR拉出来看一眼重点看不合理的紧急跳过、Review意见有没有真的被消化。这一步不是秋后算账而是用来调整Review清单本身的——如果连续两周大家都在同一个地方踩坑说明清单里漏了这条那就补进去。2.2 Review的角色分工和作者自检清单很多团队Review效率低是因为角色定义含混不清。作者以为Reviewer是来挑错的Reviewer以为作者已经测过了最后变成两个人都不太确定自己该对什么负责。我在指南里把双方职责写得很死。作者这边提交PR之前必须自己回答四个问题这个改动为什么存在影响范围有哪些测试覆盖做了哪些有没有把代码格式、类型错误这类低级问题处理干净这四个问题不回答清楚PR模板里直接就无法勾选。别小看这个动作我们统计过强制自检实行两个月后Review中啊这个没考虑到忘了改这里这类对话出现频率降了七成。Reviewer这边我把角色拆成两个一个主审逻辑一个主审设计。主审逻辑的人逐行过重点关注状态流转对不对、边界条件有没有处理、数据流是否干净主审设计的人从整体往回看重点思考组件边界是否合理、有没有过度设计、这个改动三个月后是否还能被人看懂。两个人同时Approve才允许合并。刚开始有人觉得多一个人看是浪费人力实际操作下来俩人分工明确反而比一个人来回纠结快得多而且能从两个截然不同的视角发现深层问题。这里放一份我们实际在用、后来又不断迭代的作者自检清单参考是否已经跑过lint、类型检查和全部单元测试是否补充了与本次改动相关的测试用例而不是只测了主流程是否解剖了这个改动带来的全部外部影响包括路由变化、埋点字段、接口变更等是否检查过变更文件里没有夹杂无关的格式调整或功能是否确认了关键路径上的异常处理逻辑而不是任由错误同步崩塌是否在本地验证过极端场景空数据、超长文案、弱网下的表现3. 前端代码Review的核心关注点3.1 组件设计与状态管理的Review要点前端Review里我最花时间看的通常不是单个函数写得好不好而是组件拆分和状态设计。这里有一套固定的审查逻辑首先问这个组件是否承担了超过它应该承担的责任一个组件既拉数据又管UI状态又处理埋点又拼交互逻辑基本就是坏味道。我们规定业务组件和纯展示组件要严格区分纯展示组件只能用props接数据自己不允许发请求不依赖任何全局store。这样分类之后UI组件可以被放心复用到任何业务场景中而业务组件再重也不担心拖垮展示层。状态管理这部分问题集中在几个高频场景。第一类是状态放错了层级本该在父组件的状态被塞到了子组件里兄弟组件之间互相捅咕事件来搞同步第二类是useEffect依赖数组写错要么漏依赖导致闭包捕获了旧值要么把对象字面量写进依赖导致死循环第三类是全局状态滥用团队新人对store无所畏惧页面里随便一个Input的临时值都放进全局state最后整个项目被重渲染拖得奇慢无比。Review遇到这几类问题我会直接在PR上留评论但留评论的口径有讲究只指出问题和提出备选方案不直接甩一个改法。比如说我不会写把这段代码改成XXX而是写这里把接口请求放在子组件里会导致列表项每次重渲染都触发请求你考虑下把数据获取上移到父组件或者交给Suspense处理这种提问式Review能让作者自己想明白比直接给答案有效十倍。3.2 性能隐患的排查视角前端的性能问题在Review阶段发现成本几乎为零到了线上被用户投诉成本就是一次紧急发版加一次故障复盘。所以Review里我会刻意训练团队一种性能敏感意识。拿到一段代码先跑一遍心智流程这个组件重渲染的频率高不高这个列表数据量会不会突破一百条这个计算有没有被反复触发这个依赖有没有被无谓地拉大常被漏掉的性能雷点我整理在表里常见隐患为什么危险审查时看什么内联对象/函数写在渲染函数内每次渲染生成新引用导致子组件无法命中memo组件树里大量React.memo是否形同虚设用数组的find/filter替代Map查找大列表下O(n)系数会叠加页面卡顿的隐形元凶高频遍历逻辑是否集中在渲染热路径图片全量加载没有懒加载占位首屏体积爆炸弱网直接空白img标签有没有loadinglazy有没有用现代图片格式巨大的依赖被打进主包首屏JS体积膨胀解析执行时间线性上升import路径有没有按需引用子模块有没有用routes级拆包定时器/监听器没清理页面切走之后回调还在跑内存泄漏叠加useEffect返回值是否清理了clearInterval/removeEventListener性能审查方面有一条实测有效的心法不要只盯着代码要把代码放到浏览器开发者工具的Performance面板里去验证。Review阶段如果条件允许我会要求作者在PR描述里附上关键操作场景下的Lighthouse评分或Performance录制结果差值明显的一眼就能看出来。这一步用一次就能上瘾因为几个数字比一百句笼统的你需要优化性能有说服力得多。3.3 可测试性和可维护性的审查原则前端Review里经常被忽略的是这个改动本身好不好测试。我在open-code-review指南里专门写了一章这个如果一个改动很难为它写测试通常不是测试难写的问题而是代码设计出了问题。依赖注入做不好、纯函数抽取得少、组件里塞了太多直接操作window或navigator的语句这些都会让测试变得极其痛苦紧接着团队就会放弃测试然后质量就滑坡。维护性审查看的是一个月后你自己还能看懂吗。我对团队提出的最低要求是任何超过十行的函数必须有清晰的名字承载它的意图任何超过三十行的组件必须有拆分依据任何非显而易见的判断逻辑必须有一行注释交代为什么这么写。这个标准不算高但严格执行之后跨人跨月的代码维护成本肉眼可见地降了下来。4. 让工具替你跑掉机械化的Review4.1 在CI门禁里埋什么检查项最值人工Review是稀缺资源不该浪费在确定性规则上。这一节我详细说下工具链怎么搭建。我们前端CI现在固定跑五类检查按性价比排序TypeScript严格模式检查、ESLint加自定义规则、Stylelint、单元测试加覆盖率阈值、构建产物对比。TypeScript严格模式这一项可以过滤掉大量可能运行时才能暴露的类型问题强烈建议所有团队把tsconfig里的strict打开。ESLint我们不仅用eslint:recommended还加了一批团队自定义的规则例如禁止组件内直接操作全局window、禁止把fetch调用写在componentDidMount之外的混乱位置、禁止在依赖哪都没写清楚时有隐式any出现。覆盖率阈值这条容易被忽视。我们不追求数字漂亮但要求新增代码的覆盖率不低于80%核心工具函数必须100%。这个规则是底线目的是确保作者真的对自己写的代码测过而不是应付差事地补一个smoke测试。实际执行中偶尔会有团队说这个纯展示组件测试起来没意义对这种声音唯一的态度是可以先不放行等你想清楚为什么没意义再说。多数情况下你会同意所谓没意义只是因为懒惰不是真的没意义。4.2 用danger.js和自定义脚本做自动Review除了传统的lint类工具我们还引入了danger.js来做“Review评论机器人”。它可以在PR上自动跑一些规则然后把结论以评论的形式贴在PR下。我配置了十几条规则最有价值的是这些检测PR的改动是否触及了src目录里超过15个文件如果触及就评论提示改动面较大请确认是否需要拆分检测有没有无用的console.log被带进主干检测package.json里的依赖版本是否用了通配符范围比如^1.x.x检测代码里有没有包含本地调试用的mock数据地址对比新增代码里有没有出现过超过三层的嵌套判断出现就标记为可重构这套自动Review的好处是让机器先跑一轮把程序员的洁癖类问题全部过滤掉剩下的时间全部留给真正的逻辑讨论。要注意的一点是自动评论必须有明确且可行动的信息如果你让人去看一条含糊的提示人就会熟练地无视它最终这个机器人就变成纯噪音。4.3 AI辅助Review的实操边界最近团队里不少人在用AI做代码Review坦白讲这事有正反两面。正面是AI非常擅长做模式和规范的检测能快速发现遗漏的边界条件、能补全主干之外的测试场景建议负面是AI对业务上下文的理解依然有限容易给出语法正确但业务上非常离谱的建议不能无脑采纳。我的用法是让AI做第二双眼睛。作者在提交PR前先在本地把diff抛给AI过一遍让它回答几个定向问题这段代码的异常处理有没有盲区状态更新的时序有没有可能产生竞态有没有隐藏的性能风险点然后再把AI的回答当成参考清单逐条人工确认。注意没有AI说没问题就等于没问题这回事它只适合做启发式提示不适合做决断。主审逻辑的Reviewer仍然需要一字一字地读完整个diffAI帮不了也行但它能帮作者减少让Reviewer抓狂的低级问题。5. 常见问题与排查技巧实录5.1 Review流程跑不起来先把流程卡住的地方拉出来推行任何新流程头一个月几乎一定会遇到卡壳。我把团队里真实碰到过的问题和解决办法整理成了速查表分享出来给有类似困扰的团队参考。典型症状常见原因我们验证过的解法Review经常拖两三天没人看缺少明确的响应时限和责任绑定规定PR创建后2小时内必须有第一个reviewer响应超时自动提醒到组长Review意见像吵架讨论的是风格偏好而不是质量问题在指南里明确风格问题交给lintReview只讨论正确性和合理性总有人绕过流程直接合并自动检查太弱合并了也不会有代价开启分支保护必须状态检查全绿、至少两个reviewer approve才能合入Review意见改了但又引入了新bug修改后没重跑全量验证规定作者每次push后必须重新触发CI合并前必须看到最后一轮绿色团队新人不敢提意见怕说错话被嘲笑建立建议不等于批评的共识Review里统一用提问句式并给新人配Review buddy这里最想强调的一点是拆掉流程上的形式主义和质量上的放水之间没有灰色地带。一旦分支保护打开合并的门槛就实实在在地立起来了一旦门槛被打破过一两次队伍就会觉得这个门可以闯再想收紧就难了。5.2 Review文化建设的几个心得流程和工具都到位之后真正决定这套东西能不能跑长远的还是团队文化。这里分享几个实操层面的小技巧。第一个技巧是Review意见要有可执行性标准。我们内部有一条不成文的规矩每条Review意见必须要让作者能立刻知道怎么改至少要指出到函数这一个粒度。如果一条意见连自己都说不清具体指哪一行、影响是什么、怎么验证就不要发出去了先自己想明白再说。第二个技巧是定期把大家Review时提过的高质量问题收集起来做成案例集分享给全组。每两周一次挑两个线上曾经漏过去、后来出问题的案例讲一遍当时的判断为什么不够好、以后遇到类似场景要看什么指标。这个动作长期做下来团队的整体代码嗅觉提升极快Review的质量自然也跟着涨。第三个技巧是设置Review时间预算。很多团队的问题是大家白天被需求会、周会、评审会轮番轰炸到了晚上才有体力看代码那Review质量当然差。我们错峰执行每天上午11点到12点固定为Review时间其他人这个时段不安排会议有问题集中处理。这样做的目的就是让Review这件事在每个人的日程表里拥有正经的位置而不是把它当成下班后的苦役。5.3 敏感业务改动的评审升级策略不是所有PR都一样重要。我们对涉及支付、用户数据、登录鉴权、权限映射、核心主流程的改动制定了额外的红线Review制度。这类改动必须让至少一位熟悉该业务模块的老员工参与设计评审而不是只做走查式Review。具体操作上要求作者在PR描述里单独起一段风险影响说明列举这个改动对线上用户的潜在影响以及触发后的回滚方案。这么做不是因为老员工比新人高明太多而是因为这类模块里的隐含约束通常写在团队的集体记忆里不写在任何文档上。比如说这个支付渠道只能走同一套加密协议或者这个接口虽然有三个参数但中间那个参数只有两个月前某个活动时才用到。新人在Review时容易按常识推断但业务逻辑不允许只按常识推断的地方最需要的就是有记忆的人多看一眼。6. 写在最后的心态调整最后想聊一点关于Review本身的定位。我带过不少团队见过很多同事把Review当成一种被审查的紧张时刻这种心态对质量没有好处。代码评审的真正目的从来不是追责而是在走进主干之前让多一双眼睛帮你看到自己没看到的坑。它是团队共同投资在工程质量上的方式是经验的流动不是一种行政管控。如果你团队正在被Review流于形式或者Review拖垮开发节奏这个问题困扰不用急着一口气上齐全套。你可以先做三件事把PR模板改掉让作者在提交前先自检开起分支保护至少让人不能绕过评审直接合代码再加一条让Reviewer必须回答这个改动在设计上是否站得住脚。这三板斧落地一个月你再看团队的代码质量曲线会发现和之前完全不是一个水位的。我个人最大的体会其实是写一个review指南不费劲费劲的是每次在PR下耐着性子把一个问题讲清楚、把一个隐患指出来、帮一个同事补上他认知里的空档。但这些东西攒起来就是整个团队的工程底气。open-code-review这个名字我很喜欢因为它意味着让每一个代码审查的决定、每一次质量的把关都不再是秘传心法而是一份团队里每个成员都能拿到、都能用起来、也都愿意去完善的标准。
open-code-review前端代码评审指南：流程、工具与实战
1. 先聊清楚为什么前端团队需要一本Review指南1.1 open-code-review到底是什么我大概两年前开始折腾团队里的代码评审流程一开始大家Review全凭感觉看得快就多批两句赶上发版就草草点个Approve线上出了事才回头骂当初怎么没人看出来。后来我整理了整整一套前端Review的规范团队里管它叫open-code-review版——意思很简单评审的标准、流程、检查项全部开放出来不再收在某个人的脑子里也不依赖某位资深同事的神之直觉。这套东西拉平了团队成员之间的经验差让一个刚入职两周的新人拿到任何一份PR都知道该从哪看起、该重点关注什么、哪些问题必须拦下来。说实话open-code-review不是什么黑科技框架它更像一份活文档加一套配套检查流程。核心思想就是三件事第一把Review的检查项显式化做成清单而不是靠感觉第二把Review从事后挑毛病前置到提交之前就自检第三把机械化、重复性的检查交给工具和脚本让人力集中在真正需要判断力的地方比如设计合理性、业务边界、性能隐患。我见过太多团队把Review做成形式主义。代码写得稀烂CI是绿的Reviewer秒Approve技术债越滚越大最后只能靠重构来还。根本原因就是没有把Review当成一个工程问题来对待而是一直停留在找个人看看的原始阶段。open-code-review这套思路的价值不在于发明了新东西而在于把散落在各处的评审经验收敛成一套团队都能执行的标准动作并且把能自动化的部分全部自动化。1.2 前端代码Review和别的岗位不一样在哪不少后端出身的技术管理者会把Review的规范从Java/C团队直接搬过来用到了前端这边就会各种别扭。原因很简单前端的代码交付物不只是一堆业务逻辑还包含了UI状态管理、浏览器兼容性、构建产物体积、用户交互体验、埋点行为、错误上报这类非常前端特有的东西。后端Review会问这个接口的事务边界对不对而前端Review必须多问一句这几个状态到底该放Redux还是放组件内部State或者问你这样渲染列表数据量大一点会不会白屏。另一个显著差异是前端的可测试性通常比后端差一截这导致Review承担了比测试更多的质量职责。后端的单元测试覆盖率动不动能跑到80%前端的业务组件交互测试覆盖率经常惨不忍睹。在这种情况下代码评审就成了拦截缺陷的重要关卡。如果前端团队的Review只是盯着缩进和命名规范等于把最值钱的审查能力浪费在了机器能干的活上真正的业务风险和性能隐患全放过去了。所以你就能理解为什么需要一本专门针对前端的Review指南。它不是把后端那套套个壳而是真正从浏览器极限、用户体感、组件复用、包体积预算这些前端日常关心的点出发把评审的关注维度彻底重排。2. 制定团队Review秩序从流程约定到角色分工2.1 一套可落地的Review流程长什么样我们团队在推行open-code-review之后把Review流程收敛成四个阶段每个阶段都有明确的出入口标准。不搞复杂的审批流也不搞什么多级评审委员会只追求一件事每个PR在合并之前必须经过一次结构化的检查而不是随缘看一眼。第一步是提交阶段开发者自己跑一遍自检清单。我们把这个清单挂在仓库的PR模板里提交PR的时候要求逐项勾选。别看只是个形式动作它能逼着作者在提交前先过一遍自己的代码很多低级错误在这一步就拦掉了。第二步是自动检查阶段CI里挂了ESLint、Stylelint、TypeScript类型检查、单测和覆盖率报告。这些工具的作用是把格式对不对类型对不对基础规则有没有违反这类确定性问题全部接管。注意这里的执行顺序很关键自动检查必须在人工Review之前不然Reviewer一上来看到的全是缩进、分号、类型错误效率极低而且会消磨耐心。第三步是人工Review阶段要求至少一名团队成员做逐行的逻辑审查另一名成员做全局设计审查。分散Review和集中Review交替进行这样既能保证细节不遗漏又能保证整体设计的一致性。我们会刻意控制一次Review的代码量禁止一个PR塞进超过400行改动超过就拆。这招实测下来极其有效Review密度上去了讨论质量也上去了。第四步是合并后追溯阶段周五花半小时把本周合并的PR拉出来看一眼重点看不合理的紧急跳过、Review意见有没有真的被消化。这一步不是秋后算账而是用来调整Review清单本身的——如果连续两周大家都在同一个地方踩坑说明清单里漏了这条那就补进去。2.2 Review的角色分工和作者自检清单很多团队Review效率低是因为角色定义含混不清。作者以为Reviewer是来挑错的Reviewer以为作者已经测过了最后变成两个人都不太确定自己该对什么负责。我在指南里把双方职责写得很死。作者这边提交PR之前必须自己回答四个问题这个改动为什么存在影响范围有哪些测试覆盖做了哪些有没有把代码格式、类型错误这类低级问题处理干净这四个问题不回答清楚PR模板里直接就无法勾选。别小看这个动作我们统计过强制自检实行两个月后Review中啊这个没考虑到忘了改这里这类对话出现频率降了七成。Reviewer这边我把角色拆成两个一个主审逻辑一个主审设计。主审逻辑的人逐行过重点关注状态流转对不对、边界条件有没有处理、数据流是否干净主审设计的人从整体往回看重点思考组件边界是否合理、有没有过度设计、这个改动三个月后是否还能被人看懂。两个人同时Approve才允许合并。刚开始有人觉得多一个人看是浪费人力实际操作下来俩人分工明确反而比一个人来回纠结快得多而且能从两个截然不同的视角发现深层问题。这里放一份我们实际在用、后来又不断迭代的作者自检清单参考是否已经跑过lint、类型检查和全部单元测试是否补充了与本次改动相关的测试用例而不是只测了主流程是否解剖了这个改动带来的全部外部影响包括路由变化、埋点字段、接口变更等是否检查过变更文件里没有夹杂无关的格式调整或功能是否确认了关键路径上的异常处理逻辑而不是任由错误同步崩塌是否在本地验证过极端场景空数据、超长文案、弱网下的表现3. 前端代码Review的核心关注点3.1 组件设计与状态管理的Review要点前端Review里我最花时间看的通常不是单个函数写得好不好而是组件拆分和状态设计。这里有一套固定的审查逻辑首先问这个组件是否承担了超过它应该承担的责任一个组件既拉数据又管UI状态又处理埋点又拼交互逻辑基本就是坏味道。我们规定业务组件和纯展示组件要严格区分纯展示组件只能用props接数据自己不允许发请求不依赖任何全局store。这样分类之后UI组件可以被放心复用到任何业务场景中而业务组件再重也不担心拖垮展示层。状态管理这部分问题集中在几个高频场景。第一类是状态放错了层级本该在父组件的状态被塞到了子组件里兄弟组件之间互相捅咕事件来搞同步第二类是useEffect依赖数组写错要么漏依赖导致闭包捕获了旧值要么把对象字面量写进依赖导致死循环第三类是全局状态滥用团队新人对store无所畏惧页面里随便一个Input的临时值都放进全局state最后整个项目被重渲染拖得奇慢无比。Review遇到这几类问题我会直接在PR上留评论但留评论的口径有讲究只指出问题和提出备选方案不直接甩一个改法。比如说我不会写把这段代码改成XXX而是写这里把接口请求放在子组件里会导致列表项每次重渲染都触发请求你考虑下把数据获取上移到父组件或者交给Suspense处理这种提问式Review能让作者自己想明白比直接给答案有效十倍。3.2 性能隐患的排查视角前端的性能问题在Review阶段发现成本几乎为零到了线上被用户投诉成本就是一次紧急发版加一次故障复盘。所以Review里我会刻意训练团队一种性能敏感意识。拿到一段代码先跑一遍心智流程这个组件重渲染的频率高不高这个列表数据量会不会突破一百条这个计算有没有被反复触发这个依赖有没有被无谓地拉大常被漏掉的性能雷点我整理在表里常见隐患为什么危险审查时看什么内联对象/函数写在渲染函数内每次渲染生成新引用导致子组件无法命中memo组件树里大量React.memo是否形同虚设用数组的find/filter替代Map查找大列表下O(n)系数会叠加页面卡顿的隐形元凶高频遍历逻辑是否集中在渲染热路径图片全量加载没有懒加载占位首屏体积爆炸弱网直接空白img标签有没有loadinglazy有没有用现代图片格式巨大的依赖被打进主包首屏JS体积膨胀解析执行时间线性上升import路径有没有按需引用子模块有没有用routes级拆包定时器/监听器没清理页面切走之后回调还在跑内存泄漏叠加useEffect返回值是否清理了clearInterval/removeEventListener性能审查方面有一条实测有效的心法不要只盯着代码要把代码放到浏览器开发者工具的Performance面板里去验证。Review阶段如果条件允许我会要求作者在PR描述里附上关键操作场景下的Lighthouse评分或Performance录制结果差值明显的一眼就能看出来。这一步用一次就能上瘾因为几个数字比一百句笼统的你需要优化性能有说服力得多。3.3 可测试性和可维护性的审查原则前端Review里经常被忽略的是这个改动本身好不好测试。我在open-code-review指南里专门写了一章这个如果一个改动很难为它写测试通常不是测试难写的问题而是代码设计出了问题。依赖注入做不好、纯函数抽取得少、组件里塞了太多直接操作window或navigator的语句这些都会让测试变得极其痛苦紧接着团队就会放弃测试然后质量就滑坡。维护性审查看的是一个月后你自己还能看懂吗。我对团队提出的最低要求是任何超过十行的函数必须有清晰的名字承载它的意图任何超过三十行的组件必须有拆分依据任何非显而易见的判断逻辑必须有一行注释交代为什么这么写。这个标准不算高但严格执行之后跨人跨月的代码维护成本肉眼可见地降了下来。4. 让工具替你跑掉机械化的Review4.1 在CI门禁里埋什么检查项最值人工Review是稀缺资源不该浪费在确定性规则上。这一节我详细说下工具链怎么搭建。我们前端CI现在固定跑五类检查按性价比排序TypeScript严格模式检查、ESLint加自定义规则、Stylelint、单元测试加覆盖率阈值、构建产物对比。TypeScript严格模式这一项可以过滤掉大量可能运行时才能暴露的类型问题强烈建议所有团队把tsconfig里的strict打开。ESLint我们不仅用eslint:recommended还加了一批团队自定义的规则例如禁止组件内直接操作全局window、禁止把fetch调用写在componentDidMount之外的混乱位置、禁止在依赖哪都没写清楚时有隐式any出现。覆盖率阈值这条容易被忽视。我们不追求数字漂亮但要求新增代码的覆盖率不低于80%核心工具函数必须100%。这个规则是底线目的是确保作者真的对自己写的代码测过而不是应付差事地补一个smoke测试。实际执行中偶尔会有团队说这个纯展示组件测试起来没意义对这种声音唯一的态度是可以先不放行等你想清楚为什么没意义再说。多数情况下你会同意所谓没意义只是因为懒惰不是真的没意义。4.2 用danger.js和自定义脚本做自动Review除了传统的lint类工具我们还引入了danger.js来做“Review评论机器人”。它可以在PR上自动跑一些规则然后把结论以评论的形式贴在PR下。我配置了十几条规则最有价值的是这些检测PR的改动是否触及了src目录里超过15个文件如果触及就评论提示改动面较大请确认是否需要拆分检测有没有无用的console.log被带进主干检测package.json里的依赖版本是否用了通配符范围比如^1.x.x检测代码里有没有包含本地调试用的mock数据地址对比新增代码里有没有出现过超过三层的嵌套判断出现就标记为可重构这套自动Review的好处是让机器先跑一轮把程序员的洁癖类问题全部过滤掉剩下的时间全部留给真正的逻辑讨论。要注意的一点是自动评论必须有明确且可行动的信息如果你让人去看一条含糊的提示人就会熟练地无视它最终这个机器人就变成纯噪音。4.3 AI辅助Review的实操边界最近团队里不少人在用AI做代码Review坦白讲这事有正反两面。正面是AI非常擅长做模式和规范的检测能快速发现遗漏的边界条件、能补全主干之外的测试场景建议负面是AI对业务上下文的理解依然有限容易给出语法正确但业务上非常离谱的建议不能无脑采纳。我的用法是让AI做第二双眼睛。作者在提交PR前先在本地把diff抛给AI过一遍让它回答几个定向问题这段代码的异常处理有没有盲区状态更新的时序有没有可能产生竞态有没有隐藏的性能风险点然后再把AI的回答当成参考清单逐条人工确认。注意没有AI说没问题就等于没问题这回事它只适合做启发式提示不适合做决断。主审逻辑的Reviewer仍然需要一字一字地读完整个diffAI帮不了也行但它能帮作者减少让Reviewer抓狂的低级问题。5. 常见问题与排查技巧实录5.1 Review流程跑不起来先把流程卡住的地方拉出来推行任何新流程头一个月几乎一定会遇到卡壳。我把团队里真实碰到过的问题和解决办法整理成了速查表分享出来给有类似困扰的团队参考。典型症状常见原因我们验证过的解法Review经常拖两三天没人看缺少明确的响应时限和责任绑定规定PR创建后2小时内必须有第一个reviewer响应超时自动提醒到组长Review意见像吵架讨论的是风格偏好而不是质量问题在指南里明确风格问题交给lintReview只讨论正确性和合理性总有人绕过流程直接合并自动检查太弱合并了也不会有代价开启分支保护必须状态检查全绿、至少两个reviewer approve才能合入Review意见改了但又引入了新bug修改后没重跑全量验证规定作者每次push后必须重新触发CI合并前必须看到最后一轮绿色团队新人不敢提意见怕说错话被嘲笑建立建议不等于批评的共识Review里统一用提问句式并给新人配Review buddy这里最想强调的一点是拆掉流程上的形式主义和质量上的放水之间没有灰色地带。一旦分支保护打开合并的门槛就实实在在地立起来了一旦门槛被打破过一两次队伍就会觉得这个门可以闯再想收紧就难了。5.2 Review文化建设的几个心得流程和工具都到位之后真正决定这套东西能不能跑长远的还是团队文化。这里分享几个实操层面的小技巧。第一个技巧是Review意见要有可执行性标准。我们内部有一条不成文的规矩每条Review意见必须要让作者能立刻知道怎么改至少要指出到函数这一个粒度。如果一条意见连自己都说不清具体指哪一行、影响是什么、怎么验证就不要发出去了先自己想明白再说。第二个技巧是定期把大家Review时提过的高质量问题收集起来做成案例集分享给全组。每两周一次挑两个线上曾经漏过去、后来出问题的案例讲一遍当时的判断为什么不够好、以后遇到类似场景要看什么指标。这个动作长期做下来团队的整体代码嗅觉提升极快Review的质量自然也跟着涨。第三个技巧是设置Review时间预算。很多团队的问题是大家白天被需求会、周会、评审会轮番轰炸到了晚上才有体力看代码那Review质量当然差。我们错峰执行每天上午11点到12点固定为Review时间其他人这个时段不安排会议有问题集中处理。这样做的目的就是让Review这件事在每个人的日程表里拥有正经的位置而不是把它当成下班后的苦役。5.3 敏感业务改动的评审升级策略不是所有PR都一样重要。我们对涉及支付、用户数据、登录鉴权、权限映射、核心主流程的改动制定了额外的红线Review制度。这类改动必须让至少一位熟悉该业务模块的老员工参与设计评审而不是只做走查式Review。具体操作上要求作者在PR描述里单独起一段风险影响说明列举这个改动对线上用户的潜在影响以及触发后的回滚方案。这么做不是因为老员工比新人高明太多而是因为这类模块里的隐含约束通常写在团队的集体记忆里不写在任何文档上。比如说这个支付渠道只能走同一套加密协议或者这个接口虽然有三个参数但中间那个参数只有两个月前某个活动时才用到。新人在Review时容易按常识推断但业务逻辑不允许只按常识推断的地方最需要的就是有记忆的人多看一眼。6. 写在最后的心态调整最后想聊一点关于Review本身的定位。我带过不少团队见过很多同事把Review当成一种被审查的紧张时刻这种心态对质量没有好处。代码评审的真正目的从来不是追责而是在走进主干之前让多一双眼睛帮你看到自己没看到的坑。它是团队共同投资在工程质量上的方式是经验的流动不是一种行政管控。如果你团队正在被Review流于形式或者Review拖垮开发节奏这个问题困扰不用急着一口气上齐全套。你可以先做三件事把PR模板改掉让作者在提交前先自检开起分支保护至少让人不能绕过评审直接合代码再加一条让Reviewer必须回答这个改动在设计上是否站得住脚。这三板斧落地一个月你再看团队的代码质量曲线会发现和之前完全不是一个水位的。我个人最大的体会其实是写一个review指南不费劲费劲的是每次在PR下耐着性子把一个问题讲清楚、把一个隐患指出来、帮一个同事补上他认知里的空档。但这些东西攒起来就是整个团队的工程底气。open-code-review这个名字我很喜欢因为它意味着让每一个代码审查的决定、每一次质量的把关都不再是秘传心法而是一份团队里每个成员都能拿到、都能用起来、也都愿意去完善的标准。
