登录Code Review 惊魂：同事的“优雅”重构，差点让管理员全部掉线
差点就酿成了“生产事故”
昨天下午，组里的实习生小李提交了一个 PR，说是把权限管理模块的代码“优化”了一下。
我扫了一眼 Diff，绿油油的一片，看起来确实清爽了不少。代码风格从原本的命令式变成了函数式，逻辑似乎也没啥大问题。手指悬在 "Approve" 按钮上，正准备点下去，突然看到一行对数组的处理，心里咯噔了一下。
赶紧把代码拉到本地跑了一遍——果然，原本正常的权限列表，重构后直接变成了空数组！
如果这行代码真的上线了，明天所有的管理员都会因为没有权限而被拦在后台门外，那我们组这个季度的绩效怕是要集体泡汤。
小李改动的地方很简单，他觉得原来的 push 写法太繁琐，想用 concat 让他变得“优雅”一点。
于是代码从：
// 老代码
defaultRoles.forEach(role => {
userPermissions.push(role);
});
变成了：
// 新代码：
userPermissions.concat(defaultRoles);
我把他叫到工位上，指着这行代码问他：“你觉得这两段代码等价吗？”
他一脸茫然：“不都是把数组拼起来吗？concat 不是更符合函数式编程习惯吗？”
看来，是时候聊聊 JS 数组方法里那些容易让人“阴沟里翻船”的返回值陷阱了。
还原一下“案发现场”
原始逻辑（正常工作）
我们的业务逻辑大概是这样的：从后端拉取用户的角色，然后把对应的权限塞到一个数组里。
// 伪代码：将嵌套的权限数组扁平化
// 以前是用 reduce + push 写的，虽然丑点，但能用
export const flatPermissions = allPermissions.reduce((acc, role) => {
// ... 省略中间处理逻辑
// 把处理好的权限 push 进累加器
acc.push(...processedPermissions);
return acc;
}, []);
这段代码跑了一年多，稳如老狗。
重构后的代码（引入 Bug）
小李为了追求代码整洁，在重构时把 push 换成了 concat：
export const flatPermissions = allPermissions.reduce((acc, role) => {
// ... 省略中间处理逻辑
// 他以为这行代码会把新权限加到 acc 里
acc.concat(processedPermissions);
return acc;
}, []);
看起来逻辑没变，对吧？
但在 JS 引擎眼里，这行代码的意思是：
- 拿出
acc数组。 - 创建一个新数组，内容是
acc+processedPermissions。 - 把这个新数组扔掉（因为没有接收返回值）。
- 返回原本的、没有任何变化的
acc。
结果就是：flatPermissions 永远是个空数组。
到底哪里出了问题？
我给小李画了张图，解释这俩方法的本质区别。
concat vs push：本质的区别
这俩方法的区别，不仅仅是写法不同，而是设计哲学完全不同。
1. concat：我只产生新东西，不碰旧东西
concat 是**非变异（Non-mutating）**方法。它不会修改调用它的数组，而是返回一个新的。
const arr1 = [1, 2];
const arr2 = [3, 4];
// 错误用法：以为 arr1 变了
arr1.concat(arr2);
console.log(arr1); // [1, 2] -> 根本没动！
// 正确用法：必须接收返回值
const result = arr1.concat(arr2);
console.log(result); // [1, 2, 3, 4]
2. push：我就改旧东西
push 是变异（Mutating）方法。它直接修改原数组，返回的是新数组的长度（这个返回值也经常坑人）。
const arr1 = [1, 2];
const arr2 = [3, 4];
// push 修改了 arr1
const length = arr1.push(...arr2);
console.log(arr1); // [1, 2, 3, 4] -> 变了！
console.log(length); // 4 -> 返回的是长度
内存和引用对比
为了加深印象，咱们看个图：
graph TD
subgraph Push操作
A[原数组 arr1] -->|push| A
A -.->|变大了| A
end
subgraph Concat操作
B[原数组 arr1] -->|concat| C[新数组 result]
B -.->|保持原样| B
end
- Push: 在原数组内存地址上扩容。
- Concat: 申请新内存，复制旧数据，复制新数据，返回新地址。
怎么修？这有三招
方案一：继续用 push，配合展开运算符
这是性能最好的改法，虽然看起来稍微没那么“函数式”。
export const flatPermissions = allPermissions.reduce((acc, role) => {
// 使用展开运算符 ... 把新数组打散塞进去
acc.push(...role.permissions);
return acc;
}, []);
优点：性能好，内存抖动少。
缺点：修改了 acc（但在 reduce 内部通常是可以接受的）。
方案二：正确使用 concat
如果你坚持要用 concat，记得把返回值接住。
export const flatPermissions = allPermissions.reduce((acc, role) => {
// 返回新的数组给下一次迭代
return acc.concat(role.permissions);
}, []);
优点：纯函数式，不修改原数组。 缺点：在循环中频繁创建新数组，可能带来额外的内存开销。
方案三：用 flatMap（推荐）
既然是“映射”+“扁平化”，JS 早就给我们准备好了专用 API。
export const flatPermissions = allPermissions.flatMap(role => role.permissions);
优点：代码最简洁，语义最清晰，专门干这事的。 缺点：老旧浏览器（如 IE）不支持，需要 Polyfill。
避坑指南：还有哪些方法是“只读”的？
concat 只是冰山一角。JS 的数组方法里，修改原数组和返回新数组的方法经常让人晕头转向。
我整理了一份清单，建议背下来，或者贴在电脑屏幕旁边：
⚠️ 会修改原数组（Mutating）
-
push()/pop() -
unshift()/shift() -
splice()(这个最容易混淆！) sort()reverse()fill()
✅ 只返回新数组（Non-Mutating）
concat()-
slice()(注意是 slice 不是 splice) -
map()/filter() reduce()-
toSorted()/toReversed()/toSpliced()(ES2023 新出的“安全版”方法)
写在最后
那天下午，我让小李把代码改了回去，并顺便给他科普了一波数组方法的副作用。
Code Review 的意义其实就在这里：不仅是找 Bug，更是团队技术栈的校准和经验的传承。
对于每一个开发者来说，写代码不能光图“看着顺眼”。在把 push 换成 concat 之前，先问自己两个问题：
- 我接收返回值了吗？
- 这代码是在循环里吗？
希望这次“险些发生”的生产事故，能给你的 Code Review 清单里增加一项检查点。
