Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Improve code]: Linterの設定修正 #74

Merged
merged 5 commits into from
Apr 4, 2024
Merged

[Improve code]: Linterの設定修正 #74

merged 5 commits into from
Apr 4, 2024

Conversation

ZEKE320
Copy link
Collaborator

@ZEKE320 ZEKE320 commented Apr 2, 2024

概要

このPRでは次の変更を行っています。

  1. コーディングスタイルに関するルールを削除
  2. 未使用の変数に関するルールを改善
  3. JSXの属性に対するPromise型チェックのルールを無効化

修正後のリンティングエラー例

_(アンダースコア)から始まる変数の場合

$("meta").each((_, element) => { ... })
$ npx astro check
...
15:00:11 [check] Getting diagnostics for Astro files in /workspaces/skyshare/astro...
Result (104 files): 
- 0 errors
- 0 warnings
- 0 hints

$ npx eslint src/pages/api/getOgpMeta.ts (出力なし)

任意の変数の場合

$("meta").each((unusedVar, element) => { ... })
$ npx astro check
...
15:01:16 [check] Getting diagnostics for Astro files in /workspaces/skyshare/astro...
src/pages/api/getOgpMeta.ts:143:21 - warning ts(6133): 'unusedVar' is declared but its value is never read.

143     $("meta").each((unusedVar, element) => { ... })
                        ~~~~~~~~~

Result (104 files): 
- 0 errors
- 0 warnings
- 1 hint

$ npx eslint src/pages/api/getOgpMeta.ts 

/workspaces/skyshare/astro/src/pages/api/getOgpMeta.ts
  143:21  warning  'unusedVar' is defined but never used. Allowed unused args must match /^_/u  @typescript-eslint/no-unused-vars

1 problem (0 errors, 1 warning)

別モジュールからインポートした変数の場合

import { corsAllowOrigin, unusedImport} from "@/lib/vars"
$ npx astro check
...
src/pages/api/getOgpMeta.ts:3:27 - warning ts(6133): 'unusedImport' is declared but its value is never read.

3 import { corsAllowOrigin, unusedImport} from "@/lib/vars"
                            ~~~~~~~~~~~~

Result (104 files): 
- 0 errors
- 0 warnings
- 1 hint

$ npx eslint src/pages/api/getOgpMeta.ts 

/workspaces/skyshare/astro/src/pages/api/getOgpMeta.ts
  3:27  warning  'unusedImport' is defined but never used. Allowed unused vars must match /^_/u  @typescript-eslint/no-unused-vars

✖ 1 problem (0 errors, 1 warning)

Resolves #64

@ZEKE320 ZEKE320 self-assigned this Apr 2, 2024
@ZEKE320 ZEKE320 linked an issue Apr 2, 2024 that may be closed by this pull request
@ZEKE320
Copy link
Collaborator Author

ZEKE320 commented Apr 2, 2024

@nkte8

@typescript-eslint/no-misused-promises
この設定も上記の推奨構成に含まれるうちの一つです。公式ドキュメントを覗いた限りでは、Voidを返す関数を引数とする箇所のみで、このルールを無効化することも出来るようですね[2]> (https://github.com/nkte8/skyshare/discussions/38#user-content-fn-2-5a030d1f7af0e7a171f79916c700b548)。

Discussion #381 でも話題に挙がりましたが、

  • onClick等で利用される関数のPromise型戻り値を許容する設定

も、こちらで解決しましょうか?

Footnotes

  1. https://github.com/nkte8/skyshare/discussions/38#discussioncomment-8927127

@nkte8
Copy link
Owner

nkte8 commented Apr 2, 2024

@ZEKE320

Discussion #38 でも話題に挙がりましたが、

  • onClick等で利用される関数のPromise型戻り値を許容する設定

も、こちらで解決しましょうか?

可能そうであればぜひ対応していただきたいです...!

Copy link

cloudflare-workers-and-pages bot commented Apr 3, 2024

Deploying skyshare with  Cloudflare Pages  Cloudflare Pages

Latest commit: 15a34ed
Status: ✅  Deploy successful!
Preview URL: https://6d94906f.skyshare.pages.dev
Branch Preview URL: https://preview-issue-64.skyshare.pages.dev

View logs

@ZEKE320 ZEKE320 marked this pull request as ready for review April 3, 2024 12:44
@ZEKE320 ZEKE320 requested a review from nkte8 April 3, 2024 12:44
@ZEKE320
Copy link
Collaborator Author

ZEKE320 commented Apr 3, 2024

@nkte8
対応完了しました
レビューお願いします!

ちなみに、JSXのPromise型チェックは次のような挙動になっています

  • 変更前
    スクリーンショット 2024-04-03 213646

  • 変更後
    スクリーンショット 2024-04-03 213737

公式ドキュメントにてJSXのチェックに関する記述1が見つかったので、そちらをそのまま適用しています

Footnotes

  1. https://typescript-eslint.io/rules/no-misused-promises/#:~:text=attributes%3A%20Disables%20checking%20an%20asynchronous%20function%20passed%20as%20a%20JSX%20attribute%20expected%20to%20be%20a%20function%20that%20returns%20void

Copy link
Owner

@nkte8 nkte8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

リクエスト本文と追加コメント分の2点、動作確認しました!対応ありがとうございました

@nkte8 nkte8 merged commit 71e8dd2 into develop Apr 4, 2024
2 checks passed
@nkte8 nkte8 deleted the preview/issue_64 branch April 4, 2024 04:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Improve code]: Linterの設定修正
2 participants