前几天在前端技术群里聊起Code Review的事，大伙儿似乎都憋了一肚子气：
我觉得这份难言之隐应该要让更多人看到，就跟Henry约了个稿：
于是Henry赶在周末，一边带娃，一边给我抹眼泪整理（脱敏）出了这篇小小的屎山合集，供大家品鉴。
以下是正文。
（文字大部分是Henry所写，沐洒进行了一些精简和调整）
1. 直接操作DOM
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）。
优化方法也很简单，充分发挥响应式编程的优点，用变量代替即可：
const [refreshStatus, setRefreshStatus] = useState(''); const scrollListener = throttle(() => { if (tab.getBoundingClientRect().top < topH) { setRefreshStatus('show'); } else { setRefreshStatus(''); } }, 300); return <div className={['page_refresh', refreshStatus].join(' ')}/>;
2. useEffect不指定依赖
依赖参数缺失。
useEffect(() => { console.log('no deps=====') // code... });
这样的话，每次页面有重渲染，该useEffect都会执行，带来严重的性能问题。例如我们项目中，这个useEffect内部执行的是第一点中的内容，即每次都会绑定一个scroll事件的回调，而且页面中有实时轮询接口每隔5s刷新一次列表，用户在该页面稍加停留，就会有卡顿问题出现。解决方案很简单，根据useEffect的回调函数内容可知，如果需要在第一次渲染之后挂载一个scroll的回调函数，那么就给useEffect第二个参数传入空数组即可，参考官方文档(https://react.dev/reference/react/useEffect#useeffect)。
useEffect(() => { // code... }, []);
3. 硬编码
硬编码，即一些数据信息或配置信息直接写死在逻辑代码中，例如
这两行代码本意是从url上拿到指定的参数的值，如果没有，会用一个固定的配置做兜底。
乍一看代码逻辑很清晰，但再想深一层，兜底值具体的含义是什么？为什么要用这两个值来兜底？写这行代码的同学可能很快可以解答，但是一段时间之后，写代码的人和提需求的人都找不到了呢？
这个示例代码还比较简单，拿对应的值去后台可以找到对应的含义，如果是写死的是枚举值，而且还没有类型定义，那代码就很难维护了。
解决此类问题，要么将这些内容配置化，即写到一个config文件中，使用清晰的语义化命名变量；要么，至少在硬编码的地方写上注释，交代清楚这里需要硬编码的前因后果。
沐洒：
关于硬编码问题，我在之前的一篇关于“配置管理”的文章里有详细阐述和应对方案，感兴趣的朋友可以看看《小白也能做出满分前端工程：01 配置管理》
