前端Code Review中的常見問題解析(一)

語言: CN / TW / HK

theme: smartblue

此文是在團隊內做的分享,期望通過列舉並解析code review中的常見問題,提升團隊的程式碼規範水平。

前言

在我們日常工作中,從功能開發,到提交程式碼稽核,是一個比較普遍的流程,不過再往後程式碼有沒有review出問題、 需不需要修改,往往只有有限的兩三個稽核參與人知道。

為了幫助大家更好地理解規範,提升程式碼質量,這裡收集了我們前端code review中的一些常見問題,並做一些解析。

程式碼衝突頻繁、甚至把未測試通過的功能釋出到了線上

主要原因是沒有一個類似git-flow的合理git工作流,具體表現上為多人共用了同一分支開發、或者同一分支提測, 在釋出時沒有把測試不通過的功能分離出來,因此造成了衝突頻繁、未測試通過功能釋出上線這些問題。

以下是某專案組經歷了一些程式碼提交的痛,而去強調的要求,應以此為鑑。

image.png

目前在專案中推薦的git工作流:

```text 分支規則 命名格式:以/(斜線)隔離欄位,-(橫線)連線字元,例如:feature/goods-modules/wjh

master:生產主分支,需保持穩定性,一般由釋出分支合併進來,不直接修改程式碼 release/production:釋出分支 release/test:預釋出環境(UAT)提測分支 test:測試環境提測分支 feature:新功能分支,規則:feature/功能名/人名,例如:feature/goods-modules/wjh(從master分支拉出來) hotfix:線上bug修復分支,規則:hotfix/bug名/人名(從master分支拉出來)

工作流 開發完成:從feature 或 hotfix合併至test提測 測試通過:  從feature 或 hotfix合併至release/test進行預釋出環境提測 預釋出測試通過:從feature 或 hotfix合併至release/production釋出,當本次釋出確認穩定後,合入master分支

注意:release/test、test分支作為公共提測分支,永遠只進不出,否則可能將測試中未通過的程式碼釋出到線上! ```

如果是新專案從標品建新分支開發,請按照規範的格式(專案名/分支名),比如dp/master,有利於統一規範,以及方便配置gerrit稽核許可權。

值得注意的是,區別於傳統的git-flow,該git工作流追求最大限度的新程式碼隔離,以避免多個功能提測時,部分功能未通過卻不小心釋出到了線上。

儘管如此,即使制定了合理的git工作流,我們仍不能完全杜絕問題,比如有人把test分支合到了自己的分支,比如在code review上沒有嚴格把關,這些問題需要我們更多地去貫徹規範,避免生產事故。

Commit Message模糊、不知所云

image.png 上面列舉了一些公司裡的錯誤示例,提交描述不清晰,會直接導致我們無法理解這個commit做了什麼事情,以後出了問題,也會增加程式碼審查的成本,目前推薦的commit格式為:

bash git commit -m "type(scope): subject"

公司裡某專案已使用commitlint做了提交規範,可參考根目錄的.commitlintrc.js檔案: ```js /* * commitlint * 所有專案通過此模板統一配置,如有變動也統一更新,不在專案中單獨修改 * * 格式: * git commit -m "type(scope): subject" * * type-enum: * build 編譯相關的修改,例如釋出版本、對專案構建或者依賴的改動 * chore 其他修改, 比如改變構建流程、或者增加依賴庫、工具等 * ci 持續整合修改 * docs 文件修改 * feat 新特性、新功能 * fix 修復bug * perf 優化相關,比如提升效能、體驗 * refactor 程式碼重構 * revert 回滾到上一個版本 * style 程式碼格式修改, 比如空格、符號,不是css修改 * test 測試用例修改 * * 使用示例: * git commit -m "feat(商品管理): 新增了虛擬商品功能" * git commit -m "fix(埋點): 修復了使用者註冊的上報欄位錯誤" * * 詳細配置請參考: https://github.com/conventional-changelog/commitlint/blob/master/@commitlint/config-conventional/index.js /

module.exports = { extends: ['@commitlint/config-conventional'], rules: { 'scope-empty': [2, 'never'], }, }; ```

主要的原則是:描述清楚我們的提交是什麼性質的,在哪裡做了什麼事情。

公共常量、方法、正則等未抽離出頁面

image.png

image.png

截圖3.png

能夠複用的常量、方法、正則、元件等應該抽離到公共的地方統一維護,否則會生產以下一些問題:

  1. 程式碼冗餘,經常出現重複的程式碼;
  2. 不利於後續修改,比如後續改了一處的常量、正則,又忘了改另一處的,導致bug的出現;
  3. 產生魔法數字,別人接手後,無法理解數字的意圖,甚至連程式碼作者本人都忘了,只能猜。

針對上面三個圖裡的問題:

常量

  1. 僅在當前頁面使用的常量,可在當前頁面定義後再使用,必要時增加註釋;
  2. 公共常量請統一在src/constants定義後使用,如果是一組常量,按照列舉的形式來定義,例如:

```js // 營銷型別 export const MARKETING_MODE = { SMS: 1, MMS: 2, };

export const MARKETING_MODE_LABELS = {

};

export const MARKETING_MODE_OPTIONS = Object.values(MARKETING_MODE).map(val => ({ value: val, label: MARKETING_MODE_LABELS[val], })); ```

第一組可用於型別判斷,第二組可用於中文欄位回顯,第三組可用於下拉元件選擇。

命名格式上遵循全大寫加下劃線格式,如果是提供給第三方元件使用的,可按照第三方元件的格式,例如上面的第三組是提供給element ui使用的,label、value可以不用大寫。

方法、正則、元件等

公共工具方法請放到src/utils裡面,比如上面計算導航欄高度的方法放到src/utils/screen裡面,正則放到src/utils/regexps裡面,公共元件放到src/components裡面。

如果某些情況下拿不定主意,比如涉及到原有公共模組的修改,可諮詢原來的程式碼作者、前端組長,我們有必要為了程式碼的高質量而進一步探索,而不是優先考慮同時維護幾份類似的程式碼。

檔案、元件等命名格式不規範

截圖 1.png

截圖2 1.png

市面上常用的命名規範:

  • camelCase(小駝峰式命名法 ——首字母小寫)
  • PascalCase(大駝峰式命名法 ——首字母大寫)
  • kebab-case(短橫線連線式)
  • snake_case(下劃線連線式)

對於目錄名、檔案命名(包括HTML、CSS、JS、圖片等等)遵循短橫線連線(kebab-case)。

對於JSX元件,遵循大駝峰式命名法(PascalCase),包括在其他元件中使用時。

js import Bar from './Bar'; // do something <Bar></Bar>

對於template元件,遵循PascalCase或者kebab-case命名(但在專案中需要統一)。

對於HTML屬性,遵循kebab-case命名,class也可遵循BEM規範命名。

在命名規範上其實還有很多的細則,這裡只列舉一部分常見問題,後續會統一整理更詳細的命名規範。

變數、方法名等命名含義不準確

截圖 2.png

截圖2 2.png

圖一里面如果我們遮蔽“播放”兩個字,應該沒人能夠看出來eventVideo方法是用來做什麼的,這裡可以修改為playVideo,再按照我們事件處理加handle命名的慣例,正確的命名應該為handlePlayVideo。

圖二的本意是要呼叫以下的人民幣分和元互轉方法

js /** * 頁面和服務端,匯率為100的 互相轉換 * @param money * @param isFromServer */ convertHundredMoneyUnit(money, isFromServer) { if (isFromServer) { return !!money ? money / 100 : 0; } return !!money ? floatNum.floatMul(money, 100) : 0; }

其實這上面methods→moneyUtil不止有三個問題,就連這個方法本身的存在必要性、命名都是有問題的,不過這裡主要講下引數命名也需要定義清晰,能使人一目瞭然。

這裡面val、whether可以統一為money、isFromServer,但isFromServer仍然不是最優解,需要額外的去聯想到後端返回來的資料是分,才能準確瞭解方法的意圖,所以應該改為isCent(是否為分)。

總的說來,對於命名規範,我們應該首要遵循“程式碼即註釋”。

使用了寬鬆的比較(==)而不是嚴格比較(===)

截圖 3.png 有些前端同學養成了使用==寬鬆比較的習慣,有時候在資料型別不統一的情況下確實是個好辦法,比如前端有一個id = 5,而後端返回了一個id = '5',

此時5 == '5'是成立的,然而也是因為這一隱式轉換的特性,會埋下bug的隱患,比如:

```js let num = 0; let str = "0"; let obj = new String("0");

console.log(num == obj); // true console.log(num == str); // true console.log(obj == str); // true console.log(null == undefined); // true console.log('' == false); // true console.log(0 == false); // true ```

隱式轉換造成的諸多可能性,會帶來一些不容易發現的bug,所以推薦都使用嚴格比較,如果有型別不一致的情況,可以顯式轉換,比如id === Number(id)。

程式碼巢狀過深,閱讀困難

Untitled 1.png 上圖的程式碼存在層層巢狀的現象,而好的程式碼應該是儘量平級的,這樣會有利於閱讀,並且減少潛在bug的可能性。

這裡面有些巢狀是不必要的,比如validate、API請求可以使用async/await,if可以使用“衛語句”,即if(!valid) return false,從而減少程式碼的巢狀。

更復雜的情況,我們可以考慮函式拆分、策略模式等,一個核心的思想是開放封閉原則——對擴充套件是開放的,而對修改是封閉的。優先通過擴充套件實現,而不是通過修改來實現。

樣式未加scoped、巢狀層級過深等

Untitled 2.png

Untitled 3.png

Untitled 4.png

  1. PC端寫樣式時需要加上scoped;
  2. CSS class巢狀不超過三級;
  3. 修改element ui樣式時,需要考慮統一性、可維護性,避免出現視覺不統一,以及後續有換膚等需求時,便於調整。