4 年 React 经验 写出这样的业务代码，请问代码质量如何，有什么值得改进的地方吗
rookie2luochao · rookie-luochao · 174 天前 · 7835 次点击这是一个创建于 174 天前的主题，其中的信息可能已经有所发展或是发生改变。
github 仓库地址为：openapi-ui
求各位大佬 review 代码，并指导一二
76 条回复 • 2024-06-08 05:40:46 +08:00
 |
1
vczyh 174 天前
学习学习
|
 |
2
rookie2luochao OP @vczyh 后端都要疯狂卷前端了吗？给我们留口饭吃😭😭😭
|
 |
3
maocat 174 天前 via iPhone
学习学习，粗略看了下，内联样式比较多，可以用用 tw
|
|
4
rookie2luochao OP @maocat 没有用 tailwindcss 完全是个人爱好😭，本人以前一直就比较喜欢 cssinjs 这种接近内联样式的写法(因为写 css 写的不多)， 后面为了考虑剥离 cssinjs ，所以将其改成了内联样式写法
|
 |
5
gdtdpt 174 天前
学习学习，看了一下 http 的源码，刚好最近也遇到关于 antd 主题的问题，就是 axios 的拦截器定义是在 react 环境外的，所以在拦截器中直接使用 message 、notification 、modal 静态方法无法响应 antd 的全局主题配置，antd 官方也不建议直接使用此类静态方法。我看 demo 中切换为暗色主题后，当出现"Network Error"的 notification 依然是亮色主题，说明也存在这个问题。想问一下有没有什么好的解决方案，我是用了 vanillajs 的事件机制，不知道有没有其他方案想学习一下。
|
 |
6
jones2000 174 天前  1
数据请求为什么不放在 woker 里面， 这样后续做批量压测也好搞点， 放 UI 线程里面，又慢，还不能批量。
|
 |
7
enchilada2020 174 天前 via Android
@jones2000 能详细讲讲吗 或者给个参考资料啥的也行
|
|
9
rookie2luochao OP @gdtdpt 确实也遇到了这个问题，但是 message 、notification 、modal 静态方法不能切换主题，就弹一下，好像就暂时忽略了
|
|
10
vczyh 174 天前
@rookie2luochao 有个疑问，cssinjs 这种对缓存影响大吗？
|
|
11
rookie2luochao OP @jones2000 请问这个要怎么理解？数据请求放 worker 里面？
|
|
12
rookie2luochao OP @vczyh 不会，打包后，hash 都是算好固定了的，可以支持缓存的
|
 |
13
rencoo 174 天前
文件命名比较随意，有中划线格式的，也有驼峰的；用 useEffect deps 都是不对的，eslint 一打开全是报错；组件的状态和视图逻辑未分离，可以用 useReducer 抽一下。
总体感觉，像一坨大便，估计在大公司要被喷死 |
|
14
jones2000 174 天前
@rookie2luochao 多线程
|
 |
15
realJamespond 174 天前
不应该用 axios 的拦截器这样写所有实例都被拦截你和别人写的项目别人会疯掉，可以自己写个 hook 包一层，useTryCatch(request)，返回结果和 loading 状态，同时在自己的 hook 里面也能用 antd 通知组件
|
|
16
rookie2luochao OP @rencoo useEffect deps 确实是不对的，执行 package.json 里面的 lint 可以看到，需要手动修复，这里[有篇文章]( https://developers.pub/article/1020466)你也可以看下，eslint 一打开全是报错是因为我加了强行排序 props 的 warn, 但是我还没有来得及推 eslint --fix 后的代码。命名如何随意了，与你认知不一样而已？组件大写开头，文件夹、函数小写字母 + 中划线？非玻璃心，但是你的一坨大便的评价太随意。我并没有用任何模板，自己能加这么多 eslint 规则，你觉得我会不知道那些 eslint 警告和报错？
|
|
17
rencoo 174 天前
@rookie2luochao 你用了 useEffect 确不显式的声明依赖，会导致各种奇怪的 bug ；另外我说的是文件命名，有的是 xx-xx-xx ，有的是 XxxXxx 这种，就很乱？另外每个组件里都很多 useState 会让渲染组件失去可读性
|
|
18
rookie2luochao OP @realJamespond 不应该用不应该用 axios 的拦截器？可否标记到 github 上面的某一行代码？
|
|
19
rencoo 174 天前
你是找人 review ，所以给你指出问题，你不会以为自己代码写的很优雅吧？笑死
|
 |
20
qingshui33 174 天前
最近正好在看 React 来着，想咨询个问题，React 中比较流行的编写 CSS 的方式是啥啊？
我看 op 好像是行内样式居多，我个人在写的时候，用的 CSS Module ，但是遇到了这么个问题，就是有的组件，通常一个文件就可以解决，这个时候不想新建文件夹，因为文件夹下就只有这么一个文件，但是如果要把 CSS Module 文件放一起的话，就得新建个文件夹，否则组件多了，感觉就会很乱，大家都是怎么处理这种情况的？ 还有个问题就是，React + ts 的时候，大家都是怎么快速找到各种事件对象的类型？ |
|
21
rencoo 174 天前
@qingshui33 tailwindcss ，避免样式体积不断增大
|
|
22
rookie2luochao OP @rookie2luochao deps 要修复，这个建议接纳，还没来得及更新上去。文件命名我组件就是 XxxXxx ，文件夹是 xx-xx-xx ，入口、utils 、config 就是 xx ？那我用了很多 useState 要怎么改进才有可读性？
|
|
23
rookie2luochao OP 2
@rencoo 我感谢你的指出，但是没必要直接就说一坨大便吧，素养？
|
|
24
rookie2luochao OP @qingshui33 前几年流行 cssinjs, 最近流行 tailwindcss 这些预制 css ，但是 tailwindcss 不写熟练随时对着文档看，而且超难维护，个人感觉
|
|
25
rookie2luochao OP @qingshui33 尽量不要写行内样式，我是样式确实很少，所以写了点行内样式
|
 |
26
MMM25O7lf09iR4ic 174 天前
作为一个非资深后端程序员，我有一个疑问，为啥文件名一会大写一会小写开头？是有什么规范吗？
|
|
27
rookie2luochao OP 1
@erwsd32ew 文件名大写是前端领域里面组件的概念，组件建议大写命名，其他 utils 这些建议小写
|
|
28
rencoo 174 天前
@rookie2luochao 4 年经验写成这样，还有脸？
|
|
29
rookie2luochao OP 1
@rencoo 感谢你的建议，我不会在回复你了，感觉你的话语好像充斥着被社会毒打的不满，人其实心态也很重要
|
 |
30
chanChristin 174 天前
@rookie2luochao #27
可以把组件都放到 components 文件夹里，方便区分，你这组件和其他文件都写在同一层级里了。 路由写的有点麻烦了，每个文件夹里都有一个 route 文件，可以统一写到一个路由文件里，方便查找维护。 |
|
31
rookie2luochao OP @rencoo 我刚刚已经修复了大部分 eslint 警告，而且你也可以看下我上面发的那个关于 deps 的文章，大佬们对 deps 的见解也不一样
|
|
33
rookie2luochao OP @chanChristin 我是业务组件就和模块一起放的，纯组件放到了 components 里面，不知道你说的可以把组件都放到 components 文件夹里是说的业务组件吗，文件夹里都有一个 route 文件也是个人习惯，vue 里面一直都是一个 route 文件
|
 |
34
yohane3016 174 天前
整个项目有 0 个 useCallback 和 1 个 useMemo ，MainLayout 第一个 useEffect 就有问题，整个项目不必要触发的更新数不胜数。文件名大小写连字符无所谓，但是你得统一啊，那么大一个 Section 是看不见吗，还有单复数，要么都单数要么都复数，这种你随便找个规范遵循都行，别只遵循你自己的规范。
|
 |
35
laobobo 174 天前
不喜欢 内联样式
|
|
36
chanChristin 174 天前
@chanChristin 文件名看的我好难受啊，有大驼峰小驼峰还有-连接的。
我提个小建议，你所有的组件都用大驼峰的形式来写，src/components 里面的就写文件夹，比如 app-title ，就写个文件夹叫 AppTitle ，里面写 index.jsx ，如果需要修改样式就写 index.css ，引用的时候就直接引用 src/components/AppTitle 就可以了。 ``` useEffect(() => { valueRef.current = v; }); ``` 这个代码有问题，没有依赖，还有 readme 里面的 ``` const SetUpOpenApiUI = () => { useEffect(() => { import("openapi-ui-dist") }, []); ``` import 不需要用 useEffect |
|
37
chanChristin 174 天前
@rookie2luochao #33
业务组件和模块放一起没毛病，那也可以在业务组件旁边新建一个 components 文件夹，把组件都放进去。 每个文件夹里都有 route 不觉得很繁琐吗？新建一个组件就需要写一个路由文件，整合在一起方便查方便写。 |
 |
38
otakustay 174 天前
没看出什么特征，几乎没有抽象的感觉，一堆可以往外提的函数写在组件里面，代码大概率过不了 react-hooks 的 lint 规则。比较像 4 个月经验的人的代码
|
 |
39
XCFOX 174 天前
整个项目没有任何性能优化，根组件的 update 将导致整个页面的 update 。没有 memo 。只有两个 useMemo ，其中一个 useMemo 根本没必要，应该直接提升出组件外作为常量。
|
 |
40
qfdk 174 天前 via Android
看了下... 感觉就是个玩的项目. 可复用组件放在 components 里面并且需要大写开头. 没必要 每个都建立一个文件夹放一个 index.... 这个都不方便代码查找. 比如里面的 curl 是个什么鬼？ 第一眼就没看明白. 另外页面最好建一个文件夹叫做 pages 扔里面.....
非专业前端， 但是看了代码以及项目结构 跟我校大一新生差不多.... 建义找下最佳实践看看 |
 |
41
royzxq 174 天前
我捣糨糊以能 run 为目的写的代码都不会是这个样子.. 我指导不了你我的问题
|
 |
43
Ritr 174 天前
@qingshui33 tailwindcss 完美解决你的问题
|
|
44
qingshui33 174 天前
@rencoo #21
@Ritr 好的，我看看去 @rookie2luochao #24 cssinjs 之前也看过一点，可能是写 vue 顺手了，以至于刚开始写 React 这种的，怎么都不太舒服，我再看看 tailwindcss 去，如果实在不行，那也认了 😂 |
 |
45
himself65 174 天前
只看 React 部分的话，写法有点过时了。
从整个前端质量看的话，属于是没有什么复杂度的 CRUD project ，里面也没有多少比较深的前端技术 |
|
46
rookie2luochao OP @yohane3016 这个不是公司项目，所以个人风格比较多，Section 这个得改一下😭🥹
|
|
47
rookie2luochao OP @chanChristin 这个 useEffect 里面用 import ，是没有办法，必须要动态导入，当然还是设计问题，以前我们项目组没有人文件夹用大写字母开头
|
|
48
rookie2luochao OP @XCFOX 说中了精髓，这是个小项目，反正不卡，就没有关注性能优化，至于什么 useCallback 配合 react.memo 嫌复杂，没有做，虚心接受，晚点学习一下根组件的 update
|
 |
49
neverm0re 174 天前
hooks ，rerender 等感觉还没领悟到位。加油。
|
|
50
himself65 174 天前
> 并指导一二
这里面有很多可以深度挖掘的东西。 如果要深挖 React ，18 、19 出了这么多 API ，简单扫了一下你这里面大多数可以迁移到新的 API 优化一些（比如 useSyncExternalStore 、useTransition……），多读读官网 docs ，很多基本的 pitfall 全在你项目里面出现了一遍（ deps 、useEffect……） 状态管理你都没有做，点一下按钮状态都丢了，这些你考虑过吗…… 从工程化角度说，你这里面代码风格挺乱的，有时候 style 有时候 css in js ，文件命名……这下下来 eslint 、prettier 你有配吗 从前端角度说，这个项目目前是一个 CSR-only app ，你这个可以有 SSR 你可以尝试，React 出了 RSC 你可以尝试，Streaming UI 你可以做，怎么随便点一下整个屏幕都白了……另外 Vite 有很多自定义的 API 可以自己搭一个 server 玩玩…… 随便说几句 |
 |
51
janus77 174 天前 1
就跟楼上说的一样，“你是找人 review ，所以给你指出问题，你不会以为自己代码写的很优雅吧？笑死”
你搁这说你为什么那样做的理由，对我来说有什么意义吗？不是应该记下来赶紧去改吗？你把这个帖子当成辩论贴还是意见收集贴啊 |
|
52
rookie2luochao OP @himself65 感谢大佬回复解答，项目复杂度确实不高，2 线城市 crud 仔。很多基本的 pitfall 全在你项目里面出现了一遍(这个我去迁移一下)。按钮状态都丢了是指按钮的 loading 吗？ eslint 、prettier 都配置了的，写到 package.json 的字段里面。最近我去尝试下 ssr rsc Streaming UI 啥的。屏幕白了可能是因为最近 1-2 天我在过 eslint --fix ，过的不仔细，晚上回去全部改进一下命名等等风格，然后把错误调试好
|
|
53
rookie2luochao OP @janus77 已经改了，我这个说话属于情商不高，说话风格是啥子都要吧啦一下，也可以改进一下
|
 |
54
hahawode 174 天前
@rookie2luochao #23
 |
 |
55
bojackhorseman 174 天前
看了下，感觉跟我的风格还是有不少偏差的。
不过老实说就算一个人写也很难保持一致的风格，比如我过段时间就闲的改另一种写法🤣 |
 |
56
wang4012055 174 天前
给的意见就是,考虑下如何精简代码和代码可读,个人代码随便写就行.但想让人 review 至少自己觉得没有地方可以优化了才应该拿出来吧.
|
 |
57
yagamil 174 天前
老哥挺强的了，前后端通杀，里面还有 go
|
 |
58
h1104350235 174 天前
review 可以，就像团队 leader 一样也是这样子做，但你 review 别人代码的时候，一上来就讲是一坨？
|
|
59
chanChristin 174 天前
@rookie2luochao #47
那我没话说了，提一个建议说一直都这么写的，提一个建议说没人这么写，那还问啥啊，想咋写咋写吧。 |
|
60
rookie2luochao OP @chanChristin 你说的建议很中肯，比如多个 routes 、components 我都改了，但是对组件文件夹名字叫 AppTitle 我提出了疑问，你看下[antd-design 命名]( https://github.com/ant-design/ant-design/tree/master/components/auto-complete)？我们也属于交流是吧？
|
|
61
rookie2luochao OP @chanChristin 你说的建议很中肯，比如多个 routes 、components 我都改了，但是对组件文件夹名字叫 AppTitle 我提出了疑问，你看下[antd-design 命名？]( https://github.com/ant-design/ant-design/tree/master/components/auto-complete)
我们也属于交流是吧？ |
|
62
rookie2luochao OP @yagamil 凑合用一下😭😭，略懂一点，入门菜鸡
|
|
63
rookie2luochao OP @h1104350235 我也觉得大家都是人，说话不能太难听吧
|
|
64
rookie2luochao OP @hahawode 你发的图看不到？
|
 |
65
ColdBird 174 天前
像是放空大脑随便写写的，感觉没什么好 review 的。。。。。。。
|
 |
66
crocoBaby 174 天前
我完了,完全看不懂了
|
|
67
chanChristin 174 天前
@rookie2luochao #61
https://github.com/ant-design/ant-design-pro/tree/master/src/components antd pro 用的全都是大驼峰和文件夹的形式写的组件，而且他们的页面也是大驼峰的组件。 你想用哪种无所谓，关键是要统一格式，格式混乱看着难受，用着也难受。 |
 |
68
mumuwen 174 天前 1
1 、行内样式太多了，看起来 js 代码一坨（视图+逻辑+样式） 不够整洁。cssModule+less 。
2 、vite 配置下 alias ，引入路径太长了，很多 '../.../../'这种，配置 src 目录下的 @/ 3 、 @chanChristin 说的没错，纯组件放在 components 里面，业务组件和通用组件需要分开，路由写的确实不太好维护，每个文件都有个 route ，写在一个路由文件里，统一维护查找比较契合 react 的写法，不能因为之前 vue 这样写 react 也这么写。 4 、项目文件有点...，可以优化一下，src 目录下可以有一个 component/util/type.ts/route/config ，作为根目录下的通用模块，可以将一些业务文件里的抽离出来，比如函数、类型声明...，每个文件下都有这些 js 很杂乱。 ----- 这些都是偏基础和规范问题，可以优化，具体的项目我也没跑，只是简单看了看。 最直观的感觉就是...不是专攻前端？或者没进过中大厂？感觉没有在中大厂的项目中历练过，项目规范性、架构差很多... 这里面很多点如果在实际工作中，会被领导喷的 |
 |
69
ruoxie 174 天前
哪些是页面，哪些是组件，看起来有点乱
|
|
70
rookie2luochao OP @mumuwen 好的，谢谢建议，接下来抽时间改造一下，整的规范见解一点
|
|
71
rookie2luochao OP @bojackhorseman 我确实不太保持风格，或者就是别人口中说的，以自己为规范🥹😭😭
|
 |
72
K120 174 天前
https://github.com/xjh22222228/nav 和我一起学多一门 Angular ，吃多一口饭，嘿嘿
|
|
73
rookie2luochao OP @ruoxie 后面加个 pages 文件夹把页面搞进去
|
 |
74
chill777 174 天前
感觉一般吧。但是比我们公司的那个十年前端钉子户好太多了。
|
|
75
rookie2luochao OP @K120 可是国内的 angular 实在太少，出海又没有渠道😭
|
 |
76
tuomasi 173 天前
一小时能送几个外卖，一小时能抢几个网约单
|
Select Language