# Code Review 规范
# 1. 好处是什么
Code Review 的最大作用在于帮助团队找到代码缺陷。
传播知识。在刚开始代码提交的过程中，不断 PR，不断 commit，最终才会被accepted。这个过程也是不断的 review 过程，通过同事之间的指导，能学到更多的代码规范和很多好的实践。
增加代码质量。有经验的 coder 在架构设计、代码细节等方面会对新人有很好的帮助。Code Review也能帮助 coder 审视到自己的代码质量，以及实施最佳实践。Code Review 最核心的目的就是 code smell，目前公司前端团队代码一般很少经过单元测试&性能测试，经验丰富的同事能很快的发现 code smell，从而杜绝潜在 bug。
图源自Code Review Checklist (opens new window)
# 2. Revivew 什么
# ① 代码风格：对团队既定规范的遵循
- 是否符合基本规范
# ② 架构设计：是否有整体设计，设计是否合理
- 如果有设计文档，是否按照设计文档思路来写代码
- 是否发现了更好的解决方案
- 提供了很好的解决思路
# ③ 代码：代码细节上是否尽可能易读和可扩展
- 是否明显重复代码
- 是否合理抽取枚举值，禁止使用“魔法值”
- 是否合理使用已有的组件和方法
- 对已有的、不合理的代码进行重构和优化
- 职责（组件、方法）、概念是否清晰
- 过度设计
# ④ 健壮性：错误处理、业务逻辑的边界和基本的安全
- 边界和异常是否考虑完备
- 是否存在明显bug
- 是否考虑安全性（xss）
# ⑤ 效率：是否贡献了整体，为组件库和工具库添砖加瓦
- 是否抽取沉淀基础组件和通用业务组件到组件库
# 3. Review CheckList
# 3.1 HTML
启用标准模式 - 使用 HTML5 的 doctype 来启用标准模式 | <!DOCTYPE html> | 使用 HTML5 的 doctype 来启用标准模式 |
|---|---|---|
统一使用 UTF-8 编码 | <meta charset="UTF-8" /> | |
| 移动设备添加 viewport | <meta name="viewport" content="initial-scale=1, maximum-scale=1, minimum-scale=1, user-scalable=no" /> | |
| 自闭合标签无需闭合 | <img src="https://xxx.png" alt="Google" /> <br /> <input type="text" name="title" /> | 例如： img， input， br， hr 等 |
| 使用语义化标签 | <header></header> <footer></footer> |
# 3.2 CSS
| BEM 命名原则 | .block__element { } .block--modifier { } | |
|---|---|---|
| 有效使用 CSS 选择器 | 见注1 | 选择器嵌套应少于 3 级 |
| 避免重绘重排 | 见注2、注3 | 当发生重排的时候，浏览器需要重新计算布局位置与大小，不利于性能优化。 |
| 避免重复样式 |
注1: 有效使用 css 选择器，因遵循以下原则
- 保持简单，不要使用嵌套过多过于复杂的选择器。
- 通配符和属性选择器效率最低，需要匹配的元素最多，尽量避免使用。
- 不要使用类选择器和 ID 选择器修饰元素标签。
- 不要为了追求速度而放弃可读性与可维护性
- 避免使用 CSS 表达式
注2: 常见引起重绘重排属性和方法
- 添加或者删除可见的
DOM元素； - 元素尺寸改变——边距、填充、边框、宽度和高度
- 内容变化，比如用户在
input框中输入文字 - 浏览器窗口尺寸改变——
resize事件发生时 - 计算
offsetWidth和offsetHeight属性 - 设置
style属性的值
注3: 减少重绘重排的方法
- 使用
transform替代top - 使用
visibility替换display: none，因为前者只会引起重绘，后者会引发回流（改变了布局） - 不要把节点的属性值放在一个循环里当成循环里的变量。
- 不要使用
table布局，可能很小的一个小改动会造成整个table的重新布局 - 动画实现的速度的选择，动画速度越快，回流次数越多，也可以选择使用
requestAnimationFrame - CSS 选择符从右往左匹配查找，避免节点层级过多
# 3.3 JavaScript
| 命名 | 见注4 | 命名采用小驼峰式命名 |
|---|---|---|
| 字符串模板而不是 '+' 来拼接字符串 | Hello,${world} | |
| 扩展运算符做数组浅拷贝 | const copy = [...arr] | |
| 用 Array.from 去将一个类数组对象转成一个数组 | const arr = Array.from(arrLike) | |
| 使用数组解构 | const [first, second] = arr | |
对象浅拷贝时，更推荐使用扩展运算符 ... | const copy = { ...original, c: 3 } | |
| 函数参数使用默认值替代使用条件语句进行赋值 | function createMicrobrewery(name = 'Jack') { ... } | |
优先使用 rest 语法 ...，而不是 arguments | function concatenateAll(...args) { return args.join('') } | |
| 尽量使用箭头函数 | [1, 2, 3].map((x) => { const y = x + 1 return x * y }) | |
| 第三方库 | 最好别随意修改版本号，确保版本升级不会影响 | |
| 复杂的判断条件 | 一般逻辑判断非常复杂一般前期没有想得很清楚，或者后期的维护不断的迭代，持续往上叠加，在这种情况下，逻辑会变得越来越复杂，在开发或CR时可以考虑重新梳理清楚，重点查看，进行优化。 | |
| 逻辑范围变化 | if (type === 1) {} else {}if (type === 1 && isShow) {} else {} | 此问题一般出现在增量代码上，因为前面的条件判断的范围变小了，导致后面的逻辑处理的范围变大了。此时要注意这种范围的变化，是否真的符合预期，还是只想修改一处逻辑，却不小心影响到了后面的逻辑。 |
| 异常处理 | else为空，确认无需处理if (conditionA) {}报错catch UserService.getList().then()try...catch，未处理catch try {...} catch() {} | 确保所有的边界逻辑都已经处理或者无需处理。 |
| ⼤概率正确 !== 正确 | ||
| object链式取值 | 经常出现很多这样的报错xxx is undefined，大部分的原因主要是因为我们在对象取值时，喜欢直接点点点。建议使用lodash的get或者?.??。 | |
| 隐式类型转换 | if ( amount == '22' ) {} | 隐式类型转换容易出现很多问题，==可以使用eslint避免。 |
| 数据展示 | 对于资产、金额等关键数据的展示，尽可能直接展示后台返回数据，前端不做计算。 | |
| 数据校验 | 对传输/接收的数据都进行校验、认证，确保数据的来源和正确。校验有效位、计算精度、完整性、一致性、时效性（获取时机是否正确、缓存是否更新） | |
| 数据转换 | 数据转换处理一定要经过充分的测试验证，并且尽量选取源数据进行传输，而并非转换后的数据。 | |
| 防抖与节流 | debounce | resize、scroll、mousemove 等事件我们通常不需要事件持续触发，频繁执行 |
注4: 命名需要符合语义化，如果函数命名，可以采用加上动词前缀：
- can 判断是否可执行某个动作
- has 判断是否含有某个值
- is 判断是否为某个值
- get 获取某个值
- set 设置某个值
# 3.4 Vue
| Prop 定义尽量详细，至少需要指定其类型 | props: { status: String， required: true } | props |
|---|---|---|
| v-if 和 v-for 不要用在同一个元素上 | ||
| v-for 遍历必须添加 key | <ul> <li v-for="todo in todos" :key="todo.id"></li> </ul> | |
| 组件模板应该书写简洁 | ||
| 指令缩写 | ||
| 多个属性进行分行 | ||
| 为组件样式设置作用域 | ||
| 清除定时器或者事件监听 | beforeDestory |
# 3.5 性能与安全
| 图片大小 | 图片是否有进行过压缩处理，非页面级的图片一般不要超过200kb | |
|---|---|---|
| http请求 | 页面初始化请求过多？白屏时间过长？初始化加载数据是否在合理范围内？ | |
| 懒加载 | 是否可以通过懒加载或者按需加载进行优化？ | |
| 缓存数据 | 需要重复加载数据时，可以通过缓存数据减少请求 | |
| 影响范围 | 底层架构、组件或者方法的修改，是否确认影响范围，每个受影响的依赖都能正常使用。 | |
| 修改范围 | 是否属于本次迭代正常上线的功能范围，有没有对本次范围进行变更，是否通知到测试同学。 | 我们上线的代码往往有很多属于夹带私货，比如，上个迭代有一个影响不大的小Bug，趁着还没被发现，偷偷将它带上线，或者，发现上一次写的代码太蠢了，还有更好的解决思路，于是洁癖发作，默默地改了。 |