[Chore] Run ts-migrate on core package #286
Conversation
| "no-unused-vars": "off" | ||
| "@typescript-eslint/no-unused-vars": ["error", { "ignoreRestSiblings": true }] |
There was a problem hiding this comment.
這個是避免 import type 的時候造成 no-unused-vars error:
https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-unused-vars.md
ignoreRestSiblings 是為了讓 const { a, b, ...other } = props 不會出錯，有蠻多地方我們會這樣寫來濾掉不要的 props。
| # Temp turn off following rules because we're doing ts-migrate | ||
| # Will turn them on after migration completed. | ||
| "max-len": "warn" | ||
| "no-use-before-define": "warn" |
There was a problem hiding this comment.
這就是在 ts-migrate 後暫時加入的 rule，理由是 ts-migrate 會加入太長的 @ts-expect-error comment，並且有產生一些會提前使用 type 的 code(違反 no-use-before-define)。
There was a problem hiding this comment.
稍微補充說明一下 @ts-expect-error 的用途：
The 3.9 release of TypeScript introduced @ts-expect-error comments. When a line is prefixed with a @ts-expect-error comment, TypeScript will suppress that error from being reported. If there’s no error, TypeScript will report that @ts-expect-error wasn’t necessary. In the Airbnb codebase we switched to using the @ts-expect-error comment instead of @ts-ignore.
簡單講標上 @ts-expect-error typescript 不會報錯，而且如果你修掉了，他還會提醒你要拿掉 @ts-expect-error。
| # It's fix after upgrading eslint-plugin-react to v7.20.6 | ||
| # to keep consistency with rule config in eslint-config-ichef. | ||
| # just add `static-variables` in the first line. | ||
| # Should remove after we upgrade eslint-plugin-react in eslint-config-ichef. | ||
| 'react/sort-comp': ['error', { |
There was a problem hiding this comment.
這個是升了 eslint-plugin-react 到 7.20 後，有部分的 component 出現 react/sort-comp 的 error。
原因似乎是多了 static-variables 的 group，就會使一些本來不會出錯的地方出錯。
這裡把 eslint-config-ichef 的設定抄過來，加入 static-variables 來解決。
見 jsx-eslint/eslint-plugin-react#2408
There was a problem hiding this comment.
這個加在 eslint-config-ichef 中會不會比較好？因為在後台、online restaurant 都有用到 eslint-plugin-react，我們在共用的 eslint-config-ichef 修掉這個問題，就不用三個專案都寫類似的 rule 。
There was a problem hiding this comment.
應該是要這樣，我想說暫時先讓它 pass，最後再來收尾
目前是想先盡快把 ts 加上 typing
There was a problem hiding this comment.
感謝班尼建議，有另外開個 task 了
| @@ -7,6 +7,8 @@ const defaultConfigs = require('../../../configs/webpack.dist'); | |||
| const packageDirname = process.cwd(); | |||
|
|
|||
| module.exports = webpackMerge(defaultConfigs, { | |||
| entry: './src/index.ts', | |||
There was a problem hiding this comment.
對，因為現在是 index.ts 了。
| // eslint-disable-next-line react/prop-types | ||
| ineditable, // unwanted prop |
There was a problem hiding this comment.
這裡在升 eslint-plugin-react 也噴錯，但本意是拿掉不要的 props，想說先暫時 ignore 了
| # It's fix after upgrading eslint-plugin-react to v7.20.6 | ||
| # to keep consistency with rule config in eslint-config-ichef. | ||
| # just add `static-variables` in the first line. | ||
| # Should remove after we upgrade eslint-plugin-react in eslint-config-ichef. | ||
| 'react/sort-comp': ['error', { |
|
Benny 前面提到，轉換出來的 type 無法判斷對錯，我個人也是這樣覺得
|
|
決定不轉 ts，先 close 啦 |
Purpose
接下來會開始 migrate
corepackage 裡的 typescript。原則上步驟如下：@ts-expect-error的 comment，加上 typing。這支 PR 直接在 core package 上跑 ts-migrate 轉換所有的 js(除了 icon component / test)，並且修正一些 lint error。
ts-migrate這是 airbnb 製作的工具，目的是節省 ts migration 的時間。
我只有在
packages底下跑npx ts-migrate rename core和npx ts-migrate migrate core兩個指令去轉換core裡面的程式碼(有先把src/icons列入 exclude)這會自動把 js 轉換成 ts，並且盡它所能的加上 type。對於沒辦法解的 error，會加上
@ts-expect-errorcomment 來避免錯誤，之後我們再自己手解。Review Tips
src/底下的東西不用看，因為這邊是ts-migrate轉過的 code，一些 coding style 可能有被改到、type 也有一堆 any，我想在接下來的 PR 加 types 的時候順道修正。eslint-plugin-react，原因是把全部的 component 變成 tsx 後，eslint 因為 這個 issue 跑不起來。造成的影響可以見下面的留言。Changes
Risk
Usually none, if you have any please write it here.
TODOs