要Review的指标

前端代码Review主要关注以下几个方面：

1、代码质量：代码是否简洁、易读、易维护。是否遵循了一致的编码风格和规范，例如ESLint、Prettier等。
2、功能实现：代码是否实现了预期的功能，是否有潜在的bug或逻辑错误。
3、性能优化：代码是否进行了必要的性能优化，例如避免不必要的渲染、使用了合适的数据结构和算法等。
4、安全性：代码是否存在可能的安全风险，例如XSS攻击、CSRF攻击等。

5、可测试性：代码是否易于测试，是否有单元测试和集成测试。
6、可读性：代码是否易于理解，是否有足够的注释和文档。
7、可复用性：代码是否有高度的模块化和复用性。
8、兼容性：代码是否兼容不同的浏览器和设备。

助记：功性读复全兼测

功能Review

• 无需人为检测，但注释要写明白，TS类型要写清楚，让用的人如沐春风；
• 该匹配单元测试的建议配套单元测试，不落人以口实~

性能Review

前端代码中一些典型的、肉眼可见的需要性能优化的地方包括：

1、过度渲染：如果你的应用在没有必要的情况下进行了过多的渲染，这可能会导致性能问题。例如，在React中，如果你的组件在props没有改变的情况下进行了重新渲染，那么你可能需要使用shouldComponentUpdate/SCU或React.PureComponent或useEffect来避免不必要的渲染。
2、大量DOM操作：频繁的DOM操作是非常耗费性能的。如果你的代码中有大量的DOM操作，你可能需要考虑使用虚拟DOM或其他优化技术（例如DocumentFragment）。
3、大量的网络请求：如果你的应用发送了大量的网络请求，这可能会导致性能问题。你可能需要考虑使用缓存、预加载、懒加载等技术来减少网络请求。
4、大型JavaScript文件：如果你的JavaScript文件过大，这可能会导致加载和解析时间过长。你可能需要考虑使用代码分割、懒加载等技术来减小文件大小。
5、未优化的图片和媒体文件：如果你的网站使用了大量的未优化的图片和媒体文件，这可能会导致加载时间过长。你可能需要考虑使用图片压缩、适当的文件格式、CDN等技术来优化你的媒体文件。
6、阻塞渲染的CSS和JavaScript：如果你的CSS和JavaScript阻塞了页面的渲染，这可能会导致用户体验不佳。你可能需要考虑使用非阻塞的加载技术，如异步加载、延迟加载等。

非阻塞加载CSS和JS

阻塞渲染的CSS和JavaScript是一个常见的前端性能优化问题。当浏览器加载网页时，它会按照HTML文档的顺序解析每个元素。当遇到<link>或<script>标签时，浏览器会停止HTML解析，去下载和执行CSS或JavaScript文件，这就是所谓的“阻塞渲染”。

以下是一些优化阻塞渲染的CSS和JavaScript的策略：

1、异步加载JavaScript：使用async属性可以使浏览器异步加载JavaScript，即在下载JavaScript文件的同时，浏览器可以继续解析HTML。但是，这可能会导致JavaScript在DOM还未完全解析时就开始执行，因此只适用于那些不依赖DOM的JavaScript文件。

2、延迟加载JavaScript：使用defer属性可以使浏览器延迟执行JavaScript，即在DOM解析完毕后，再执行JavaScript。这适用于那些依赖DOM的JavaScript文件。

3、内联关键CSS：将关键的CSS（即渲染首屏内容所需的CSS）内联到HTML中，可以避免浏览器等待CSS文件的下载和解析。但是，这可能会增加HTML文件的大小，因此只适用于小量的CSS。

4、媒体查询：使用媒体查询可以让浏览器只下载适用于当前设备的CSS，从而减少不必要的下载；本质是一种CSS的按需加载技术

以上就是优化阻塞渲染的CSS和JavaScript的一些策略。

可读性Review

关注一下以下要点：

• 变量、函数、类、文件和文件夹的语义化命名
• 注释
• 文档
• TS类型声明

复用性Review

正在review的这段代码可复用性如何，高的话是否可以：

• 封装为函数
• 封装为类
• 封装为指令
• 封装为hook
• 封装为组件
• 封装为模块

安全性Review

在前端代码的安全性Review中，主要关注以下几个方面：

1、跨站脚本攻击（XSS）：检查代码中是否正确地对用户输入进行了转义，以防止插入恶意的JavaScript代码。例如，当使用React时，它默认会转义所有的用户输入，但如果你使用dangerouslySetInnerHTML（或Vue中的v-html），则需要特别小心。
2、跨站请求伪造（CSRF）：检查是否在所有的POST请求中使用了CSRF令牌，以防止攻击者伪造用户的请求。
3、点击劫持：检查是否使用了适当的HTTP头，如X-Frame-Options，来防止点击劫持攻击。
4、内容篡改：检查是否所有的HTTP请求都使用了HTTPS，以防止混合内容问题，这可能会导致用户的数据被窃取（传输过程中被抓包）。
5、输入验证：检查是否在客户端和服务器端都进行了输入验证，以防止注入攻击，图形或字符验证码、短信验证、CSRF-token等的本质都是加入随机真人校验机制，狠重要！
6、敏感信息泄露：检查代码中是否有可能泄露敏感信息的地方，如在URL、错误消息或日志中包含敏感信息。
7、依赖的安全性：检查所有的第三方依赖是否都是最新的，是否有已知的安全漏洞。

点击劫持

X-Frame-Options 是一个HTTP响应头，用于控制网页是否可以被其他网页通过、或等元素嵌入。这个响应头的主要目的是为了防止点击劫持/Clickjacking攻击。

X-Frame-Options有三个可能的值：

1. DENY：此页面不能被嵌入到任何其他页面中。

2. SAMEORIGIN：只有同源的页面（即，URL的协议、域名和端口都相同）才能嵌入此页面。

3. ALLOW-FROM uri：只有指定的页面可以嵌入此页面。但是，这个值已经被废弃，不再被大多数现代浏览器支持。

如果没有设置X-Frame-Options头，或者设置的值不是上述三个值之一，那么任何页面都可以嵌入此页面。因此，为了防止点击劫持攻击，建议总是设置X-Frame-Options头。

兼容性Review

在前端代码的兼容性Review中，主要关注以下几个方面：

1、浏览器兼容性：检查代码是否能在所有支持的浏览器中正常工作。这包括检查是否使用了某些浏览器可能不支持的JavaScript特性或CSS属性，以及是否正确地使用了polyfill和前缀。
2、设备兼容性：检查代码是否能在所有支持的设备中正常工作，包括不同的操作系统、屏幕大小和分辨率。
3、响应式设计：检查代码是否适应了不同的屏幕大小，包括手机、平板和桌面。
4、无障碍性：检查代码是否遵循了无障碍性标准，如WAI-ARIA，以确保所有用户，包括那些使用辅助技术的用户，都能使用你的应用。
5、国际化和本地化：检查代码是否支持多种语言和地区，包括正确地使用了日期、时间和数字的格式。
6、性能：检查代码在低性能的设备或网络环境下是否还能正常工作。

即充分考虑不同浏览器、不同设备、不同屏幕、不同国家、不同人群。

什么是polyfill

• Polyfill是一段代码（通常是JavaScript），用于为旧的或不支持某些特性的浏览器提供这些特性的实现。Polyfill的目的是让开发者能够使用新的、更高级的API，而不用担心兼容性问题。
• 例如，Array.prototype.includes是一个新的JavaScript特性，用于检查数组中是否包含某个元素。但是，这个特性在IE浏览器中是不支持的。为了在IE中也能使用这个特性，我们可以使用一个polyfill，如下：

• 这段代码首先检查Array.prototype.includes是否存在。如果不存在（即，浏览器不支持这个特性），那么就给Array.prototype添加一个includes方法，这个方法的行为和原生的includes方法一样。
• 在实际开发中，我们通常不会自己写polyfill，而是使用已经存在的polyfill库，如core-js或polyfill.io。
• 需要注意的是，虽然polyfill可以让我们在旧浏览器中使用新特性，但是它也有一些缺点。首先，polyfill会增加代码的大小，可能会影响网页的加载速度。其次，polyfill可能无法完全模拟新特性的行为，特别是一些复杂的或与性能相关的特性。因此，在使用polyfill时，需要权衡其利弊。

配置polyfill

• 安装polyfill库通常是第一步，但是你可能还需要进行一些配置，以确保polyfill能够在你的代码中正确地工作。
• 具体的配置步骤取决于你使用的polyfill库和构建工具。
• 以core-js和babel为例，你可以按照以下步骤来配置polyfill：

1. 首先，安装core-js和@babel/preset-env：

2. 然后，在你的.babelrc文件（或者babel的配置文件）中，添加@babel/preset-env预设，并设置useBuiltIns和corejs选项：

• 这里的useBuiltIns选项有两个可能的值："usage"和"entry"。如果设置为"usage"，那么babel会自动检测你的代码中使用了哪些新特性，然后只包含这些特性的polyfill。如果设置为"entry"，那么你需要在你的代码的入口文件中，手动导入所有需要的polyfill：

以上就是配置polyfill的一个示例。需要注意的是，这只是一个基本的配置，实际的配置可能会更复杂，取决于你的具体需求和环境。

可测试性Review

在前端代码的可测试性Review中，主要关注以下几个方面：

1、单元测试：检查代码是否易于进行单元测试。这包括检查函数是否是纯函数（即，相同的输入总是产生相同的输出，没有副作用），以及是否避免了全局状态。

2、集成测试：检查代码是否易于进行集成测试。这包括检查组件是否正确地使用了props和state，以及是否避免了直接操作DOM。

3、端到端测试：检查代码是否易于进行端到端测试。这包括检查页面是否有易于定位的元素（如有特定id或data-test属性的元素），以及是否有清晰的用户流程。

4、测试覆盖率：检查代码的测试覆盖率是否足够。这包括检查是否所有的函数和分支都有对应的测试。

5、模拟和打桩：检查代码是否易于进行模拟和打桩。这包括检查函数是否避免了直接调用复杂的依赖（如网络请求或数据库操作），以及是否提供了足够的接口来替换这些依赖。

6、错误处理：检查代码是否正确地处理了可能的错误。这包括检查是否有错误处理函数或catch块，以及是否有对应的错误测试。

端到端测试

• 端到端（End-to-End，E2E）测试 是一种测试方法，用于测试应用的整个工作流程，从用户界面到数据库，确保所有部分都能正确地协同工作。
• 在React应用中，我们可以使用Cypress或Puppeteer等工具来进行E2E测试。以下是一个使用Cypress进行E2E测试的基本步骤：

1、安装Cypress：首先，你需要在你的项目中安装Cypress。你可以使用npm或yarn来安装：

2、编写测试：然后，你可以在cypress/integration目录下编写你的E2E测试。以下是一个简单的测试示例，它测试了用户是否能正确地登录：

3、运行测试：最后，你可以使用Cypress的CLI来运行你的测试：

这将会打开Cypress的测试运行器，你可以在这里选择你要运行的测试。

以上就是在React应用中进行E2E测试的基本步骤。需要注意的是，这只是一个基本的示例，实际的E2E测试可能会更复杂，包括测试更多的用户交互和应用状态。

功性读复全兼测! OVER，收工干饭！

原文链接：https://juejin.cn/post/7283748394601840680

者：magentaqin，腾讯前端开发工程师

说到 Code Review，经常有同学会问，究竟从哪些方面下手？除了一些抽象的 Review 原则，有没有更细化的实施准则来指导实践？

PCG 代码委员会曾推出过通道晋级代码检查报告。笔者打算在这些报告基础上，从代码格式、代码错误、代码习惯、代码优化四个角度，并结合腾讯医典前端 Code Review 过程中遇到的一些 bad case，逐一列出更细化的实施准则。希望对各位有一定的参考价值。

1. 代码格式

代码格式问题完全可以通过自动化工具来解决。 标准的 eslint 规则( 如Airbnb或公司统一推出的eslint规则) + husky( 本地pre-commit校验 ) + 远端 CI 流水线 eslint 校验（开启cache，增量校验）就可以解决。

2. 代码错误

2.1 是否存在会导致内存泄露的代码

对于 SPA 应用，用户无需刷新浏览器，所以要想确保垃圾回收生效，我们需要在组件对应生命周期里做主动销毁。

1）存在不必要的全局变量且未及时解除引用

全局变量，除非你关闭窗口或者刷新页面，才会被释放，如果缓存大量数据，很可能导致内存泄露。 比如，我们之前就遇到过把 IM SDK放在全局window上，但在页面卸载时却没有解除引用。

mounted () {
  window.im=TWebLive.createIM({ SDKAppID });
}

解决方案：在页面卸载时解除该全局引用。

destroyed () {
  window.im=null;
}

其实该 im 实例也不需要挂在window上，直接绑定在vue实例上即可，组件销毁时该实例也会销毁；但没有绑定在vue实例上的一定要主动销毁。

2）闭包内部变量未被销毁

来看一个容易忽视的闭包引发内存泄漏的例子。 outer函数内部定义了两个函数: unused 和 foo。虽然inner函数中并没有使用outer函数中的变量，但是由于unsed函数使用了outer函数的bar变量，bar也不会被释放，所以foo相当于隐式持有了bar。每次执行outer，bar都会指向上一次的foo；而foo也会隐式持有bar，这样的引用关系导致bar和foo都无法释放。

let foo=null;

function outer() {
  let bar=foo;
  
  // 该函数历史原因，调用方被注释掉。并无调用
  function unused () {
    doSomething();
    console.log(`unused ${bar}`)
  }
  
  // foo赋值
  foo={
    bigData: new Array(10000),
    inner: function () {
       doSomething();
    }
  }
}

for (let i=0; i < 1000; i++) {
  outer();
} 

解决方案：在 outer 执行完毕时手动释放bar。这样，隐式持有 bar 的 foo 也没有其他变量引用，也会被回收了。

let foo=null;

function outer() {
  let bar=foo;
  
  // 该函数历史原因，调用方被注释掉。并无调用
  function unused () {
    doSomething();
    console.log(`unused ${bar}`)
  }
  
  // foo赋值
  foo={
    bigData: new Array(10000),
    inner: function () {
       doSomething();
    }
  }
  
  bar=null; // 手动释放bar
}

for (let i=0; i < 1000; i++) {
  outer();
} 

3）定时器是否及时清理

常见的情况是mounted设置了定时任务，但却没有及时清理。

mounted () {
  this.timer=setTimeout(()=> {
    doSomething();
  }, 300)
}

参考写法，页面销毁时需清理定时器：

destroyed () {
  if (this.timer) {
    clearTimeout(this.timer)
  }
}

4）监听事件是否有解绑

window/body 等事件需要解绑：

mounted() {
  window.addEventListener(‘resize’, this.func)
}
beforeDestroy () {
  window.removeEventListener('resize', this.func);
}

5）第三方库的销毁函数，在页面卸载时也需要调用，比如EventBus：

destroyed () {
  this.eventBus.off()
}

6）v-if 指令导致的内存泄露

拿 vue 官网避免内存泄漏 的例子来看下。

v-if 指令只是控制虚拟DOM的添加和移除，但是由 Choices.js 添加的 DOM 片段并没有被移除。

<template>
  <div id="app">
    <button v-if="showChoices" @click="hide">Hide</button>
    <button v-if="!showChoices" @click="show">Show</button>
    <div v-if="showChoices">
      <select id="choices-single-default"></select>
    </div>
  </div>
</template>

<script>
new Vue({
  el: "#app",
  data: function () {
    return {
      showChoices: true
    }
  },
  mounted: function () {
    this.initializeChoices()
  },
  methods: {
    initializeChoices: function () {
      let list=[]
      for (let i=0; i < 1000; i++) {
        list.push({
          label: "Item " + i,
          value: i
        })
      }
      new Choices("#choices-single-default", {
        searchEnabled: true,
        removeItemButton: true,
        choices: list
      })
    },
    show: function () {
      this.showChoices=true
      this.$nextTick(()=> {
        this.initializeChoices()
      })
    },
    hide: function () {
      this.showChoices=false
    }
  }
})
</script>

解决办法是在hide方法里调用 Choices.js 的API来清理DOM片段：

hide: function() {
  this.choicesSelect.destroy();
}

以下是优化前、后的 JS Heap 对比图：

2.2 异步操作是否有异常处理

异步操作拿接口请求来说，大家都知道的是，使用 promise 时要有.catch 处理。但使用 async/await 时，有.catch 处理的，也有try...catch处理的使用方法。这里推荐使用.catch。原因在于：

• 可以控制接口请求出错后，是否要阻塞后续业务逻辑执行。
• .catch里的 error 能明确知道是接口请求导致的错误，而不需要再对 error 进行分类判断，是接口200返回后的业务逻辑处理报错还是接口报错。

// CASE 1: 接口报错，阻塞业务逻辑执行
async fetchList() {
  const res=await someApi().catch(error=> {
  // error处理逻辑
  })
  if (res) {
    doA();
  }
 }
 
 // CASE 2: 接口报错，不阻塞业务逻辑执行
 async fetchList() {
  const res=await someApi().catch(error=> {
  // error处理逻辑
  })
  doA();
 }
 
 // CASE 3:使用try...catch的情况
 async fetchList() {
  try {
  const res=await someApi()
     doA(); 
  } catch (error) {
    // 接口请求出错 + 接口响应成功后业务逻辑处理出错都会进入catch block。需要进一步区分错误类型 
    if (error.bizcode !==0 || error.retcode !==0) {
      reportApiError(error)
    } else {
      reportBusinessError(error)
    }
  }
 }

2.3 取值时是否进行了空判断、调用函数时是否进行了类型判断**

拿医典3月中下旬的错误日志来说，这类错误在错误日志中占了1/3。

如果项目里已经全量使用了Typescript，这类错误应该都可以避免。 但如果项目里还存在 js 代码，可以使用lodash.get来做空判断，在调用函数之前要对函数做类型判断。

2.4 存在无意义的 if else代码块或考虑漏的条件

无意义的if else代码块，指的不仅是空的if else 代码块，还有只写了console.log 的情况。 另外，也存在条件判断过于复杂，else情况考虑不全，导致逻辑没有正常处理的情况。

//  else 代码块里只写了console.log 
if (a) {
} else {
  console.log('something')
}

// 条件判断过于复杂，else情况考虑不全，导致逻辑不能正常处理
if ((a && (b || c)) || d || (e && (f || g))) {
} else {
  doSomething()
}

解决办法： 开启eslint no-empty 规则，不允许有空的block。但这个插件的问题在于，如果是无效的代码块，比如在else代码块只做了console.log操作，并不会检测出来。 另外，较为复杂的条件判断尽量拆成单独的变量，并分别配上注释说明，这样可以防止逻辑处理漏。

2.5 存在无意义的 catch 代码块

和无意义的 else 代码块一样，也存在空catch代码块、只有console.log的catch代码块的情况。

// bad case
try {
  doSomething();
} catch (e) {
}

// bad case
try {
  doSomething();
} catch (e) {
  console.log(e);
}

// bad case
somePromise().then(()=> {
  doSomething()
}).catch(()=> {
})

somePromise().then(()=> {
  doSomething()
}).catch((e)=> {
  console.log(e)
})

为了解决这个问题，医典这边做了一个eslint插件@tencent/eslint-plugin-medical，能够检查try catch里的catch代码块、promise 的catch代码块，是否为空，是否只有console调用。cuow当然，有些时候，并不需要对异常逻辑进行额外业务逻辑处理，catch里可以加一个上报。

2.6 是否含有安全风险的代码

这一步可以在流水线里接入安全风险检测插件进行处理。以下是医典接入后的一个示例分析

确实存在一些误判，比如会把mixin关键字当做泄露人名来进行告警，开发人员可以对照分析是否需要处理。

An

前端常见的硬编码场景有：

• 请求参数和返回对象 比如之前就遇到过，开发同学把mock的请求参数id，放到了线上的情况。（因为该接口是根据医生id，拉取评论，评论也是后端灌的假数据，所以在测试阶段都没发现）

// 请求参数硬编码
getCommentsApi({ doctorId: 1 }).then((res)=> {
  doSomething()
}).catch(()=> {
  handleError();
})

也出现过ABtest接口，开发同学本地模拟返回对象，忘记删除硬编码的情况

fetchABTestApi().then((res)=> {
  // res.data
  const obj={
    imgSrc: 'xxx',
    name: 'qinmu'
  }
})

接口mock的硬编码，完全可以通过使用mock平台来解决。推荐使用专业的接口管理平台来进行接口管理、mock等，这里我们使用的是腾讯内部接口管理平台tolstoy。该产品还未正式开源，欢迎提前关注。

• 路由参数

// bad case 硬编码1001
const isActive=this.$route.query.id==='1001'

// good case 写到配置信息中。这样，id和状态的对应关系一目了然，便于管理和维护。
const idConfig={
  1001: STATUS.ACTIVE
}
const isActive=idConfig[this.$route.query.id]===STATUS.ACTIVE

2.8 格式校验

输入框的校验规则除了满足产品需求，比如多少字符以内、允许哪些字符，还有一个点：前后端需要校验规则保持一致。最好用统一的正则表达式，不然容易造成前端校验通过、后端校验不通过的情况。

上传文件，前后端需求校验文件格式、文件大小。尤其是后端，需要对 content-type 为 text/html 的加以限制，防止出现安全问题。我们已经有过此类安全问题的工单了。

3. 代码习惯

3.1 if-else嵌套不能超过4层

拒绝面条代码，减少代码中各种结构的嵌套，例如 if-else、try-catch、循环等。尽量控制在三层以内，增加可读性、降低圈层复杂度。 你肯定不愿意维护这样辣眼睛的代码：

async getConfig(id, type='', isReset=false) {
  try {
    if (liveid) {
       const res=await someApi(id);
       if (res && res.info) {
         const { status }=res.info
         status===2 && doA({id, type});
         if (status===1 || status===2 || status===4) {
           this.setData({
             info: res.info,
             status,
           })
           if (isReset) {
             this.setData({
               current: 0,
               index: 0
             })
             status===2 && doB();
           }
           return { code: 0};
         }
         return doC();
       } else if (isReset) {
         resetSomething();
       } else if (isReset && type===someType) {
         handleType();
       }
       return wx.showModal({ //... })
    }
  } catch (error) {
    if (error.code===1001) {
      reportA();
    } else {
      reportB();
      doD();
    }
  }
}

3.2 Don't repeat yourself

逻辑相同或相似的代码，应封装为函数进行调用。

// bad case 都有展示modal的逻辑，开发同学直接复制粘贴
function handleA(msg) {
  wx.showModal({
    title: '提示',
    content: msg,
    showCancel: false,
    confirmText: '确定',
    confirmColor: '#02BACC',
    success: (res)=> {
      if (res.confirm) {
        doA();
       }
     },
  });
}

function handleB(msg) {
  wx.showModal({
    title: '提示',
    content: msg,
    showCancel: false,
    confirmText: '确定',
    confirmColor: '#02BACC',
    success: (res)=> {
      if (res.confirm) {
        doB();
       }
     },
  });
}

function handleC(msg) {
  wx.showModal({
    title: '提示',
    content: msg,
    showCancel: false,
    confirmText: '确定',
    confirmColor: '#02BACC',
    success: (res)=> {
      if (res.confirm) {
        doC();
       }
     },
  });
}

解决方案，封装showModal函数。

function showModal (msg) {
  return new Promise((resolve, reject)=> {
    wx.showModal({
      title: '提示',
      content: msg,
      showCancel: false,
      confirmText: '确定',
      confirmColor: '#02BACC',
      success: (res)=> {
        if (res.confirm) resolve()
      },
      fail: (err)=> {
        reject(err)
      }
    })
  })
}

funtion handleA(msg) {
  showModal(msg).then(
    doA();
  ).catch(()=> { catchHandler();})
}

funtion handleB(msg) {
  showModal(msg).then(
    doB();
  ).catch(()=> { catchHandler();})
}

funtion handleC(msg) {
  showModal(msg).then(
    doC();
  ).catch(()=> { catchHandler();})
}

3.3 不建议直接修改Object原型(或者Function, Array原型等)

在Object.prototype上定义方法就相当于C++里定义宏, 而且还是 #define private public 这种。 从可靠性来说，多人协作很容易出现冲突。 从兼容性来说，你不能保证后续推出的原生方法实现和你现有的一致，也不能保证多个库之间对该方法的实现一致。比较有名的故事是 prototype库的getElementsByClassName。在还没有这个原生方法之前，prototype这个库实现的是返回Array，并且加了“each”方法：document.getElementsByClassName('myclass').each（doSomething）；但原生方法出来后，返回的是NodeList，并没有each方法；所以就悲剧了。 详细说明可以看：Nicholas C. Zakas 的 Maintainable JavaScript: Don’t modify objects you don’t own

3.4 回调嵌套不建议超过3层回调嵌套

减少回调的嵌套，避免产生callback hell:

fs.readdir(source, function (err, files) {
  if (err) {
    console.log('Error finding files: ' + err)
  } else {
    files.forEach(function (filename, fileIndex) {
      console.log(filename)
      gm(source + filename).size(function (err, values) {
        if (err) {
          console.log('Error identifying file size: ' + err)
        } else {
          console.log(filename + ' : ' + values)
          aspect=(values.width / values.height)
          widths.forEach(function (width, widthIndex) {
            height=Math.round(width / aspect)
            console.log('resizing ' + filename + 'to ' + height + 'x' + height)
            this.resize(width, height).write(dest + 'w' + width + '_' + filename, function(err) {
              if (err) console.log('Error writing file: ' + err)
            })
          }.bind(this))
        }
      })
    })
  }
})

建议使用promise、async/await 的方式让代码更为清晰可读；也可以将callback要做的事拆成独立的function，并分别对err进行处理。

3.5 函数不超过80行

函数尽量精简在80行以内，并且以小function进行组织，方便维护、复用。

3.6 缺少注释及注释规范化

除了知道下面的逻辑是在绘制canvas，其他逻辑你能看懂吗？

function doSomething() {
      let count;
      if ((count=width * height / 1000000) > 1) {
        count=~~(Math.sqrt(count) + 1);
        const nw=~~(width / count);
        const nh=~~(height / count);
        const tCanvas=document.createElement('canvas');
        const tctx=tCanvas.getContext('2d');
        tCanvas.width=nw;
        tCanvas.height=nh;

        for (let i=0; i < count; i++) {
          for (let j=0; j < count; j++) {
            tctx.drawImage(img, i * nw * ratio, j * nh * ratio, nw * ratio, nh * ratio, 0, 0, nw, nh);
            ctx.drawImage(tCanvas, i * nw, j * nh, nw, nh);
          }
        }
      } else {
        ctx.drawImage(img, 0, 0, width, height);
      }
}

常用的注释分类有这些，建议参考 JSDoc：

1）文件注释

2）变量注释

3）常量注释

4）函数注释

5）枚举注释

6）类的注释

7）类的属性注释

3.7 注释与实现功能不符

这一条在团队里是出现过现网bug的。

故事背景是开发M在重构代码时，设置底部栏状态这一逻辑已经封装出来，所以根据注释，下面几行代码做的事情也是设置底部栏状态，开发M就把这几行代码都删掉了。但是注释下面的代码，除了做设置底部栏状态的事情，还有一个setBanner 函数，是为了设置banner位的，也被连同删掉，进而导致了bug。

追溯原因，设置底部栏状态是A同学做的，设置 banner 位是B同学做的，B同学在没有看注释的情况下，直接把setBanner放在了错误的位置上。

function fetchData() {
  // ...
  doManythings();
  
  // 设置底部栏状态
  setBanner();
  fetchStatusApi().then(res=> {
    // ...
  }).catch(()=> {
    // ...
  })
}

3.8 避免存在大量注释掉的无用代码

git的版本管理能帮我们回溯之前的代码。如果项目里存在大量注释掉的代码，会降低可读性。

3.9 避免遗留大量多余的console.log调试日志

虽然console.log调试日志在生产环境构建时不会输出，但就本地开发环境来说，代码里惨杂过多console.log调试日志，控制台满屏的调试日志，对于每个接手的开发都是噩梦。另外，就像上面说的一样，catch处理或else分支里存在只打console.log而不做任何处理的情况。尽量避免少使用console.log，也可以减少这类意外的发生。

所以，日常开发调试建议使用浏览器sources tab的断点调试；另外，就算要输出调试日志，也不止有console.log可以使用，参考这篇文章。你可以使用console.table等来格式化输出

3.10 存在很多eslint-disable注释

我能想到的允许 eslint-disable的场景只有一种，那就是解构后端返回对象。后端返回对象属性名是下划线，这个时候可能需要 // eslint-disable-next-line camelcase。其他情况我都不建议使用 eslint-disable，尤其是整个文件全局eslint-disable。

之前遇到过某文件全局禁用"no-undef"规则，结果代码里使用了未定义的变量，导致现网bug。如果你有全局定义的变量，建议写在eslintrc.js的globals字段里。当然，就正如上文代码错误-内存泄露提到的一样，非必要情况，不建议使用全局变量。

3.11 没有使用空行对代码分组

为了增强可读性，建议使用空行对代码分组。

3.12 命名规范

常见的不规范命名有这些，会让之后维护的同学很懵逼：

• 单词拼写错误，比如submitForm，写成submitFrom。
• 中英文混用。比如gotoZaihai。你能知道这是什么意思吗？其实是跳到灾害专区活动页。goToDisasterZone是不是要好一点，同学？
• 以1-9、a-z命名 在项目里我曾经见过不知道怎么命名，就type1、type2、type3直接上了，也不写注释。非常让人抓狂。
• 混用命名格式 就评论列表，代码里有comments、commentList、也有commentData、commentsData。？？？能规范统一一下吗。
• 单复数不分 明明是一个列表数据，非要用单数表示，比如 disease。建议区分下单复数，如果是数组就用List来表示。
• 动词、名词、形容词不分 比如，一个函数名，命名为名词“doctor”；而一个Vue computed属性，又命名为getUserInfo；表示关闭状态，命名为“close”；真是让人非常头疼。

// bad case 1
function doctor () {}
 
// bad case 2
computed: {
  getUserInfo() {}   
}

// bad case 3
close=false; 

同学，就不能好好写代码吗？

// good case 1
function getDoctorInfo() {}

// good case 2
computed: {
  getUserInfo() {}   
}

// good case 3
closed=false

3.13 过多的非业务逻辑相关代码(如超过10行的上报, 参杂在业务逻辑里)

如果在业务逻辑里掺杂太多的上报，后续理解业务逻辑时需要看上报逻辑，查上报逻辑的时候也需要理解大量的业务代码。 点击埋点和曝光埋点都可以以属性的形式挂在元素上，通过冒泡，统一进行处理。

<button
	data-exposurename="test-exposure"
	:data-exposureval="{event:'bottom.btn'} | jsonStringify"
  data-eventname="button.click"
  :data-eventval="{id: buttonId} | jsonStringify"
/>

如果你上报的参数需要根据不同渠道来配置，建议封装出来，不要和业务逻辑耦合了。

3.14 没有README文档、或者README太简单、太作用有限

除了项目的READMD，每个模块都应该有各自的README，说明这个模块的功能点、技术实现方案等。看个人习惯，你也可以写在iwiki里，在README放一个iwiki的链接。

3.15 尽量使用export 而 不是 export default来导出

export default 有两个问题： 1）不利于tree shaking 2）如果使用了一个导出对象上不存在的属性，要运行时才能发现。

4.代码优化【持续更新中】

4.1 避免大量直接操作dom节点

直接操作DOM的性能损耗至少有两个地方：进行DOM操作的时候上下文切换 + DOM操作引起的页面重绘

4.2 避免使用delete

delete 操作符并不会释放内存，而且会使得附加到对象上的hidden class失效，让对象变成slow object。 （hidden class是V8为了优化属性访问时间而创建的隐藏类） 来看一下执行速度对比：undefined > delete > omit 。

4.3 是否引用了不必要的npm包

比如做一个简单图表的需求，不选轻量的库，非要整一个echarts；或者实现一个简单的代码编辑器，monaco-editor有min版本不使用，非要引用一整个monaco-editor。还有，lodash没法做tree-shaking，要么引用一个具体的子包lodash.get，要么引用lodash-es，非要引用一整个lodash。

4.4 尽量使用CDN地址的图片

如果代码里引用的是本地图片，构建打包会有耗时。可以在引用之前就把图片传到cdn上，代码里直接使用cdn地址。

以上就是CR的细则了。

手都写麻了。当然，肯定还有很多遗漏的点，欢迎补充。