公众号｜**沐洒（ID：musama2018）** 关注我，带你学点有用的前几天在前端技术群里聊起Code Review的事，大伙儿似乎都憋了一肚子气： ![图片](https://p3-juejin.byteimg.com/tos-cn-i-k3u1fbpfcp/be44574222ca43d089c13eb0d044ead7~tplv-k3u1fbpfcp-zoom-1.image) ![图片](https://p3-juejin.byteimg.com/tos-cn-i-k3u1fbpfcp/51ec07f1ffd54158a1df7b058358c96c~tplv-k3u1fbpfcp-zoom-1.image) 我觉得这份难言之隐应该要让更多人看到，就跟Henry约了个稿： ![图片](https://p3-juejin.byteimg.com/tos-cn-i-k3u1fbpfcp/d619d6887675486da97ad92a397506b4~tplv-k3u1fbpfcp-zoom-1.image) 于是Henry赶在周末，一边带娃，一边给我抹眼泪整理（脱敏）出了这篇小小的屎山合集，供大家品鉴。以下是正文。（文字大部分是**Henry**所写，沐洒进行了一些精简和调整） *** # 1. 直接操作DOM ```javascript const a = document.querySelector('.a'); const scrollListener = throttle(() => { const currentY = window.scrollY; if (currentY > 100) { a.classList.add('show'); } else { a.classList.remove('show'); } }, 300); window.addEventListener('scroll', scrollListener); return () => { window.removeEventListener('scroll', scrollListener); }; ``` 上面的代码在监听`scroll`方法的回调函数中，直接上手修改DOM的类名。众所周知，React属于响应式编程，大部份情况都不需要直接操作DOM，具体原因参考[官方文档](https://react.dev/learn/manipulating-the-dom-with-refs) 优化方法也很简单，充分发挥响应式编程的优点，用变量代替即可： ```javascript const [refreshStatus, setRefreshStatus] = useState(''); const scrollListener = throttle(() => { if (tab.getBoundingClientRect().top < topH) { setRefreshStatus('show'); } else { setRefreshStatus(''); } }, 300); return ; ``` > 沐洒：（`2023-05-23 15:01:42`） > 这个例子举得不太好，评论区也和大家讨论了比较多了，感谢大家指出来。 > 确实，类似上面这个例子的场景（滚动处理），其实反而是直接操作dom更好一些，毕竟滚动是`高频操作`，很吃性能，用响应式编程的方式反而拖累了性能。 > 不过这里其实重点是想说尽量`不要直接操作dom`，而是尊重框架的结构化表达。当然，`可读性`和`性能`的平衡是需要拿捏分寸的。 # 2. useEffect不指定依赖依赖参数缺失。 useEffect(() => { console.log('no deps=====') // code... }); 这样的话，每次页面有重渲染，该useEffect都会执行，带来严重的性能问题。例如我们项目中，这个useEffect内部执行的是第一点中的内容，即每次都会绑定一个scroll事件的回调，而且页面中有实时轮询接口每隔5s刷新一次列表，用户在该页面稍加停留，就会有卡顿问题出现。解决方案很简单，根据useEffect的回调函数内容可知，如果需要在**第一次渲染之后**挂载一个scroll的回调函数，那么就给useEffect第二个参数传入空数组即可，参考官方文档( useEffect(() => { // code... }, []); # 3. 硬编码硬编码，即一些数据信息或配置信息直接写死在逻辑代码中，例如 ![图片](https://p3-juejin.byteimg.com/tos-cn-i-k3u1fbpfcp/df13909977a44d75aa0ea2e4555878b7~tplv-k3u1fbpfcp-zoom-1.image) 这两行代码本意是从url上拿到指定的参数的值，如果没有，会用一个固定的配置做兜底。乍一看代码逻辑很清晰，但再想深一层，兜底值具体的含义是什么？为什么要用这两个值来兜底？写这行代码的同学可能很快可以解答，但是一段时间之后，写代码的人和提需求的人都找不到了呢？这个示例代码还比较简单，拿对应的值去后台可以找到对应的含义，如果是写死的是枚举值，而且还没有类型定义，那代码就很难维护了。

解决此类问题，要么将这些内容配置化，即写到一个config文件中，使用清晰的语义化命名变量；要么，至少在硬编码的地方写上注释，交代清楚这里需要硬编码的前因后果。 > **沐洒**： > > 关于硬编码问题，我在之前的一篇关于“配置管理”的文章里有详细阐述和应对方案，感兴趣的朋友可以看看《[小白也能做出满分前端工程：01 配置管理](https://juejin.cn/post/7224514908761686073)》 # 4. 放任文件长度，只着眼于当下的需求很多同学做需求、写代码都比较少从全局考虑，只关注到当前需求如何完成。从“战术”上来说没有问题，快速完成产品的需求、快速迭代产品也是大家希望看到的。可一旦只关注“战术实现”而忽略“战略设计”，除非做的产品是月抛型的，否则一定会遇到旧逻辑难以修改的情况。如果再加上一个文件被多达10余人修改过的情况，那么每改一行代码都会是一场灾难，例如最近接手的一个页面：

单文件高达1600多行！哪怕去除300多行的注释，和300多行的模板，剩下的逻辑代码也有1000行左右，这种代码可读性就极其糟糕，必须进行拆分。而很常见的是，由于每一任经手人都疏于考虑全局，导致大量代码毫无模块化可言，甚至出现多个useEffect的依赖是完全相同的：

这里明显还有另一个问题：**滥用hooks。** 从行号可以看出来确实是相同的依赖写了多个useEffect，很明显是多个同学各写各的的需求引入的这些hooks。\ 这代码跑肯定是能跑的，但是很可能会出现多个hooks中修改同一个变量，导致其他地方在使用的时候需要搞一些很tricky的操作来修Bug。 # 5.变量无初始值在typescript的加持下，对变量的类型定义可以说是日益严格了。可是在一些变量的类型定义比较复杂的情况下，可能一个变量的字段很多、层级很复杂，此时有些同学就可能想偷个懒了，例如： const [variable, setVariable] = useState(); // some code... const queryData = function() { // some logic setVariable({ show: true }); }; useEffect(() => { queryData(); }, []); return variable.show ? : null; 这里的问题很明显，如果queryData耗时比较长，在第一次渲染的时候，最后一行的`variable.show`就会报错了，因为variable的初始值是undefined。所以声明变量时，一定要根据变量的类型设置好有效默认值。 # 6. 三元选择符嵌套使用网上很多人会推荐说用三元选择符代替简单的if-else，但几乎没有见过有人提到嵌套使用三元选择符的事情，如果看到如下代码，不知道各位读者会作何感想？ {condition1 === 1 ? "数据加载中" : condition2 ? "没有更多了" : condition3 ? "当前没有可用房间" : "数据加载中"} 真的很难理解，明明只是一个简单的提示语句的判断，却需要拿出分析性能的精力去理解，多少有点得不偿失了。这还只是一种比较简单的三元选择符的嵌套，因为当各个条件分支都为true时，就直接返回了，没有做更多的判断，如果再多做一层，都会直接把人的cpu的干爆炸了。替代方案： 1. **直接用if-else**，可读性更高，以后如果要加逻辑也很方便。 2. **Early Return**，也叫卫语句，这种写法能有效简化逻辑，增加可读性。 if (condition1 === 1) return "数据加载中"; if (condition2) return "没有更多了"; if (condition3) return "当前没有可用房间"; return "数据加载中"; 虽然不嵌套的三元选择符很简单，但是在例如jsx的模版中，仍然不建议大量使用三元选择符，因为可能会出现如下代码： return ( condition1 ? ( {condition3 ? "111" : "222"} {condition4 ? ( ) : null ) : ( {condition6 ? children1 : children2} ) ) 类似的代码在我们的项目中频繁出现，模版中大量的三元选择符导致文件内容拉得很长，很容易看着看着就不记得自己在哪个逻辑分支上了。像这种简单的三元选择符，做成一个简单的memo变量，哪怕是在组件内直接写变量定义（例如：const clsName = condition2 ? cls1 : cls2），最终到模板的可读性也会比上述代码高。 # 7. 逻辑不拆分 React hooks可以很方便地帮助开发者聚合逻辑抽离成自定义hooks，千万不要把一个页面所有的useState、useEffect等全都放在一个文件中： ![图片](https://p3-juejin.byteimg.com/tos-cn-i-k3u1fbpfcp/e59595579c3846958a40b83dc60a8ceb~tplv-k3u1fbpfcp-zoom-1.image) 其实从功能上可以对页面进行拆分，拆分之后这些变量的定义也就可以拆出去了。其中有一个很简单的原则就是，**如果一个逻辑同时涉及到了useState和useEffect，那么就可以一并抽离出去成为一个自定义hooks**。例如接口请求大家一般都是直接在业务逻辑中做： const Comp = () => { const [data, setData] = useState({}); const [loading, setLoading] = useState(false); useEffect(() => { setLoading(true); queryData() .then((response) => { setData(response); }) .catch((error) => { console.error(error); }) .finally(() => { setLoading(false); }); }); if (loading) return "loading..."; return

{data.text}

; } 根据上面的原则，和数据拉取相关的内容涉及到了useState和useEffect，这整块逻辑就可以拆出去，那么最终就只剩下： const Comp = () => { const { data, loading } = useQueryData(); if (loading) return "loading..."; return

{data.text}

; }; 这样下来，Comp组件就变得身份清爽了。大家可以参考阿里的ahooks库，里面收集了很多前端常用的hooks，可以极大提升开发效率和减少重复代码。 # 8. 随意读取window对象的值作为大型项目，很容易需要依赖别的模板挂载到window对象的内容，读取的时候需要考虑到是否有可能拿不到window对象上的内容，从而导致js报错？例如： window.tmeXXX.a.func(); 如果这个tmeXXX所在的js加载失败了，或者是某个版本中没有a这个属性或者func这个函数，那么页面就会白屏。好啦，最近CR常出现的8种屎山代码都讲完了，你写过哪几种？你们团队的代码中又有哪些让你一口老血喷出来的不良代码呢？欢迎评论区告诉我。 *** **技术交流群** 我建了个前端技术交流群，整体氛围非常好，每天大家都会在群里真诚的交流技术问题，从前端到全栈，从尚未毕业的小年轻，到不惑之年的老大哥，大家都在拼命的汲取知识! 最近群里正在组织**前端项目实战**练习，很快就要开始大干一场了！有想要交流&学习前端知识的老少朋友，欢迎加入我们！ **进群方式**【重要】：关注公众号 “**沐洒**”，回复“**技术群**”，获取最新群聊二维码。 *** **全文完。** **码字不易，如果你还想继续看我写的东西，就关注我吧（记得加星标🌟哦），顺便给个赞👍或点一下在看，你的支持是我继续写下去的动力。** 公众号｜**沐洒（ID：musama2018）** 关注我，带你学点有用的