Replies: 7 comments 31 replies
-
|
(2024/03/24 更新しました) Skyshareの場合は次の手順でESLintを導入できます。
module.exports = {
env: {
browser: true,
es2021: true,
},
overrides: [
{
files: ["*.astro"],
parser: "astro-eslint-parser",
parserOptions: {
parser: "@typescript-eslint/parser",
extraFileExtensions: [".astro"],
},
processor: "astro/client-side-ts",
extends: [
"plugin:astro/recommended",
"plugin:astro/jsx-a11y-recommended",
],
},
{
files: ["*.{tsx, jsx}"],
extends: ["plugin:react/recommended", "plugin:react/jsx-runtime"],
},
{
files: [".eslintrc.{js,cjs}"],
env: {
node: true,
},
parserOptions: {
sourceType: "script",
},
extends: ["plugin:@typescript-eslint/disable-type-checked"],
},
{
files: ["tests/**/*"],
extends: ["plugin:@typescript-eslint/disable-type-checked"],
},
],
extends: [
"eslint:recommended",
"plugin:@typescript-eslint/recommended-type-checked",
"prettier",
],
parser: "@typescript-eslint/parser",
parserOptions: {
project: true,
ecmaVersion: "latest",
sourceType: "module",
},
plugins: ["@typescript-eslint", "react"],
settings: {
react: {
version: "detect",
},
},
rules: {
indent: ["warn", 4, { SwitchCase: 1, MemberExpression: "off" }],
quotes: ["warn", "double"],
semi: ["warn", "never"],
"linebreak-style": ["warn", "unix"],
"no-irregular-whitespace": "warn",
"prefer-const": "warn",
"@typescript-eslint/strict-boolean-expressions": "warn",
"@typescript-eslint/no-namespace": "warn",
"@typescript-eslint/no-unused-vars": [
"warn",
{ destructuredArrayIgnorePattern: "^_" },
],
},
ignorePatterns: ["dist", "node_modules"],
}Footnotes
|
Beta Was this translation helpful? Give feedback.
-
|
{
"name": "skyshare",
/*****/
"devDependencies": {
/*****/
"@typescript-eslint/eslint-plugin": "^7.1.0",
"@typescript-eslint/parser": "^7.1.0",
"eslint": "^8.57.0",
"eslint-config-prettier": "^9.1.0",
"eslint-plugin-astro": "^0.31.4",
"eslint-plugin-jsx-a11y": "^6.8.0",
"eslint-plugin-react": "^7.33.2",
"typescript-eslint": "^7.1.0"
}
} |
Beta Was this translation helpful? Give feedback.
-
参考 |
Beta Was this translation helpful? Give feedback.
-
|
Prettierもやや追加の設定が必要なので追記しておきますね
{
"name": "skyshare",
/*****/
"devDependencies": {
/*****/
"prettier": "^3.2.5",
"prettier-plugin-astro": "^0.13.0",
}
}
{
"plugins": ["prettier-plugin-astro"],
"overrides": [
{
"files": "*.astro",
"options": {
"parser": "astro"
}
}
],
"tabWidth": 4,
"useTabs": false,
"semi": false
} |
Beta Was this translation helpful? Give feedback.
-
参考 |
Beta Was this translation helpful? Give feedback.
-
|
既に出ていますが、静的なチェックはESLint husky+lint-stagedを導入すると、gitのpre-commitに割り込んで自動でフォーマットしてくれるため、 |
Beta Was this translation helpful? Give feedback.
-
|
@ZEKE320 @nkte8 わかりやすくPR出しておきました。 既にコミットされてるものには適用されないので、 モノレポになっているようなので、共通管理できるようルートディレクトリに また、このままモノレポにする場合、 |
Beta Was this translation helpful? Give feedback.
-
|
@takecchi マージについては一度コードオーナーのねこのさんから連絡を待ちましょうか |
Beta Was this translation helpful? Give feedback.
-
|
@takecchi @ZEKE320 うわー！いろいろありがとうございます！先こっち読むべきでした。 |
Beta Was this translation helpful? Give feedback.
-
|
@nkte8 Cloudflareの件、承知しました～！ 今Cloudflareで指定されてるビルドコマンドは |
Beta Was this translation helpful? Give feedback.
-
|
@takecchi ありがとうございます〜！現状は以下のような感じです。 モノレポやろうとした時の状況残ってたので共有します！（これより前の4コミットほどエラーで失敗しています） ビルドログです。多分コミットも残っているので参考にしてください！ |
Beta Was this translation helpful? Give feedback.
-
|
|
Beta Was this translation helpful? Give feedback.
-
|
@ZEKE320 ぼくの最初の書きかたがあまりよくなかったかな、とおもってるんですが、ぼくは Linter を導入したのにエラーとワーニングが残っている現状にちょっと困惑してるんですよね。修正を当面やらないというのであれば、ルールのほうを緩めてもいいんじゃないかとおもってかきこんでみたんですが、ルールにかんするおふたりのご意見は上のコメントでよくわかりました。 そういうことであれば、イシューをつくって対応するというのはいかがでしょうね。 上でぼくが引用した数字は @ZEKE320 さんのいうように一見して大げさではありますが、 まあ少ないといえない数なのかな、と。
というのがコメントを受けてのぼくの提案です。 |
Beta Was this translation helpful? Give feedback.
-
|
なるほど コード品質を一定にするという意味では、開発フローにリンティングを強制するのが最善だろうと、自分も考えてはいます。以前ディスカッションにあった通り、コミット時に検査する方法が良さそうですね ただ、この手のツールの導入時は変更箇所が広範になりやすく、コンフリクトが起こりがちなので またリンターの自動実行とは話題が変わりますが、警告数の課題として、ESLintがコードフォーマットの警告を大量に出してしまっている点も良くないですね というのも、Prettierをフォーマッタとして導入している以上、Prettierの実行を徹底するなら、あえてESLintで警告する必要がないと言えるためです @so-asanoさんの言う通り、エラー・警告が多いコードは見通しが良くないので Prettierの設計思想としても、リンターは脆弱性やコードの品質そのものを改善することに注力させるほうが良いということなので1、それに従うというのも悪くないなと 対応方針としては次のようなものがあると考えています
Footnotes |
Beta Was this translation helpful? Give feedback.
-
|
@ZEKE320 @so-asano 課題の整理と対応方針の議論、ありがとうございます！ 対応方針について、メリデメの整理もありがとうございます。 対応案1:
非常に大きなメリットだと思っています。レビューする側としてもかなり楽ができます。
これ、特にReactNodeの部分で痛感しています。タグをやたら変なところで切ろうとしますよね... 一方これってVS Codeの
これについてはあさのさんがおっしゃられていた以下のような対応を取る必要がありそうですね。
対応案2:
前記のようなコードをなくせるメリットがありますね。
こちらはあまり許容したくないですね... @nkte8 の考え総合的にみて とりあえず大きな範囲でのIssue（個別ファイルについてはtask）を立ててみようと思います。 |
Beta Was this translation helpful? Give feedback.
-
|
@ZEKE320 すみません、どこかのタイミングで本方針に伴ったESLintのルールを調節していただきたいです。 |
Beta Was this translation helpful? Give feedback.
-
|
Linterの設定について、追加したい項目（要望）をコメントに記載しました。 |
Beta Was this translation helpful? Give feedback.
-
|
@ZEKE320 @so-asano 本Disscussion対応として、マイルストーンを設定しました。対応実施の目安としてよければ活用してください。 |
Beta Was this translation helpful? Give feedback.
-
|
リンティングエラーの対応方針はここでまとめましょうか （解決）
|
Beta Was this translation helpful? Give feedback.
-
|
@ZEKE320 ありがとうございます！ バリデーションチェックについて、この辺( https://zenn.dev/uzimaru0000/articles/json-type-validation ) がヒットしていたのですが、ライブラリを使うのであればできるだけメジャーな製品を選ぶべきで、そもそもあまりライブラリを使うのが一般的ではない・自前実装ありき、というのであれば自前実装でしょうか。ご意見伺いたいです。 |
Beta Was this translation helpful? Give feedback.
-
|
@nkte8 ただ 例えば次のような実装ができるそうです import { z } from 'zod';
// Data型とError型のZodスキーマを定義
const DataSchema = z.object({
id: z.number(),
code: z.string(),
});
const ErrorSchema = z.object({
code: z.string(),
message: z.string(),
});
// ZodオブジェクトからTypeScriptの型定義を取得することも可能
type Data = z.infer<typeof DataSchema>;
// APIレスポンスの型を定義
const ApiResponse = z.union([DataSchema, ErrorSchema]);
// レスポンスを処理する関数
const result = ApiResponse.safeParse(response);
if (!result.success) {
throw new Error('Invalid API response');
}
if (DataSchema.check(result.data)) {
console.log('Data received:', result.data);
} else if (ErrorSchema.check(result.data)) {
console.error('Error received:', result.data);
throw new Error(result.data.message);
} else {
throw new Error('Unknown response type');
}一度ローカルで試してから、後々PRで共有しますね |
Beta Was this translation helpful? Give feedback.
-
|
ありがとうございます！そしたら(PR見てからの前提ではありますが、基本的には) |
Beta Was this translation helpful? Give feedback.
-
|
@nkte8 |
Beta Was this translation helpful? Give feedback.
-
|
@ZEKE320 確認ありがとうございます！その認識でOKです！ |
Beta Was this translation helpful? Give feedback.
-
（解決）
|
Beta Was this translation helpful? Give feedback.
-
|
@ZEKE320 ありがとうございます！
もし発生箇所が |
Beta Was this translation helpful? Give feedback.
-
|
@nkte8 |
Beta Was this translation helpful? Give feedback.
-
（解決）union型における
|
Beta Was this translation helpful? Give feedback.
-
|
@ZEKE320 ありがとうございます！ 先ほど確認したのですが、 以上より、"blob"型のみ残す対応で大丈夫です。 |
Beta Was this translation helpful? Give feedback.
-
|
@nkte8 |
Beta Was this translation helpful? Give feedback.
-
LinterとFormatter導入の提案
Linterの導入
用途
コードを静的解析し、
などをマークします。
利点
具体的なLinter
ESLint
npmでパッケージをインストールしてESLint自体の拡張(AstroやReactの対応)ができる.eslintrc.jsonから設定可能SonarLint
@ext:sonarsource.sonarlint-vscode java homeと入力Sonarlint > Ls: Java HomeにインストールしたJDKのパスを指定する(画像では
/usr/local/sdkman/candidates/java/...から始まる箇所)Microsoft Edge Tools for VS Code (VS Codeの拡張機能)
Code Spell Checker (VS Codeの拡張機能)
cspell.jsonから設定可能Formatterの導入
用途
コードを自動で整形します。
利点
具体的なFormatter
Prettier
npmでパッケージをインストールしてPrettier自体の拡張(本プロジェクトでは特にAstroへの対応)ができる.prettierrcから設定可能Beta Was this translation helpful? Give feedback.
All reactions