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

refactor(frontend): Reactivityで型を明示するように #12791

Merged
merged 3 commits into from
Dec 26, 2023

Conversation

zyoshoka
Copy link
Contributor

What

Vue の Reactivity を用いている箇所で、型を明示する必要がある箇所について(全てではないですが)ある程度修正を行いました。また、必要に応じてエンドポイントのスキーマを更新しました。

Why

  • 既存のコードに含まれる ref()ref<any>()Ref<any> と推論されます。any 以外の型を明示することで、このような any をなるべく減らします。
  • 既存のコードに含まれる ref(null)Ref<null> と推論されるため、null 以外の値を代入しようとするとエラーが発生してしまいます。型を明示することで、このエラーをなるべく減らします1
  • 既存のコードに含まれる ref([])Ref<never[]> と推論されるため、何かしらの値を代入しようとするとエラーが発生します。型を明示することで、このエラーをなるべく減らします。

Additional info (optional)

Checklist

  • Read the contribution guide
  • Test working in a local environment
  • (If needed) Add story of storybook
  • (If needed) Update CHANGELOG.md
  • (If possible) Add tests

Footnotes

  1. なお ref(null) については、修正しきれなかった箇所は(any で検索した際に候補に出るようにしたいので)ref<any>(null) へ書き換えを行っていますが、もしこれが不適切であればもとに戻します。

@github-actions github-actions bot added packages/frontend Client side specific issue/PR packages/backend Server side specific issue/PR packages/misskey-js labels Dec 25, 2023
Copy link
Contributor

github-actions bot commented Dec 25, 2023

このPRによるapi.jsonの差分

差分はこちら
--- base
+++ head
@@ -400,6 +400,10 @@
                       "type": "string",
                       "nullable": true
                     },
+                    "shortName": {
+                      "type": "string",
+                      "nullable": true
+                    },
                     "objectStorageS3ForcePathStyle": {
                       "type": "boolean",
                       "nullable": false
@@ -515,6 +519,7 @@
                     "maintainerEmail",
                     "maintainerName",
                     "name",
+                    "shortName",
                     "objectStorageS3ForcePathStyle",
                     "privacyPolicyUrl",
                     "repositoryUrl",
@@ -31954,79 +31959,9 @@
                       "type": "array",
                       "nullable": false,
                       "items": {
-                        "properties": {
-                          "id": {
-                            "type": "string"
-                          },
-                          "firstRetrievedAt": {
-                            "type": "string"
-                          },
-                          "host": {
-                            "type": "string"
-                          },
-                          "usersCount": {
-                            "type": "number"
-                          },
-                          "notesCount": {
-                            "type": "number"
-                          },
-                          "followingCount": {
-                            "type": "number"
-                          },
-                          "followersCount": {
-                            "type": "number"
-                          },
-                          "isNotResponding": {
-                            "type": "boolean"
-                          },
-                          "isSuspended": {
-                            "type": "boolean"
-                          },
-                          "isBlocked": {
-                            "type": "boolean"
-                          },
-                          "softwareName": {
-                            "type": "string"
-                          },
-                          "softwareVersion": {
-                            "type": "string"
-                          },
-                          "openRegistrations": {
-                            "type": "boolean"
-                          },
-                          "name": {
-                            "type": "string"
-                          },
-                          "description": {
-                            "type": "string"
-                          },
-                          "maintainerName": {
-                            "type": "string"
-                          },
-                          "maintainerEmail": {
-                            "type": "string"
-                          },
-                          "isSilenced": {
-                            "type": "boolean"
-                          },
-                          "iconUrl": {
-                            "type": "string"
-                          },
-                          "faviconUrl": {
-                            "type": "string"
-                          },
-                          "themeColor": {
-                            "type": "string"
-                          },
-                          "infoUpdatedAt": {
-                            "type": "string",
-                            "nullable": true
-                          },
-                          "latestRequestReceivedAt": {
-                            "type": "string",
-                            "nullable": true
-                          }
-                        }
+                        "type": "object",
+                        "nullable": false,
+                        "$ref": "#/components/schemas/FederationInstance"
                       }
                     },
                     "otherFollowersCount": {
@@ -32036,79 +31971,9 @@
                       "type": "array",
                       "nullable": false,
                       "items": {
-                        "properties": {
-                          "id": {
-                            "type": "string"
-                          },
-                          "firstRetrievedAt": {
-                            "type": "string"
-                          },
-                          "host": {
-                            "type": "string"
-                          },
-                          "usersCount": {
-                            "type": "number"
-                          },
-                          "notesCount": {
-                            "type": "number"
-                          },
-                          "followingCount": {
-                            "type": "number"
-                          },
-                          "followersCount": {
-                            "type": "number"
-                          },
-                          "isNotResponding": {
-                            "type": "boolean"
-                          },
-                          "isSuspended": {
-                            "type": "boolean"
-                          },
-                          "isBlocked": {
-                            "type": "boolean"
-                          },
-                          "softwareName": {
-                            "type": "string"
-                          },
-                          "softwareVersion": {
-                            "type": "string"
-                          },
-                          "openRegistrations": {
-                            "type": "boolean"
-                          },
-                          "name": {
-                            "type": "string"
-                          },
-                          "description": {
-                            "type": "string"
-                          },
-                          "maintainerName": {
-                            "type": "string"
-                          },
-                          "maintainerEmail": {
-                            "type": "string"
-                          },
-                          "isSilenced": {
-                            "type": "boolean"
-                          },
-                          "iconUrl": {
-                            "type": "string"
-                          },
-                          "faviconUrl": {
-                            "type": "string"
-                          },
-                          "themeColor": {
-                            "type": "string"
-                          },
-                          "infoUpdatedAt": {
-                            "type": "string",
-                            "nullable": true
-                          },
-                          "latestRequestReceivedAt": {
-                            "type": "string",
-                            "nullable": true
-                          }
-                        }
+                        "type": "object",
+                        "nullable": false,
+                        "$ref": "#/components/schemas/FederationInstance"
                       }
                     },
                     "otherFollowingCount": {
@@ -32274,79 +32139,9 @@
                       "type": "array",
                       "nullable": false,
                       "items": {
-                        "properties": {
-                          "id": {
-                            "type": "string"
-                          },
-                          "firstRetrievedAt": {
-                            "type": "string"
-                          },
-                          "host": {
-                            "type": "string"
-                          },
-                          "usersCount": {
-                            "type": "number"
-                          },
-                          "notesCount": {
-                            "type": "number"
-                          },
-                          "followingCount": {
-                            "type": "number"
-                          },
-                          "followersCount": {
-                            "type": "number"
-                          },
-                          "isNotResponding": {
-                            "type": "boolean"
-                          },
-                          "isSuspended": {
-                            "type": "boolean"
-                          },
-                          "isBlocked": {
-                            "type": "boolean"
-                          },
-                          "softwareName": {
-                            "type": "string"
-                          },
-                          "softwareVersion": {
-                            "type": "string"
-                          },
-                          "openRegistrations": {
-                            "type": "boolean"
-                          },
-                          "name": {
-                            "type": "string"
-                          },
-                          "description": {
-                            "type": "string"
-                          },
-                          "maintainerName": {
-                            "type": "string"
-                          },
-                          "maintainerEmail": {
-                            "type": "string"
-                          },
-                          "isSilenced": {
-                            "type": "boolean"
-                          },
-                          "iconUrl": {
-                            "type": "string"
-                          },
-                          "faviconUrl": {
-                            "type": "string"
-                          },
-                          "themeColor": {
-                            "type": "string"
-                          },
-                          "infoUpdatedAt": {
-                            "type": "string",
-                            "nullable": true
-                          },
-                          "latestRequestReceivedAt": {
-                            "type": "string",
-                            "nullable": true
-                          }
-                        }
+                        "type": "object",
+                        "nullable": false,
+                        "$ref": "#/components/schemas/FederationInstance"
                       }
                     },
                     "otherFollowersCount": {
@@ -32356,79 +32151,9 @@
                       "type": "array",
                       "nullable": false,
                       "items": {
-                        "properties": {
-                          "id": {
-                            "type": "string"
-                          },
-                          "firstRetrievedAt": {
-                            "type": "string"
-                          },
-                          "host": {
-                            "type": "string"
-                          },
-                          "usersCount": {
-                            "type": "number"
-                          },
-                          "notesCount": {
-                            "type": "number"
-                          },
-                          "followingCount": {
-                            "type": "number"
-                          },
-                          "followersCount": {
-                            "type": "number"
-                          },
-                          "isNotResponding": {
-                            "type": "boolean"
-                          },
-                          "isSuspended": {
-                            "type": "boolean"
-                          },
-                          "isBlocked": {
-                            "type": "boolean"
-                          },
-                          "softwareName": {
-                            "type": "string"
-                          },
-                          "softwareVersion": {
-                            "type": "string"
-                          },
-                          "openRegistrations": {
-                            "type": "boolean"
-                          },
-                          "name": {
-                            "type": "string"
-                          },
-                          "description": {
-                            "type": "string"
-                          },
-                          "maintainerName": {
-                            "type": "string"
-                          },
-                          "maintainerEmail": {
-                            "type": "string"
-                          },
-                          "isSilenced": {
-                            "type": "boolean"
-                          },
-                          "iconUrl": {
-                            "type": "string"
-                          },
-                          "faviconUrl": {
-                            "type": "string"
-                          },
-                          "themeColor": {
-                            "type": "string"
-                          },
-                          "infoUpdatedAt": {
-                            "type": "string",
-                            "nullable": true
-                          },
-                          "latestRequestReceivedAt": {
-                            "type": "string",
-                            "nullable": true
-                          }
-                        }
+                        "type": "object",
+                        "nullable": false,
+                        "$ref": "#/components/schemas/FederationInstance"
                       }
                     },
                     "otherFollowingCount": {
@@ -48589,25 +48314,42 @@
                         "type": "object",
                         "nullable": false,
                         "properties": {
-                          "place": {
+                          "id": {
                             "type": "string",
-                            "nullable": false
+                            "nullable": false,
+                            "format": "id",
+                            "example": "xxxxxxxxxx"
                           },
                           "url": {
                             "type": "string",
                             "nullable": false,
                             "format": "url"
                           },
+                          "place": {
+                            "type": "string",
+                            "nullable": false
+                          },
+                          "ratio": {
+                            "type": "number",
+                            "nullable": false
+                          },
                           "imageUrl": {
                             "type": "string",
                             "nullable": false,
                             "format": "url"
+                          },
+                          "dayOfWeek": {
+                            "type": "integer",
+                            "nullable": false
                           }
                         },
                         "required": [
-                          "place",
+                          "id",
                           "url",
-                          "imageUrl"
+                          "place",
+                          "ratio",
+                          "imageUrl",
+                          "dayOfWeek"
                         ]
                       }
                     },
@@ -56438,7 +56180,19 @@
               "application/json": {
                 "schema": {
                   "type": "object",
-                  "nullable": false
+                  "nullable": false,
+                  "properties": {
+                    "sourceLang": {
+                      "type": "string"
+                    },
+                    "text": {
+                      "type": "string"
+                    }
+                  },
+                  "required": [
+                    "sourceLang",
+                    "text"
+                  ]
                 }
               }
             }

Get diff files from Workflow Page

Copy link

codecov bot commented Dec 25, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (237fe24) 79.79% compared to head (43af1fc) 79.77%.
Report is 5 commits behind head on develop.

Files Patch % Lines
packages/frontend/src/scripts/get-note-menu.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #12791      +/-   ##
===========================================
- Coverage    79.79%   79.77%   -0.02%     
===========================================
  Files          956      956              
  Lines       108807   108794      -13     
  Branches      8372     8372              
===========================================
- Hits         86818    86789      -29     
- Misses       21989    22005      +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@syuilo
Copy link
Member

syuilo commented Dec 25, 2023

const favorites = ref();

だったのが

const favorites = ref<Misskey.entities.Clip[]>([]);

みたいに初期値に空配列を入れるように変わってる箇所がいくつかあるけど、「値がまだfetchされていない」ことと「fetchしたけど結果が空だった」ことは区別される場合が多いから不具合が起きそう

@syuilo
Copy link
Member

syuilo commented Dec 25, 2023

ので

const favorites = ref<Misskey.entities.Clip[] | null>(null);

みたいにするのが良さそう

@zyoshoka
Copy link
Contributor Author

ごもっともです。修正します。

@zyoshoka
Copy link
Contributor Author

元々空配列だった部分についてはあえてそのままにしてあります🙏

@@ -62,7 +62,7 @@ const props = defineProps<{
}>();

const note = ref<null | Misskey.entities.Note>();
const clips = ref();
const clips = ref<Misskey.entities.Clip[]>();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const clips = ref<Misskey.entities.Clip[]>();
const clips = ref<Misskey.entities.Clip[] | null>(null);

かも

Copy link
Contributor Author

Choose a reason for hiding this comment

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

これは undefined になるのでいいかなと思っていたのですが、null に寄せたほうが良いでしょうか?

Copy link
Member

Choose a reason for hiding this comment

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

どっちでも良さそう
ただ undefined にする場合でも型は
Misskey.entities.Clip[] | undefinedにする必要がありそう

Copy link
Contributor Author

Choose a reason for hiding this comment

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

初期値の省略時は Ref<Misskey.entities.Clip[] | undefined> で推論されるので、明示する必要はなさそうです。
https://ja.vuejs.org/guide/typescript/composition-api#typing-ref

Copy link
Member

Choose a reason for hiding this comment

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

ほーー

@@ -53,7 +54,7 @@ const props = withDefaults(defineProps<{
});

const tab = ref(props.initialTab);
const role = ref();
const role = ref<Misskey.entities.Role>();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const role = ref<Misskey.entities.Role>();
const role = ref<Misskey.entities.Role | null>(null);

packages/frontend/src/pages/role.vue Show resolved Hide resolved
@@ -58,8 +59,8 @@ const { widgetProps, configure, save } = useWidgetPropsManager(name,
emit,
);

const list = ref();
const users = ref([]);
const list = ref<Misskey.entities.UserList>();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const list = ref<Misskey.entities.UserList>();
const list = ref<Misskey.entities.UserList | null>(null);

@syuilo syuilo merged commit 75034d9 into misskey-dev:develop Dec 26, 2023
18 checks passed
@syuilo
Copy link
Member

syuilo commented Dec 26, 2023

🙏🙏🙏🙏🙏

@zyoshoka
Copy link
Contributor Author

ご丁寧なレビューありがとうございました🙏

@zyoshoka zyoshoka deleted the type-ref-null branch December 26, 2023 05:21
camilla-ett pushed a commit to kaseiski/misskey that referenced this pull request Jan 2, 2024
* refactor(frontend): Reactivityで型を明示するように

* fix: プロパティの参照が誤っているのを修正

* fix: 初期化の値を空配列に書き換えていた部分をnullに置き換え
madonuko pushed a commit to FyraLabs/nyaakey that referenced this pull request Feb 26, 2024
* refactor(frontend): Reactivityで型を明示するように

* fix: プロパティの参照が誤っているのを修正

* fix: 初期化の値を空配列に書き換えていた部分をnullに置き換え
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend Server side specific issue/PR packages/frontend Client side specific issue/PR packages/misskey-js
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants