稀土掘金技术社区 01月19日
前端 Code Review 常见问题
index_new5.html
../../../zaker_core/zaker_tpl_static/wap/tpl_guoji1.html

 

本文总结了前端开发中常见的代码规范问题，包括commit message、依赖管理、包引用顺序、方法命名、调试代码、eslint使用、注释、文件命名、CSS属性顺序、类名规范、内联样式、样式作用域、git使用、lint配置、README编写、forEach和map混用、option chain兼容性、回调函数、async/await滥用、根据现象编程、穷举代码、magic number、常量抽离、函数副作用、以及UI逻辑和代码混杂等问题。旨在帮助开发者提高代码质量，减少潜在bug，并养成良好的编程习惯。

📦 **依赖管理与引用规范**：项目中应明确区分 dependencies 和 devDependencies，避免混用。包引用顺序需分类，如CSS、项目内文件、三方包，并利用工具自动调整。

📝 **代码风格与规范**：方法名应使用动词短语，如`fetchProductList`而非`productListFetch`。避免遗留console调试代码，合理使用eslint，不滥用disable，并及时删除大段注释代码。文件名与代码类型匹配，CSS属性按布局、文本、其他等顺序排列，类名使用小写连字符。

🛠️ **工程实践与习惯**：不应将静态样式内联，避免样式反复覆盖。合理使用git，规范commit message，并使用tag标识版本。完善README文档，避免使用脚手架默认内容。区分forEach和map的语义，option chain考虑兼容性，避免不必要的回调函数和await串行调用。

💡 **代码逻辑与优化**：避免根据现象编程，应回归本质，用直接方式表达逻辑。避免穷举方式写代码，应使用更简洁的逻辑。提炼magic number为常量，避免函数副作用。区分UI思路与实现，避免代码混杂。遵守DRY原则，避免复制粘贴代码。

原创 谦行 2025-01-16 08:30 北京

点击关注公众号，“技术干货”及时达！

点击关注公众号，“技术干货” 及时达！

规范类问题

无意义的 commit message

建议所有同学了解一下看看 commit message 规范，并且为项目配置 commit lint，感受一下两个项目

无用和缺省的依赖

使用 depcheck 感受下有没有触目惊心

Unused dependencies
* @alife/bc-big-brother
* @alife/scc-gangesweb-inquiry
* babel-runtime
* caniuse-lite
Unused devDependencies
* @ali/kyle-config-saga
* style-loader
Missing dependencies
* @src/utils: ./src/utils/do-dot.js
* @alife/bc-safe-log: ./src/utils/log.js
* @src/pages: ./src/pages/template/showroom-list/index.jsx
* @src/modules: ./src/pages/template/showroom-gallery/index-v2.jsx
* @src/components: ./src/pages/template/promotion-venue/index.jsx
* @alife/bc-video-player: ./src/pages/template/pla-detail/product-detail.jsx

dependencies 和 devDependencies 不分

因为 tnpm 打平了依赖，混写虽然最终都能 work，但把 react 依赖写在  devDependencies 等行为依旧很业余，如果确实不是项目直接依赖，可以看看 peerDependencies 的使用

包引用顺序

前端代码中用 import 一般有三种情况

虽然没有明确规定的顺序，但最好三者分类引用，业界也有非常多可以自动调整的工具，可以配合 eslint、prettier 等工具使用

甚至 package.json 字段的顺序也可以自动调整呢 sort-package-json

方法名不是动词短语

这是个很基础的约定，看到 productListFetch()这样的方法名会很奇怪，不如 fetchProductList

遗留的 console 调试代码

eslint 还是要配置一下，最好级别不是 warning，而是 error，确实有必要可以使用 eslint-disable

不合理的 eslint-disable 范围

上面提到 eslint-disable，很多同学在滥用该能力，为了一行代码的某个特性，全文件禁用了 eslint，合理做法应该是只禁用某一行的某一个特性

// eslint-disable-next-line no-alert
alert('foo');

不清楚 Dsiabling Rules 可以看看官方文档 https://eslint.org/docs/latest/user-guide/configuring/rules#disabling-rules

大段注释代码不删除

无力吐槽，都有 git 记录了，只要好好写 commit message，想找回代码很容易，即使一定要保留，大促后就移除注释（也是很不好的处理方案），最起码加个  TODO 标识，让 IDE 提醒自己还有债没还

文件名用了 ts，实际代码和 ts 无关

如果不认可 TypeScript 带来的价值，大可不必如此伪装

CSS 属性顺序

大部分代码规约对 css 属性也没有限制，但我们一般会遵从如下的约定

    布局、定位相关 display / position / float  等

    自身属性 width / height / margin / padding  等

    文本属性 color / font / text-decoration / text-align 等

    其它 cursor / border/ shadow / background 等

顺便说一句，也有自动调整的工具 prettier-plugin-style-order

CSS 类名规范

❝

在 CSS Modules 中，推荐使用驼峰命名 https://github.com/css-modules/css-modules#naming

❞

静态内联样式

function Demo (){
  return (
    <div className="product-list-container">
      {
        list.map(product => {
          return (
            <div style={{
              width: '200px',
              height: '400px',
              backgroundColor: '#efefef'
            }}>
            </div>
          );
        })
      }
    </div>
  );
}

这种图省事把静态样式内联，这样有两个弊端

    缺少语义，不知道为什么设置这样的属性

    在 list 场景会显著增加 DOM 体积，SSR 场景影响 download 时间

样式反复覆盖

调整样式时候遇到这种会不会感觉头大

尽可能让样式作用的精准的作用域，不要依赖层层覆盖

.container {
  margin: 0 auto;
  color: #333;
  &.mobile {
    color: #444;
  }
}

可以适当调整为

.container {
  margin: 0 auto;
  
  &.pc {
    color: #333;
  }
  &.mobile {
    color: #444;
  }
}

前端工程问题

没有价值的 README

项目的 README 不应该是脚手架生成的默认内容，不同类型的项目包含的信息可能不同，但都应该起到一个作用—— 项目需要交给其他同学，看 README 就够了，因此一般的前端项目 README 应该包含如下内容

常见方法错误使用

混用 forEach 和 map

可能是 React 让大家习惯了用 map，而忽略了其语义

❝

Array.prototype.map() 创建一个新数组，这个新数组由原数组中的每个元素都调用一次提供的函数后的返回值组成

❞
arr.map(item => {
  item.name += '--';
})

option chain 不考虑兼容

option chain 语法让我们避免了冗长的空指针检查，但大家习惯在赋值过程使用，却忽略了消费过程的兼容

const children = testObj?.elements?.items;
return (
  <>
    {
      children.map(item => {
        <span>{item}</span>
      })
    }
  </>
);

多一层包裹的回调函数

getList()
  .then(list => {
    console.log(list);
  });

是不是感觉这样的代码太熟悉不过？

getList().then(console.log);

await 带来的没必要串行调用

await 很好用，但不是所有时候都合适，可能会带来没必要串行调用

const user = await getUser(userId);
const favorites = await getFavaritesByUserId(userId);
setStore(user, favorites);

各个函数之间没有前后依赖关系时候应该尽可能并行

Promise.all([
  getUser(userId),
  getFavaritesByUserId(userId)
]).then(setStore);

不好的编程习惯

根据现象编程

if (indexOf('test') >= 0) {}
// 榜单前 3 位特殊处理
if (index < 4) {}
if (ieVersion >= 9) {
  // 使用 Array.isArray 方法
}

上面的三个语句都是根据句表象在处理逻辑，这种代码类似双重否定的 bool 值（ notDisabled）需要人多反应一次才能理解代码表达的含义，而且在后续的维护中很容易被人误改

我们应该回归现象的本质，用最直接的方式表达逻辑

if (indexOf('test') != -1) {}
if (index <= 3) {}
if (typeof Array.isArray === 'function') {
  // 使用 Array.isArray 方法
}

穷举方式写代码

这类问题其实是对“根据现象编程”的一种延伸，有一道面试题

❝

实现函数 compareVersion(v1, v2)，用例

compareVersion('1.0.0', '0.9.0') // 1

compareVersion('1.0.0', '1.0.0') // 0

compareVersion('1.0.0', '2.0.0') // -1

❞

会不会写出这样的代码

function compareVersion (v1, v2) {
  const ver1Arr = v1.split('.').map(Number);
  const ver2Arr = v2.split('.').map(Number);
  if (ver1Arr[0] > ver2Arr[0]) {
    return 1;
  } else if (ver1Arr[0] < ver2Arr[0]) {
    return -1;
  } else {
    if (ver1Arr[1] > ver2Arr[1]) {
      return 1;
    } else if (ver1Arr[1] < ver2Arr[1]) {
      return -1;
    } else {
      if (ver1Arr[2] > ver2Arr[2]) {
        return 1;
      } else if (ver1Arr[2] < ver2Arr[2]) {
        return -1;
      } else {
        return 0;
      }
    }
  }
  
  return 0;
}

例子夸张了一些，不过类似的案例非常多，用代码穷举所有可能，下面的写法感觉是不是更有逻辑一点

function compareVersion(v1, v2) {  
  const ver1Arr = v1.split('.').map(Number);
  const ver2Arr = v2.split('.').map(Number);
  for (let i = 0; i < 3; i++) {
    if (ver1Arr[i] > ver2Arr[i]) {
      return 1;
    } else if(ver1Arr[i] < ver2Arr[i]) {
      return -1;
    }
  }
  return 0;
}

感受下这段代码更好的写法是什么

function getModalWidth(length) {
  if (length <= 10) {
    return 285;
  }
  if (length > 10 && length <= 20) {
    return 570;
  }
   if (length > 20 && length <= 30) {
    return 855;
  }
   if (length > 30 && length <= 40) {
    return 1140;
  }
   if (length > 10 && length <= 20) {
    return 1425;
  }
}

第一直觉是不是改成这样

function getModalWidth(length) {
  if (length <= 10) {
    return 285;
  }
  if (length <= 20) {
    return 570;
  }
   if (length <= 30) {
    return 855;
  }
   if (length <= 40) {
    return 1140;
  }
   if (length <= 50) {
    return 1425;
  }
}

仔细分析下是不是可以改成这样

function getModalWidth(length) {
  return Math.ceil(length / 10) * 285;
}

Magic number & 常量抽离

if (column > 7) {}

每个人看到其他人代码中出现 Magic number 时候都会异常气愤，但到自己代码时候会觉得表意足够清晰，忽略数字变量的命名

除了注意了数字变量命名问题，对于全局反复使用的变量要注意抽离到公共文件中，便于后续的统一维护，很多工程脚手架甚至会初始化一个 constants文件夹，使用不同的文件存放各种常量

// 同时渲染模块数
export const DRIVE_MODULE_AMOUNT_ONCE = 3;
// 模块渲染失败最多重试次数
export const MODULE_RENDER_RETRY_TIMES = 2;

函数有副作用

({ activeIndex1, floorArray }) => {
  let finalLiveList;
  
  if (activeIndex1 === 0) {
    floorArray[0].list[0].cardType = floorArray?.[0]?.cardType;
    finalLiveList = [...floorArray[0].list, ...floorArray[1].list];
  } else {
    finalLiveList = [...floorArray[0].list];
  }
  
  return (
    <WaterFall
    activeIndex1={activeIndex1}
list={finalLiveList}
/>
  );
}

函数应该避免修改入参引发副作用，尽可能保证函数是纯函数

    如果函数的调用参数相同，则永远返回相同的结果。它不依赖于程序执行期间函数外部任何状态或数据的变化，必须只依赖于其输入参数

    该函数不会产生任何可观察的副作用

({ activeIndex1, floorArray }) => {
  let finalLiveList;
  
  if (activeIndex1 === 0) {
    finalLiveList = [...floorArray[0].list, ...floorArray[1].list];
    finalLiveList[0].cardType = floorArray?.[0]?.cardType;
  } else {
    finalLiveList = [...floorArray[0].list];
  }
  
  return (
    <WaterFall
      activeIndex1={activeIndex1}
      list={finalLiveList}
    />
  );
}

不区分思路与实现的 render

CR React UI 逻辑，最害怕看到的就是这样的代码

function Demo (){
  return (
    <div>
      <div className="demo1">
        <div>
          <div></div>
          <div></div>
        </div>
        <div>
          <div></div>
          <div></div>
        </div>
      </div>
      <div className="demo2">
        <div>
          {
            xxx && (
              <>
                <div></div>
                <div></div>
              </>
            )
          }
        </div>
        <div>
          <div></div>
          <div></div>
        </div>
      </div>
    </div>
  );
}

这种代码读起来实在是无从下手，整个 UI 的思路与实现杂糅在一起，需要了解作者的全部思路才能读懂。如果要 CR 一个解决“把大象塞进冰箱”的问题，那么我会希望首先了解到作者的整体思路是什么，可能有三步

    打开冰箱门

    把大象放进去

    关上冰箱门

开关冰箱门很好理解，快速看一下代码规约即可，重点放在把大象放进去的实现细节就可以了，如果又进一步做了思路的拆解，那代码读起来就更简单了

所以对于复杂的 UI 我希望看到的写法是这样的

function Component1 (){}
function Component2 (){}
function Component3 (){}
function Component4 (){}
function Demo() {
  return (
    <Component1>
      <Component2></Component2>
      <Component3>
        <Component4 />
        <Component4 />
      </Component3>
    </Component1>
  );
}

在处理复杂逻辑时候可以按照类似步骤处理一下

    画流程图

    每个节点定义成一个函数

    把函数串联起来

    为每个函数做实现

    测试

❝

相对于面向对象，可能大部分前端代码更适合用面向过程的写法

❞

违背 Dont Repeat Yourself 原则

这个感觉不用解释什么，但我们却在不停的违背，当想实现某个逻辑拿出的方案是“Copy 某段代码改改”时候，也需就已经违背了这个原则，无论什么理由，建议不要去按 Ctrl + C

养成好的编程习惯

上面讲的都属于知识和技巧，真正重要的是写好代码的意愿，很难想象不注重代码健壮性、可读性的工程师，会把当下的代码珍视为自己的代表作，用来流传。日常不会有那么多大事落在头上，此时、此行代码就是我。

技术素养——技术领域平素的修养。

点击关注公众号，“技术干货” 及时达！

阅读原文

跳转微信打开

Fish AI Reader

Fish AI Reader

AI辅助创作，多种专业模板，深度分析，高质量内容生成。从观点提取到深度思考，FishAI为您提供全方位的创作支持。新版本引入自定义参数，让您的创作更加个性化和精准。

FishAI

FishAI

鱼阅，AI 时代的下一个智能信息助手，助你摆脱信息焦虑

联系邮箱 441953276@qq.com

相关标签

代码规范 前端开发 编程习惯 代码质量 工程实践
相关文章
遇到不少人问我，不是计算机专业能学吗？ 基本只要他会这样问我，我就回答不能哦。 但是如果他问我，不是计算机专业如何入门？ 我就会告诉他，可以先看那个半小...
Symflower Launches DevQualityEval: A New Benchmark for Enhancing Code Quality in Large Language Models
高级工程师代码审查指南
在 React 中创建可重用组件：处理未来的无限变化
前端开发为何如此复杂？
Ask HN: 2024 年，您将如何从零开始研究前端开发？
死罪：如何在 CamelCase 中处理缩写（2012 年）
Tabnine Adds Support for Anthropic Claude 3 Model to Accelerate and Optimize Software Development
使用 NextJS 创建电子商务商店
如何使用 React.js 构建经典贪吃蛇游戏