这屎山代码终于让我给重构了😠,一起来看看这次重构的几个 case

3,016 阅读14分钟

扯皮

上个月新接手了公司内部的一个 AI 对话的移动端项目,yysy 当看到这项目代码第一眼时我就想逃离了😓,整个项目就一个对话页面但堆积的逻辑比较多,组件拆分的也不明确。但其实这还好,当时只是想着赶紧把要做的功能搞完甩手走人了,真正的导火索其实是上周五

因为周五要上线新功能,结果临发布时测出之前老业务逻辑中的 bug,初步推测是与新功能存在耦合,然后两个业务形成了一种很微妙的互相依赖关系

我和一位后端同学两人排查一中午饭都没吃,虽然找到问题了但发现在短时间内都不太方便下手去解掉它,因为之前老业务逻辑太乱了,最终无奈只能先回滚之前的实现😑

这周正好没有新功能迭代只剩 bug 修复了,赶紧趁这几天的时间把这坨屎给搞的好看点,但确实也比较费劲,毕竟屎上加屎要比屎上雕花可简单太多了,所以一边重构一边记录一些看到的问题

正文

我们从整体再到局部细节,部分编码风格和观念偏向于个人,各位看官请酌情食用🤪,当然由于我们的项目都是基于 Umi 的所以代码会以 React 为主,当然思想上都是相通滴

“拆”

为什么一开始我就想要逃离?我们的项目其实就只有一个这样的页面,核心逻辑实际上就是流式对话,跟掘金上的豆包侧边栏功能大体一样,所以后续都会以豆包作为视图来讲解

image.png 而我刚接手时整个 src 目录结构大概是这样的:

image.png

重点关注 components,当然这里我肯定不能截内部代码的图,xxx-component 代表了各种业务小组件,图上看着还挺有规律的,实际代码都是跟业务相关的命名组件,这样裸露的组件大概有二十来个吧

比较直观的问题就是即便组件命名上已经足够语义化了,但还是无法第一时间就能分辨出组件在视图上的具体位置

当然你可能会说大的项目谁能在刚接手时看 components 就能直接对照视图的,都是自己一点一点看业务逻辑链路搜索出来的🤔。但我还是想要强调我所展示的这个项目真的就只有那一个页面,如果就一个页面组件划分还这么混乱的话就更别说拓展了

为此我还下来复盘分析了一下之前那位接手同学编码时的心态,可能确实有他的道理:

  1. components 目录下堆积所有业务组件 => 项目初期只有一个页面没多少组件,所以懒得划分反正都是在一个页面下的,只是随着后续业务增多了🤔
  2. 所有组件名使用大驼峰,但文件名使用小写字母 + 连字符不够统一 => 个人编码习惯,再加上 css 方案使用的是 styled-components 不需要再单独抽离样式文件,因此每个组件直接对应一个文件🤔

针对于以上两点并结合我们的项目提出以下重构方案:

1. 所有组件或页面统一使用文件夹以及大驼峰命名,其 index 文件作为组件入口

image.png

为什么要这样做?因为随着项目越来越复杂,你的业务组件并非是最小单元,就拿我们的这个项目来说,跟掘金上的豆包一样:

image.png

可以看到我框选了三部分:ChatMessageListUserMessageAnswerMessage,很明显的可以看到它们的包含关系,这时候如果还是以原来单文件作为组件的话很难表示出这样的关系,更逆天的是这三个组件在我们的项目中直接全堆到 ChatMessageList 里了,光 DOM 结构 + 样式就有几百来行😅

所以我的工作就是将其拆分并使用文件夹,这样看起来十分清晰:

image.png

当然你可能会说这样写没考虑到子组件复用的场景。没错,如果两个业务组件同时依赖另一个组件,那这个组件完全可以抽离至 components 目录下的 common 文件夹中,该文件夹下的组件基本上属于最小单元了

除此之外以文件夹作为组件的另一个好处在于可能该组件下的业务逻辑过多,很方便进行抽离和拓展:

image.png

2. 即使只有一个页面也要考虑划分,按照页面结构做基本划分

像我们的这个项目很经典的三部分布局:HeaderBodyFooter,核心逻辑都在 BodyFooterimage.png

Body 核心对话逻辑不用说了,Footer 我们比豆包要复杂一些,涉及到快捷指令和上下文添加等交互操作,但这些在我刚接手的代码中都没有直观体现,所以直接这样改造:

image.png

这样把所有的业务组件全部映射到页面结构上可是清晰太多了,要知道我刚接手我们项目一个星期的时候因为新增了一个功能加了几个业务组件,第二周再看 components 时我就已经找不到我自己写的代码了😅

实际上在一些比较大的项目当中这样的页面结构划分应该聚焦到 pages 下,只不过我们的项目只有一个页面的话放到 components 下也说的过去:

image.png

“抽”

这一点更是逆天,但相信也是大多数屎山代码的通病,经典一个组件几千来行碰都不敢碰一下😅,我们的项目毕竟比较简单还好顶多小一千行

在之前我以为这种堆积代码都是由于这个组件逻辑过于复杂,但后面仔细阅读才发现这玩意儿真的就跟人的代码水平有关,我是不太理解为什么有人能够看着几千来行代码还不难受的

所以趁着我们这边业务不算太多,赶紧把这个现象扼杀在摇篮当中,怎么解?回到我们最喜欢的八股环节,这一点我印象里还是我在公司周会上第一次分享过的内容,不过估计也没几个人听,上图:

image.png

记得那时候我分享的一个标题就是 “真的明白这张图的含义吗”,因为我发现应届生经典八股背的一个比一个 6,组合式 API 优点什么的各种吟唱,结果真把它的优势发挥到业务代码中却没见到几个(当然按照现在应届生的卷度大概率是哥们井底之蛙了😆)

组合式 API 的最大优点我觉得就是上面这幅图:业务聚合,而实际上我看到的组合式 API 应用却都是上面图中的左半边,那我寻思还用毛组合式 API 啊,Vue 直接退回 Options API 吧,React 直接退回类组件吧😅

所以我在我们的项目中新功能业务开发时就遵循一个原则:hooks 抽离

hooks 实际上大体被分为两种:工具 hooks、业务 hooks

工具 hooks 可能大部分都被一些工具库集成了,比如像 Vue 的 VueUse、React 的 ahooks,在业务开发中比较少,这里就以我们项目中的一个小功能为例,代码没几行但是很具有代表性:

它的功能就是对话过程中需要不断将对话界面滚动至底部来让用户看到最新的内容:

GIF 2024-11-24 23-40-12.gif

这种滚动通常有两种实现方式:1. 内容 scroll 控制 2. 锚点控制

我们的实现方式是借助 scrollIntoView API 实现锚点控制

其实就是在对话消息列表下面加一个锚点元素,然后调用该 API 实现控制跳转:

Snap.png

GIF 2024-11-24 23-55-02.gif

实际上针对于这个功能就可以封装为一个 hooks: Snap.png

实际代码就这几行为什么还要封装呢?因为它在我们业务代码中不止一次用到,而且跨越多层级组件,所以直接在 store 中使用该 hook 再将 anchor 和滚动方法抛出去即可🧐

业务 hook 就更好说了,它可能是某个请求数据的封装,也可能是对一些数据的增删改查,总之与业务强相关,这里也举一个最基本的业务场景 ToDoList:

Snap.png

核心的思想都是业务代码聚合,每个功能点实际上就是一个业务 hook,如果每个项目都这样维护你的组件哪怕真有一千行看着也足够清晰

记得很早还没有学 React 的时候就在 B 站上刷到过这个视频:

为什么我不在我的 React 组件中使用 useEffect_哔哩哔哩_bilibili

之前只了解过 React 相关 hook 的基本使用,没上手写过代码看着不明觉厉,现在真的切身体会到这种思想的好处,现在看着老代码的 useEffect 就有一种莫名的恐惧感😣,真的隐藏 bug 太多了...

数据流

数据流更是老生常谈的问题了,代码逻辑晦涩难懂的一大问题就是因为组件之间的数据流混乱,我们的这个项目也不例外,直接上业务场景举例,当然这里做了精简: image.png

简单来讲这里有一个 Drawer 弹窗,其内部展示了一个 List 组件,里面包含多个 Item 组件,而这三个组件都引入了 Store 提供的方法:refreshListAction,该方法会刷新 List 组件重新获取数据

比较逆天的就是这里的 refreshListAction,它的触发场景有这些:

  1. 当打开 Drawer 组件时,在 Drawer 调用该方法获取 List 数据
  2. 在 Drawer 组件中涉及到清空操作,点击清空按钮后调用清空接口并调用该方法刷新 List 数据
  3. 在 Item 组件中涉及到删除操作,点击删除按钮后调用删除接口并调用该方法刷新 List 数据

问题在于我们真的需要 Store 存储这里的 refreshListAction 嘛?看似确实需要,因为它真的被三个组件用到了🤔

实际上这样的数据流十分混乱,因为这里相当于子组件 Item 在控制父组件 List 的行为,违背了单向数据流的原则,当然这样错么也不错,反正代码肯定能跑,就是接手的人难受了😆

跟着视图好不容易一层一层找到了 Item 组件,结果这里从 Store 里引了一个方法直接调用,又得回到原点看看这里的方法起什么作用,对哪些组件造成了影响,总之带来极大的心智负担

如何解决?遵循数据流规则就完事了,从 Item 组件来讲,它应该以回调的形式来告诉父组件触发的行为,而不是直接去控制父组件,比如我们这里的子组件只有两个操作:点击跳转详情,点击删除按钮

Snap.png

通常我习惯添加一个 success 回调,因为子组件可能当父组件完成操作之后要做一些额外操作,比如 message 提示成功等...

现在子组件不涉及到任何业务数据上的操作,这些都由父组件控制,它只针对于用户交互操作做出响应,这样的子组件看起来就十分清晰🧐

同样针对于 List 和 Drawer 也做出这样的改造,现在我们无需 Store 也能够实现上面的需求,只遵循一个原则:只能父组件去控制子组件,而不是子组件控制父组件

Snap.png

除此之外不知道都是哪养成的习惯,会想着把父组件的 setState 传递给子组件,想小头占据大头是吧😅

Snap.png

你知道我当时看到这样的数据流去每个子组件排查,结果到最后子组件只是做了清空操作:setContent("") 有多无语嘛😅,改成事件回调不更清晰?

Snap.png

其他 case

关于 useEffect

目前接触 React 业务代码有小半年了,总结下来发现有 80% 以上的 bug 都是来自于 useEffect,其实跟之前写 Vue 一样,为什么不建议 Vue 大量使用 watch 的原因也是如此

看似 useEffect 比较可控,设置的依赖项和执行机制都明摆在那挺清晰的啊,实际上随着在 effect 里的业务越来越多,它就越来越不可控

这里简单说两个在编码阶段看到的小问题吧,感觉 useEffect 就是需要多踩坑才能掌握🙃:

1. 依赖项不够精确: 这一点我在帮我们同届的同学排查问题时遇到了不少次,她们反馈的现象是 useEffect 的回调总是执行了多次,比如下面这段代码:

Snap.png

因为比较简短一眼基本上就能看到问题,实际上这里的 state 以及 useEffect 的回调逻辑在业务当中都比较复杂,但暴露的问题是一样的:回调中使用的是 info.id,而依赖项却使用了 info

由于 setState 的机制如果更新 info 的其他字段也会连带整个 info 对象更新,进而触发这里 useEffect 的回调,这不是我们期望的

2. 依赖项不够明确: 这个问题就是在我目前所接手的项目中出现的,简单还原下代码:

Snap.png

与上面不同的是 useEffect 的依赖项增加到了两个,而回调中也确实都用到了,看着好像没什么问题🤔

问题在于我们的实际业务其实不需要 info 改变时再重新执行 handleSomething 操作,但是又需要在这里传入最新的 info 值😅,一开始我们的项目里一大堆这种逻辑搞麻了...

想要解决这个问题其实可以无脑把 info 从依赖项中移除,这样 info state 更新后会 rerender 组件,传给 useEffect 的回调函数内实际上也会是一个新的函数,毕竟并不是空依赖项还没有闭包问题

但是后面别人维护时看到依赖项和回调没有对上时第一时间也会诧异这段逻辑,总之是不够清晰🤔

所以后来我复盘了一下这个问题的解法,其实关键就在于 effect 回调中使用的 info 真的必须是 state 么?不是,它是 state 的最新值,如果你对 ahooks 比较熟悉你一定知道这个 hook:useLatest - ahooks 3.0,那这个问题的解法实际是这样的,一般情况下我们也不会将 ref 设置为 useEffect 依赖项:

Snap.png

而如果 ahooks 玩的比较多的话,你还会知道这个 hook:useGetState - ahooks 3.0,它扩展了普通 state,返回第三个参数 getState 方法,源码实际上就是内部封装了一层 useLatest

Snap.png

这两种解法实际上都比死磕 state 作为依赖项要好的多

关于函数组件声明

这个其实只是个人习惯问题,只不过前段时间在逛 github 时在 create-react-app 的一个 PR,因此来记录一下😄

一般我们写函数组件会有这三种写法:

Snap.png

这三种写法我在一个项目中都见到过,想着无非就是没有统一罢了,后来看到这个 PR 后我其实更推崇的是 普通函数 + props 类型的方式

Remove React.FC from Typescript template by Retsam · Pull Request #8177 · facebook/create-react-app

省流版 React.FC 缺点,感兴趣的直接点进去看吧,里面有通俗易懂的例子说明: image.png

关于 styled-components 命名

styled-components 因为 Umi 内置了所以在项目还挺常见的,这里不介绍基本使用,只是想吐槽一下命名问题:

如果只看下面的 JSX 的话你能第一眼看出来这里的 ContextBox、InputBox 是业务组件还是 styled-components 么?🤔 反正我是看不出来

Snap.png

所以一般在使用 styled-components 时我会这样使用:

Snap.png

Snap.png

End

其实整个项目还有很多小问题,但是比较小儿科就没有整理,总的来讲即便是大厂的项目接手的人过多也不妨碍它是一坨屎😋。这年头还是开发新项目的质量更高一些,当然核心还是看开发人员的技术水平