[Improve code]: lib/* 手動formatter・Linter対応 #84

ZEKE320 · 2024-04-07T14:00:04Z

Resolves #58

本PRで行われた変更

再代入のない変数にconst宣言を追加 (6c056c3)
Astroの環境変数を文字列とみなす記述を追加 (a7a4af9)
Nullish判定を明記 (08cb5fe)
コメント内の空白形式を変更 (a33a328)
変数の型情報を明記 (e8bc360)
Prettierによるコードの整形 (*.tsx, *.astroの変更なし)
ファイル名getIdの綴りを修正 (e5835ec)
Astro API用の型をZodで再定義 (c0b67f9)
Astro APIのレスポンス取得箇所にZodのバリデーションチェックを実装 (700b531, 3e1565a)
pagedbAPIのレスポンスに用いられる型名が、モジュール間で重複しないよう変更 (bad010e)
APIレスポンスの型定義厳密化に伴い、影響箇所を修正 (3e1565a)
getOgpMetaエラー時のレスポンス処理で不足していたstatusを追加 (dcaed42)

…ssue_58

ZEKE320 · 2024-04-07T14:16:06Z

ひとまずESLintのエラーを解消し終わりました
レビューをお願いします！

nkte8

対応ありがとうございます！確認・必要に応じて対応お願いします。

エラーの出し方（エラーハンドリングの際に返却するe.nameとe.descriptionの値）については明確にどうするかをまだ決めていなかったので、いったんはこのままにしておきます。

nkte8 · 2024-04-08T11:14:50Z

astro/src/lib/pagedbAPI/createPage.ts

+const PageCreationOutputZod = z.object({
+    uri: z.string(),
+})


Zodの対応ありがとうございます！ #79 ではZodは冠詞的にくっつけていたのが、こちらでは接尾についているようです。こちらは #79 とは違った意図がある感じでしょうか？
（理由がないのであればどちらかに統一した方がいいのかなと @nkte8 は思っている感じです。）

気づいていませんでした... Zod接頭辞 + [型名]で統一しておきますね

nkte8 · 2024-04-08T11:20:52Z

astro/src/lib/api/types.ts

全体のZodオブジェクトについて、#79 ではZodは冠詞的にくっつけていたのが、こちらでは接尾についているようです。こちらは #79 とは違った意図がある感じでしょうか？
（理由がないのであればどちらかに統一した方がいいのかなと @nkte8 は思っている感じです。）

[Improve code]: lib/* 手動formatter・Linter対応 #84 (comment)
上記同様対応しました。

nkte8 · 2024-04-08T12:20:00Z

astro/src/pages/api/getOgpMeta.ts

+        const errorObject: errorResponse & { html: string } = {
+            type: "error",
+            error: name,
+            message: msg,
+            status: 500,
+            html: responseHTML,


一時期テストに使っていたコードが残ってしまっていたみたいです...
html: stringの戻り値を消していただきたいです。作業範囲的に対象外なので恐縮ですが、よろしくお願いいたします。

対応しました！

nkte8 · 2024-04-08T12:25:24Z

astro/src/lib/pagedbAPI/deletePage.ts

#79 ではZodは冠詞的にくっつけていたのが、こちらでは接尾についているようです。こちらは #79 とは違った意図がある感じでしょうか？
（理由がないのであればどちらかに統一した方がいいのかなと @nkte8 は思っている感じです。）

[Improve code]: lib/* 手動formatter・Linter対応 #84 (comment)
上記同様対応しました。

nkte8 · 2024-04-08T12:27:41Z

astro/src/lib/pagedbAPI/getIds.ts

リネーム対応ありがとうございます！気づきませんでした...

#79 ではZodは冠詞的にくっつけていたのが、こちらでは接尾についているようです。こちらは #79 とは違った意図がある感じでしょうか？
（理由がないのであればどちらかに統一した方がいいのかなと @nkte8 は思っている感じです。）

[Improve code]: lib/* 手動formatter・Linter対応 #84 (comment)
上記同様対応しました。

nkte8 · 2024-04-08T12:29:12Z

astro/src/lib/pagedbAPI/getPage.ts

#79 ではZodは冠詞的にくっつけていたのが、こちらでは接尾についているようです。こちらは #79 とは違った意図がある感じでしょうか？
（理由がないのであればどちらかに統一した方がいいのかなと @nkte8 は思っている感じです。）

[Improve code]: lib/* 手動formatter・Linter対応 #84 (comment)
上記同様対応しました。

nkte8 · 2024-04-08T12:30:55Z

astro/src/lib/pagedbAPI/models/db.json

getPage.tsの方に埋め込む形になったと思うので、消してしまいましょうか。

対応しました！

nkte8 · 2024-04-08T12:38:21Z

astro/src/pages/posts/[slug].astro


-if (typeof getpage_res?.error !== "undefined" && getpage_res.error !== "") {
+if ("error" in getPageResult && typeof getPageResult?.error !== "undefined" && getPageResult.error !== "") {


作業範囲的に対象外の修正なので恐縮です。getPageAPIはskyshare定義のAPIなので、戻り型の検証は型特定部分だけで良いと思いました。getPage.tsを見る限り、エラーの場合は必ずerrorとmessageを持つレスポンスを行うためです。
なので条件式は("error" in getPageResult)でもいいかもと思っています、ご意見伺いたいです。

（(typeof getpage_res?.error !== "undefined" && getpage_res.error !== "")という回りくどい書き方をしていたのは、原本を作成した @nkte8 がin演算子を知らなかったためです...）

了解です！
対応しました

…ssue_58

cloudflare-workers-and-pages · 2024-04-10T13:07:37Z

Deploying skyshare with Cloudflare Pages

Latest commit:	`62e71ac`
Status:	✅ Deploy successful!
Preview URL:	https://f77f46cd.skyshare.pages.dev
Branch Preview URL:	https://preview-issue-58.skyshare.pages.dev

View logs

ZEKE320 · 2024-04-10T13:12:16Z

967b327 Zodオブジェクト命名規則の統一
4fd4e58 getOgpMetaのエラーレスポンスからhtmlを削除
ce937a7 使用されていないレスポンス型定義用のJSONを削除
0fc4c2d pages/post/[slug].astroエラー型判定の実装を改善

上記4点対応済みです。確認をお願いします！

ZEKE320 · 2024-04-10T14:45:53Z

@nkte8
今のタイミングで、エラーの型判定箇所を統一しておきたいですね

[Improve code]: astro/src/components/Client/bsky/* 手動formatter・Linter対応 #83 (comment)

こちらで一度、方針を伝えていただけるとありがたいです！

nkte8 · 2024-04-10T16:30:39Z

@ZEKE320

エラーの型判定箇所を統一しておきたいですね

上について、#83 (comment) での議論通り、error in valueでの判定に揃えていただいてOKです。以下コメントです

getOgpMeta関数については、ATProtoでのエラー時のレスポンスを同じような形式、error,messageで返すようにしているので、これと同じ方針を取っていただいて大丈夫です！
- 現在はtypeによりユニオン型として処理可能なようにしていますが、この方法でパラメータを外せそうですね。これは別途Issueを上げようと思っていますが、もし可能そうであれば今対処してしまってもいいと思います。
getOgpBlobについても、現在はレスポンス自体発生しない作りになってしまっていますが、error in valueの判定によって、クライアント側でエラーハンドリングができそうな気がしますね
- 上と同じ題目でIssue上げようと思います。getOgpMeta同様に、もし可能そうであれば今対処してしまってもいいと思います。
- いずれも作業範囲が広がりすぎるような感じでしたら別Issue対応でいいかなと思っています。

ZEKE320 · 2024-04-11T15:38:11Z

@nkte8

[Improve code]: lib/* 手動formatter・Linter対応 #84 (comment)

getOgpMeta関数については、ATProtoでのエラー時のレスポンスを同じような形式、error,messageで返すようにしているので、これと同じ方針を取っていただいて大丈夫です！

こちらの件ですが、あまり良い実装例が思い浮かばず...すみません。
一度別のIssueでお願いしたいと思います。

@nkte8
また上記とは別で、実装方針を伺いたい点があり

// PageDeleteButton.tsx(32行目)

if (!("error" in resDeletePage) &&
    resDeletePage.result === "ok") {
  /* ... */
} else {
  let e: Error = new Error((resDeletePage as typeof etype).message)
  /* ... */
}

ここで条件式をif (!("error" in resDeletePage))とした場合、resDeletePage as typeof etypeの型アノテーションを削除できるのですが、おそらくresDeletePage.result === "ok"も判定したいということですよね

"error"がresDeletePageのキーに含まれる場合
→ エラー（原因A）をスロー
"error"がresDeletePageのキーには含まれないが、resDeletePage.resultが"ok"ではない場合
→ エラー（原因B）をスロー
"error"がresDeletePageのキーに含まれず、resDeletePage.resultが"ok"の場合
→ 処理を続行

の３つで場合分けをする方針で進めましょうか？

…ssue_58

nkte8 · 2024-04-11T15:51:54Z

@zeke 確認ありがとうございます！

こちらの件ですが、あまり良い実装例が思い浮かばず...すみません。
一度別のIssueでお願いしたいと思います。

承知です！

ここで条件式をif (!("error" in resDeletePage))とした場合、resDeletePage as typeof etypeの型アノテーションを削除できるのですが、おそらくresDeletePage.result === "ok"も判定したいということですよね

ここなんですけど、そもそも元の処理が以下のようになっています。
firebase/functions/src/editDbEndpoint/entrypoint.prod.ts

...
        if (resPagedbdel === null) {
            response.status(500).send(errRes(500))
            return
        }
        if (typeof resPagedbdel.result === "undefined") {
            response.status(502).send(errRes(502))
            return
        }
        // 正常に削除完了
        response.status(200).send({
            result: "ok"
        });
        // OGPを削除
        const ogpFilename = `${handleFromId}/${rkeyFromId}`
        await deleteOgp({
            env: envName,
            ogpFilename: ogpFilename
        })
        response.end();

このAPIだいぶ昔に書いたのですが、多分レスポンス自体に意味はなさそうなんですよね...エラーが発生しなかった場合は必ずresponse 200で、{ result: ok }が返却され、それ以外の200レスポンスは無い感じです。
現在のコードではerrorの判定方法が曖昧だったから念の為条件を書いた、というように見えています。過去の自分ならやりそう。

ので、resDeletePage.resultの判定は消しちゃっていいです。おおもとのこのAPIも改善しようと思います。

…ssue_58

ZEKE320 · 2024-04-11T16:32:33Z

@nkte8 返信ありがとうございます！
対応行いましたので、再度レビューをお願いします

nkte8

対応ありがとうございました！マージします

* [Improve code]: pages/* 手動formatter・Linter対応 (#77) * 🚨 未使用の引数`error`の名前にアンダースコアを追加 * 🚨 テンプレートリテラル内のURL型をstring型に変換 * 🎨 astro/src/pagesにPrettierを適用 * 🎨 astro/src/components/*.astroにPrettierを適用 (#76) * 🎨 astro/src/layoutsにPrettierを適用 (#80) * [Improve code]: utils/atproto_api/* 手動formatter・Linter対応 (#81) * 🚨 astro/src/utils/atproto_apiに`eslint . --fix`を実行 * 🎨 astro/src/utils/atproto_api/models/*にPrettierを適用 * 🎨 astro/src/utils/atproto_api/*にPrettierを適用 * ✏️ searchの誤字を修正 * 🏷️ imageの$typeを"blob"のみに限定 * 🚨 暫定的に、astro/src/utils/atproto_api以下のレスポンスを、as構文で型付け * 🎨 astro/src/utils/atproto_api/*.tsにPrettierを適用 * 🏷️ uploadBlobの戻り値の型情報を分割 * [Improve code]: astro/src/components/Client/bsky/* 手動formatter・Linter対応 (#83) * 🚨 astro/src/components/Client/bsky/にESLintの--fixオプションを適用 * 🔒️ target="_blank"付のタグの脆弱性対策を追加詳細は下記を確認してください。 https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/jsx-no-target-blank.md * 🚨 readDrafts()が常にtruthyな値を返すようなので、OR演算子を削除 * 🏷️ atproto_apiの型定義変更に伴い、既存の実装の型情報を更新 * 🚨 Promise<void>を返す関数にvoidキーワードを追加 * 🚨 バッククォートで囲われたダブルクォートのエスケープを解除 * ✏️ identifierの綴りを修正 * 🚨 map()で繰り返し出力されるコンポーネントに、key属性を追加 * 🚨 使用されないエラー変数にアンダースコアを追加 * 🏷️ atproto_apiの型定義変更に伴い、既存の実装の型情報を更新 * 🎨 astro/src/components/Client/bskyにPrettierを適用 * ✏️ `LoginForm`の注意文言に、環境変数の`servicename`を用いるよう変更 * 💬 エラー処理後の型アサーションを解説するコメントを追記 * 🥅 使用されていないエラー変数名をアンダースコアのみに変更 * 🚨 エラー型の判定を`"error" in val`形式で統一 * ♻️ 変数定義と値の代入を凝集化 * 🚨 エラー型の判定を`"error" in val`形式で統一 * 🏷️ 型アサーションを用いない実装に変更 * 🔥 不要になったimport文を削除 * 🎨 astro/src/components/Client/bsky/*にPrettierを適用 * ESLint修正漏れの修正 --------- Co-authored-by: nekono <[email protected]> * [Improve code]: src/utils/*.ts 手動formatter・Linter対応 (#78) * 🚨 browser-image-compressionエラー処理のリンティングエラーを修正 * 🎨 astro/src/utils/*.tsにPrettierを適用 * ✏️ searchの綴りを修正 * ✨ useLocalStorage.tsにzodによる型チェックを実装 * 🏷️ Zodオブジェクトをタグ、下書きに各々定義 * ➕ 依存関係にZodを追加 * 🏷️ Zodオブジェクトから型情報を定義 & 実際の使用方法に即した型名に変更 * [Improve code]: lib/* 手動formatter・Linter対応 (#84) * 🚨 astro/src/libにESLintのfixオプションを適用 * 🚨 Astroの環境変数をstring型とみなす記述を追加 * 🚨 nullish判定を明確化 * 💬 コメントの空白を半角に変更 * 🏷️ 変数の型情報を明記 * 🎨 astro/src/libにPrettierを適用 * ✏️ ファイル名getIdの綴りを修正 * ✏️ ファイル名変更に伴いimport文を修正 * 🏷️ Astro API用の型をZodで再定義 * 🦺 APIレスポンス取得箇所にバリデーションチェックを追加 * 🏷️ モジュール外で型の名前が重複しないよう変更 * 🩹 APIレスポンス処理時の型エラーに関する文言を修正 * 🏷️ API呼び出し箇所の型定義更新に伴い、エラー判定箇所を修正 * 🥅 getOgpMetaエラー時のステータス番号を定義 * ✏️ Zodオブジェクトの命名規則を統一 * 🥅 エラーレスポンスからhtmlを削除 * 🔥 使用されていないレスポンス型定義用のJSONを削除 * 🥅 エラー型判定の実装を改善 * 🥅 エラー型の判定を`"error" in val`形式で統一 * ⚰️ 使用されていない変数を削除 * ⚰️ 不要になった行を削除 * ➕ 依存関係にZodを追加 * 🥅 エラー型の判定ロジックを変更 & 型アサーション削除 * 🔥 不要になったインポート行を削除 * API応答を待機するコードを追加 (#92) * 🔥 ヘッドブランチを`preview/*`に制限するコードを削除 * 🎨 変数の定義順と使用順を統一 * 💬 分かりやすいコメントに変更 * ✨ ベースブランチに`hotfix/*`が指定できるように修正 --------- Co-authored-by: nekono <[email protected]>

ZEKE320 added 16 commits April 6, 2024 10:56

🚨 astro/src/libにESLintのfixオプションを適用

6c056c3

🚨 Astroの環境変数をstring型とみなす記述を追加

a7a4af9

🚨 nullish判定を明確化

08cb5fe

💬 コメントの空白を半角に変更

a33a328

🏷️ 変数の型情報を明記

e8bc360

Merge remote-tracking branch 'upstream/develop' into feature/issue_58

6998425

🎨 astro/src/libにPrettierを適用

623f792

✏️ ファイル名getIdの綴りを修正

e5835ec

✏️ ファイル名変更に伴いimport文を修正

4f51579

🏷️ Astro API用の型をZodで再定義

c0b67f9

🦺 APIレスポンス取得箇所にバリデーションチェックを追加

700b531

🏷️ モジュール外で型の名前が重複しないよう変更

bad010e

🩹 APIレスポンス処理時の型エラーに関する文言を修正

3e1565a

🏷️ API呼び出し箇所の型定義更新に伴い、エラー判定箇所を修正

b61fddd

🥅 getOgpMetaエラー時のステータス番号を定義

dcaed42

Merge remote-tracking branch 'origin/feature/issue_58' into preview/i…

ced281e

…ssue_58

ZEKE320 self-assigned this Apr 7, 2024

ZEKE320 linked an issue Apr 7, 2024 that may be closed by this pull request

[Improve code]: lib/* 手動formatter・Linter対応 #58

Closed

9 tasks

ZEKE320 requested a review from nkte8 April 7, 2024 14:15

nkte8 mentioned this pull request Apr 8, 2024

[Improve code]: VSCodeにおけるESLintのハイライトが正常に表示されない #87

Closed

nkte8 requested changes Apr 8, 2024

View reviewed changes

ZEKE320 added this to the [Improve code] 手動Linter/Formatter対応 milestone Apr 8, 2024

ZEKE320 added 5 commits April 10, 2024 21:46

✏️ Zodオブジェクトの命名規則を統一

967b327

🥅 エラーレスポンスからhtmlを削除

4fd4e58

🔥 使用されていないレスポンス型定義用のJSONを削除

ce937a7

🥅 エラー型判定の実装を改善

0fc4c2d

Merge remote-tracking branch 'origin/feature/issue_58' into preview/i…

f42fca5

…ssue_58

ZEKE320 requested a review from nkte8 April 10, 2024 13:12

nkte8 mentioned this pull request Apr 10, 2024

[Improve code]: astro/src/components/Client/bsky/* 手動formatter・Linter対応 #83

Merged

ZEKE320 added 3 commits April 11, 2024 14:55

🥅 エラー型の判定を"error" in val形式で統一

bc2a8e8

⚰️ 使用されていない変数を削除

9fc9f9c

⚰️ 不要になった行を削除

16e1065

ZEKE320 added 2 commits April 11, 2024 15:39

➕ 依存関係にZodを追加

5443727

Merge remote-tracking branch 'origin/feature/issue_58' into preview/i…

96e9a01

…ssue_58

ZEKE320 added 3 commits April 11, 2024 16:26

🥅 エラー型の判定ロジックを変更 & 型アサーション削除

107d954

🔥 不要になったインポート行を削除

aa42a11

Merge remote-tracking branch 'origin/feature/issue_58' into preview/i…

62e71ac

…ssue_58

nkte8 approved these changes Apr 12, 2024

View reviewed changes

nkte8 merged commit 6b1ffe1 into develop Apr 12, 2024
1 check passed

nkte8 deleted the preview/issue_58 branch April 12, 2024 14:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Improve code]: lib/* 手動formatter・Linter対応 #84

[Improve code]: lib/* 手動formatter・Linter対応 #84

ZEKE320 commented Apr 7, 2024 •

edited

Loading

ZEKE320 commented Apr 7, 2024

nkte8 left a comment •

edited

Loading

nkte8 Apr 8, 2024

ZEKE320 Apr 10, 2024

nkte8 Apr 8, 2024

ZEKE320 Apr 10, 2024

nkte8 Apr 8, 2024

ZEKE320 Apr 10, 2024

nkte8 Apr 8, 2024

ZEKE320 Apr 10, 2024

nkte8 Apr 8, 2024

ZEKE320 Apr 10, 2024

nkte8 Apr 8, 2024

ZEKE320 Apr 10, 2024

nkte8 Apr 8, 2024

ZEKE320 Apr 10, 2024

nkte8 Apr 8, 2024

ZEKE320 Apr 10, 2024

cloudflare-workers-and-pages bot commented Apr 10, 2024 •

edited

Loading

ZEKE320 commented Apr 10, 2024

ZEKE320 commented Apr 10, 2024

nkte8 commented Apr 10, 2024 •

edited

Loading

ZEKE320 commented Apr 11, 2024

nkte8 commented Apr 11, 2024

ZEKE320 commented Apr 11, 2024

nkte8 left a comment


		if (typeof getpage_res?.error !== "undefined" && getpage_res.error !== "") {
		if ("error" in getPageResult && typeof getPageResult?.error !== "undefined" && getPageResult.error !== "") {

[Improve code]: lib/* 手動formatter・Linter対応 #84

[Improve code]: lib/* 手動formatter・Linter対応 #84

Conversation

ZEKE320 commented Apr 7, 2024 • edited Loading

本PRで行われた変更

ZEKE320 commented Apr 7, 2024

nkte8 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cloudflare-workers-and-pages bot commented Apr 10, 2024 • edited Loading

Deploying skyshare with Cloudflare Pages

ZEKE320 commented Apr 10, 2024

ZEKE320 commented Apr 10, 2024

nkte8 commented Apr 10, 2024 • edited Loading

ZEKE320 commented Apr 11, 2024

nkte8 commented Apr 11, 2024

ZEKE320 commented Apr 11, 2024

nkte8 left a comment

Choose a reason for hiding this comment

ZEKE320 commented Apr 7, 2024 •

edited

Loading

nkte8 left a comment •

edited

Loading

cloudflare-workers-and-pages bot commented Apr 10, 2024 •

edited

Loading

nkte8 commented Apr 10, 2024 •

edited

Loading