关于代码评审:前端代码评审通常注意些什么

代码评审又称为（Code Review，简称 CR），对于 CR，开发同学其实都不生疏，当初大部分公司的我的项目开发流程中，它都是必不可少的一个环节，CR 的益处也都耳熟能详。不同的公司对于 CR 的形式、品质要求规范都不一样。这篇文章次要讲的是在代码评审的过程中，会对哪些代码和方向进行评审，给大家在接下来的 CR 中作为参考。

很多我的项目个别把 CR 的工夫节点放在上线前，如果我的项目越大，CR 的节点越靠前越好，越靠后邻近上线前，开发同学越不违心去批改，因为测试过的代码只有呈现改变，就有产生问题的危险，测试也须要从新回归，有可能会产生新的缺点，或者将问题带到生产。上面是咱们我的项目流程中 CR 的节点，个别会在联调前、提测后、上线前各进行一次代码评审。

1、设计方案
设计方案个别是开发后期定好的计划，在领有一份完满的设计方案的现实状态下，在 CR 阶段只需在浏览代码时判断，是否遵循设计方案进行开发即可。
一份绝对残缺设计方案通常会包含：

然而现实情况是，当初很多公司没有设计评审环节，我的项目开发前并没有进行方案设计。可能有以下几个起因：第一：我的项目太小，认为没有必要。第二：没有意识到设计方案的益处，感觉浪费时间，在方案设计的过程中，须要调研、设计、画图、出计划，少则一两天，多则三四天。在缓和的开发周期内，我的项目都心愿越早开发实现越早上线。第三：即便开发同学做了设计方案，也不会进行设计方案的评审。因为每次我的项目都须要将相干人员和评审负责人拉到一起，进行半个小时或者一个小时的计划评估。所以因为各种起因，最终设计方案都成为了需要文档的变种。

个别正文是什么时候写？常常听到这样的答复：正文应该写于代码之前，提前捋分明逻辑和思路，将正文写好，这样也无需重复批改，代码能更加整洁清晰。其实设计方案也像是一个超级大的正文，提前整顿好开发的思路，这样也可能事倍功半。然而提前做好技术设计方案不仅仅是为了在开发的过程中更加顺利，同时也是代码评审过程中的一大助攻。

很多时候咱们拿到须要评审的代码都是通过文件的程序从上而下进行逐行浏览，这种形式往往每个文件的上下文都无奈连接，了解了第一个文件然而切换到第二个文件仍然不知所云，往往须要借助正文、口述或自行查阅上下文查看代码等形式进行了解。然而通过一份残缺的设计方案咱们便能够理解大抵的需要、开发者的设计思路，并且跟随者设计方案的构造和思路对代码的主线进行浏览。

在写了设计方案并设计方案已通过评审的我的项目中，在代码设计方面更多的偏重次要在于代码是否遵循设计方案进行开发，设计方案的合理性、可行性、可扩展性等在设计方案评审阶段曾经通过了探讨有了论断。而对于没有设计方案或没有进行设计方案评审的我的项目，在了解代码的同时，也须要对整体计划的设计进行评估，而，即便方案设计并不合理，一旦通过测试面临行将上线，也没有工夫和条件去进行调整。

在设计上，个别会关注上面几个方面：

复用：性能、组件是否能够提取公共办法？是否反复造轮子？同一代码不应该反复两次以上。
可扩大：当需要产生改变，是否可能应用大量的改变达到咱们的指标，适应将来的变动？
适度设计：很多时候因为晚期为了可能领有更好的扩展性，进行了很多形象和封装，使其复杂化，造成了适度设计。
依赖倒置：模块之间的依赖是否正当？一旦模块产生批改，影响面是否能失去无效的管制？

2、编码
每个团队都有本人的一套编码标准，在评审的过程中也须要留神是否合乎团队的编码标准。然而大部分的编码标准都能够用工具进行束缚，可能应用工具束缚的内容，尽可能不用在评审时花工夫去关注。能够将关注点放在工具束缚之外的标准上，比方：

命名：首先格局（中划线、小驼峰、大驼峰、下划线）须要符合规范，不论是文件命名或者变量命名，尽可能合乎其性能个性，可能通过命名晓得它的含意，毋庸减少正文去特意阐明。
正文：是否在要害代码内减少正文阐明？是否合乎正确的标准？
复杂度：复杂度是否在正当范畴阈值，举荐文章前端代码品质 - 圈复杂度原理和实际
不合理的代码：每个我的项目都有一些难以保护的旧代码，在这个根底上持续增加代码，兴许能够很快的解决当下问题，但对于日后来说，只会让它更加难以保护。及时对不合理的旧代码进行重构和优化就显得尤为重要。
可保护：可维护性这个词其实意味着很多，比方，复杂度善可、可读性较高、可扩展性也还不错、结构合理命名标准，后面做的很多优化设计其实都在为之后的可保护做铺垫。

3、健壮性
代码是否具备安全性和健壮性，对任何一个团队来说，无疑都是十分重要的。

XSS：XSS 攻打具体内容，举荐文章前端平安系列（一）：如何避免 XSS 攻打？
CSRF：CSRF 攻打具体内容，举荐文章前端平安系列（二）：如何避免 CSRF 攻打？
逻辑边界解决：是否有思考到代码的边界逻辑？交互逻辑是否全面？
异样错误处理：一旦抛出异样或者谬误，页面或者运行的代码是否会解体？
资源开释：定时器是否及时清空？内存及时清理？
兼容性：是否有浏览器版本兼容？手机机型兼容？历史数据兼容？接口兼容？小程序版本兼容？
数据展现：对于资产、购物车金额等要害数据的展现，尽可能间接展现后盾返回数据，前端不做计算。
数据校验：对传输 / 接管的数据都进行校验、认证，确保数据的起源和正确。校验无效位、计算精度、完整性、一致性、时效性（获取机会是否正确、缓存是否更新）
数据转换：数据转换解决肯定要通过充沛的测试验证，并且尽量选取源数据进行传输，而并非转换后的数据。

4、性能范畴
很多人认为性能个性的范畴是测试应该去保障的，代码评审时不须要去关注开发了什么性能。但其实现状是，咱们可能发现，咱们上线的代码往往有很多属于 夹带私货，比方，上个迭代有一个影响不大的小 Bug，趁着还没被发现，偷偷将它带上线，或者，发现上一次写的代码太蠢了，还有更好的解决思路，于是洁癖发生，默默地改了，还感觉本人棒呆了。然而测试只晓得本次迭代的性能个性，除了回归主性能之外，并不知道还有其余须要从新测试的中央。如果开发同学刚好对本人的代码十分自信，感觉肯定没问题，没有告诉到测试回归。依据墨菲定律，这种往往感觉没有问题的代码，最初 … 都可能引发线上故障。

影响范畴：底层架构、组件或者办法的批改，是否确认影响范畴，每个受影响的依赖都能失常应用。
批改范畴：是否属于本次迭代失常上线的性能范畴，有没有对本次范畴进行变更，是否告诉到测试同学。

5、监控 / 埋点笼罩
增加监控的前提是公司有一套监控零碎，除了定义好的异样监控场景以外。通常新增的一些要害性能、页面等也须要退出监控，提前加好监控代码，无需等到要查问题时，才记起来遗记加监控了。

监控：监控个别用于监控数据的异样的状况，页面的渲染异样、数据的一致性、正确性。比方：在一些要害数据的逻辑上，如果接口返回的数据与原有约定不统一，增加了监控之后，咱们就能疾速的响应、解决问题。不至于等到引发更多的谬误之后，能力看到问题。
埋点：埋点个别用于统计用户操作行为的数据，大部分场景下须要埋点的数据产品经理都会提供。

6、合规
合规这个词在金融行业十分广泛，但也因为随着人们越来越重视隐衷和平安，法律法规日渐欠缺，对合规这个词也不再生疏。如果利用不合规，就将面临被下线的危险，而有一部分不合规的内容，可能无奈通过测试测出。所以在 CR 的过程中对合规性问题的审查，就尤为重要。任何使得用户隐衷泄露的操作都须要禁止。

敏感信息展现：用户要害敏感信息是否间接展现。
敏感信息明文上报：用户要害敏感信息是否间接明文传给后盾，没有做加密解决等操作。
敏感信息存储 ：保障用户信息的平安，对用户的敏感信息不存储在不平安的中央，比方web storage 等。

7、性能
C 端利用对前端的性能要求会比拟高，代码评审时也能够关注一下比拟常见的问题。前端性能优化 24 条倡议（2020）

图片大小：图片是否有进行过压缩解决，非页面级的图片个别不要超过 200kb。
http 申请：页面初始化申请过多？白屏工夫过长？初始化加载数据是否在正当范畴内？
懒加载：是否能够通过懒加载或者按需加载进行优化？
缓存数据：须要反复加载数据时，能够通过缓存数据缩小申请。

8、tips

checklist：在 review 的过程中，能够发现很多 TODO List，比方减少了配置，在上线前须要先公布配置平台，比方减少片，要记得公布 CDN，比方测试环境增加了测试代码，须要生产从新测试等等，所以在每次 CR 时，能够生成一份上线前的 checklist，每次上线前查看并执行，这样可能确保不会脱漏。
少吃多餐：通过很屡次的 CR 可能感觉到，每次评审的代码越多，品质就会升高，评审工夫过长，都会产生疲劳感，并且一些小细节都会更容易疏忽。所以每次评审的工夫最好管制在 30min~60min 左右为佳，可发动屡次评审，少吃多餐~
好的代码：每次 CR 都在像是找茬，找到不合理的中央。但其实，找到优良正当的代码也能够促成大家的提高。大多数我的项目 CR 并不是全员加入，所以将好的代码整理出来，生成一份最佳实际，能够供大家学习参考，扩大一些新思路。

前段时间听了我司一位资深 CR 大佬的讲座，列了很多平时在 CR 过程中产生的一些常见问题，我在此基础上进行了一些新增和扩大，供大家参考。

第三方库
第三方库的新增、删除、版本号的批改（包含新增小箭头），都要确认好批改范畴，确保理解库降级所带来的影响。不得随便批改版本号，第三方库哪怕是小版本的降级也不能保障对以后我的项目或者以后依赖包没有影响，为了防止造成线上问题，最好锁死版本号。

// package.json

// before
{
     "dependencies": {
        "eslint": "7.27.0",
        "eslint-plugin-vue": "7.15.1",
    },
}
// after
{
     "dependencies": {
        "eslint-plugin-vue": "^7.15.1",
        "typescript": "^4.3.2"
    },
}

命名
命名不对立，导致浏览的老本过高，同示意 数量，然而在 a 文件命名quantity，b 文件命名amount，c 文件命名count。

// a.js
const quantity = 1;

// b.js
const amount = 1;

// c.js
const count = 1;

循环援用
JavaScript 模块的循环加载
文件的互相援用，可能会导致援用报错 undefined。尽量避免文件之间的相互依赖，能够应用eslint 或者 webpack 进行束缚。

// a.js
import b from b.js'

// b.js
import a from 'a.js'

简单的判断条件
个别逻辑判断非常复杂个别后期没有想得很分明，或者前期的保护一直的迭代，继续往上叠加，在这种状况下，逻辑会变得越来越简单，在开发或 CR 时能够思考从新梳理分明，重点查看，进行优化。

if (!somethingA || somethingB && (!somethingC || somethingD)) {...}
or
if (...) {if (...) {...} else if (...) {...} else {...}
} else {...}

逻辑范畴变动
此问题个别呈现在增量代码上，因为后面的条件判断的范畴变小了，导致前面的逻辑解决的范畴变大了。此时要留神这种范畴的变动，是否真的合乎预期，还是只想批改一处逻辑，却不小心影响到了前面的逻辑。

// before
if (type === 1) {} else {}
// after
if (type === 1 && isShow) {} else {}

异样解决
确保所有的边界逻辑都曾经解决或者无需解决。

// else 为空，确认无需解决
if (conditionA) {}

// 报错 catch
UserService.getList().then()

// try...catch，未解决 catch
try {...} catch() {}

⼤概率正确 !== 正确
后面说过的墨菲定律：凡事只有有可能出错，那就肯定会出错。上面 setTimeout 的取值形式置信很多人都见过，应用 500 毫秒的提早，使偶现的问题，“不再呈现”。但其实只是将呈现的概率变小了，该呈现的问题还是会呈现。治标不治本。肯定要找到呈现问题的起因，真正解决它。

setTimeout(() => {... 延时拉取接口 or do something}, 500)
=> 正确 !== 小概率出错

object 链式取值
监控平台总是会呈现很多这样的报错 xxx is undefined，大部分的起因次要是因为咱们在对象取值时，喜爱间接点点点。倡议应用lodash 的get或者?.??。

const a = productObj.something.a;
=> productObj.something 是否肯定有值？// lodash
const a = get(productObj, 'something.a')
// 或者双问号操作符
?.??

隐式类型转换
隐式类型转换容易呈现很多问题，==能够应用 eslint 防止。

if (amount  == '22') {}

+new Date()

css 反复款式
很多时候 CR 时都会疏忽 css，感觉它翻不出什么浪花，然而在像小程序这种对代码包的体积有严格要求的我的项目中，CSS 的精简就显得十分重要了。很多时候在写款式时，都会依据视觉稿一股脑全副写完，但其实很多能够继承的属性，是不须要反复书写的。

.page {
    font-size: 18px;
    font-family: sans-serif;
    line-height: 18px;
    color: #000;
    ...

    .box {
        font-size: 18px;
        font-family: sans-serif;
        line-height: 18px;
        color: #000;
        ...
    }
}

关于代码评审:前端代码评审通常注意些什么

评审节点

评审内容

常见问题