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: PackedHoge型をPacked<'Hoge'>型に書き換える #7792

Merged
merged 60 commits into from
Sep 22, 2021

Conversation

tamaina
Copy link
Contributor

@tamaina tamaina commented Sep 12, 2021

Spin off of #7769

Required PR

#7789

What

misc/schema.tsでPacked型を実装し、従来repositoryファイルで定義していたpackedHogeをPacked<'Hoge'>と置き換えます。

Why

VS Code上で変数をホバーして型を見ようとするとき、PackedHogeのままだと意味不明な表示がされるが、Packed<'Hoge'>とジェネリクス型にするとPacked<'Hoge'>と表示されるので、見易くなる。

全て書き換えるメリットとして、

  • 各repositoryファイルでtype PackedHoge = Packed<'Hoge'>;と定義しなおしてPackedHogeを外部から参照するより、直接Packed型を参照すればスマートだと思う
  • PackedHoge, PackedFuga... と大量にPacked型を読み込む場合に、importをひとつにまとめることができる

@tamaina tamaina marked this pull request as draft September 12, 2021 13:31
@tamaina tamaina marked this pull request as ready for review September 12, 2021 14:10
@tamaina tamaina requested a review from syuilo September 12, 2021 14:10
@tamaina
Copy link
Contributor Author

tamaina commented Sep 12, 2021

done

@syuilo
Copy link
Member

syuilo commented Sep 18, 2021

VS Code上で変数をホバーして型を見ようとするとき、PackedHogeのままだと意味不明な表示がされるが、Packed<'Hoge'>とジェネリクス型にするとPacked<'Hoge'>と表示されるので、見易くなる。

これについては各Packedを普通にmisskey.jsみたいに型定義すれば解決しそう

@syuilo
Copy link
Member

syuilo commented Sep 18, 2021

(でもOpenAPIのスキーマと二重に定義するのも微妙か)

@tamaina
Copy link
Contributor Author

tamaina commented Sep 18, 2021

OpenAPIのスキーマと二重に定義する

二重にするとどっちかがアップデートされないとかがあり得るので(特にMisskeyは…)これはやりたくない
(というかこれ否定したらわざわざ苦労して #4770 とかでSchemaTypeを使っている理由とは…)

@@ -52,7 +52,7 @@ export default async function(

if (note.user != null) { // たぶんnullになることは無いはずだけど一応
for (const antenna of myAntennas) {
if (checkHitAntenna(antenna, note, note.user as any, undefined, Array.from(following))) {
if (await checkHitAntenna(antenna, note, note.user as any, undefined, Array.from(following))) {
Copy link
Member

Choose a reason for hiding this comment

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

これバグ修正?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

あー、これは別PRにしとくか

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#7809 にした

const matching = typeof src === 'object' ? src : await this.findOneOrFail(src);

return await awaitAll({
id: matching.id,
createdAt: matching.createdAt,
createdAt: matching.createdAt.toISOString(),
Copy link
Member

Choose a reason for hiding this comment

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

バグ修正?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

バグ修正というよりは型定義に合わせた感じ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

いや意外と影響あるか

@syuilo
Copy link
Member

syuilo commented Sep 18, 2021

リファクタ以外の変更も色々入ってそうだけど、バグ修正という認識でいいんかな

@tamaina
Copy link
Contributor Author

tamaina commented Sep 18, 2021

かも?

Copy link
Member

@syuilo syuilo left a comment

Choose a reason for hiding this comment

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

多分よさそう

@tamaina
Copy link
Contributor Author

tamaina commented Sep 22, 2021

コンフリクトしてたので解消した

@syuilo syuilo merged commit 14795b6 into misskey-dev:develop Sep 22, 2021
@syuilo
Copy link
Member

syuilo commented Sep 22, 2021

🙏

@tamaina tamaina deleted the packed-type branch September 24, 2021 02:24
snk-lab pushed a commit to snk-lab/misskey-kusa-note that referenced this pull request Sep 24, 2021
* packedNotificationSchemaを更新

* read:gallery, write:gallery, read:gallery-likes, write:gallery-likesに翻訳を追加

* fix

* add header, choice, invitation

* test

* fix

* yatta

* remove no longer needed "as PackedUser/PackedNote"

* clean up

* add simple-schema

* fix lint

* define items in full Schema

* revert misskey-dev#7772 (comment)

* user packとnote packの型不整合を修正

* add prelude/types.ts

* emoji

* signin

* game

* matching

* fix

* add emoji schema

* add reversiGame

* add reversiMatching

* remove signin schema (use Signin entity)

* add Packed type

* note-reaction

* user

* user-group

* user-list

* note

* app, messaging-message

* notification

* drive-file

* drive-folder

* following

* muting

* blocking

* hashtag

* page

* app (with modifying schema)

* import user?

* channel

* antenna

* clip

* gallery-post

* emoji

* Packed

* reversi-matching

* add changelog

* add changelog

* revert fix
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.

3 participants