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: publishHogeStreamとStreamのEventEmitterに型定義する #7769

Merged
merged 127 commits into from
Oct 20, 2021

Conversation

tamaina
Copy link
Member

@tamaina tamaina commented Sep 5, 2021

Resolve #7765

Required PR

(#7771, #7772 (merged)), #7789, #7792

What

型定義をがんばる

  • Redisから受け取る方(EventEmitter)の型定義
  • Redisにpublishする方(publish**Stream)の型定義
    ついでに炙り出したバグの修正
    • internal antennaUpdatedイベントがPromiseのままだったのをawait

Why

Streamまわりを開発するたびにどのようなデータが通信されているのか調べるのが面倒

@tamaina tamaina self-assigned this Sep 5, 2021
@tamaina tamaina marked this pull request as draft September 5, 2021 14:53
@tamaina tamaina changed the title wip: refactor: publish**StreamとStreamのEventEmitterに型定義する refactor: publish**StreamとStreamのEventEmitterに型定義する Sep 5, 2021
@tamaina
Copy link
Member Author

tamaina commented Sep 5, 2021

サーバー側のコードにmisskey.jsを使うのはやっぱ無理ある

@tamaina
Copy link
Member Author

tamaina commented Sep 5, 2021

packedNotificationSchemaは別のPRにするか

@tamaina
Copy link
Member Author

tamaina commented Sep 5, 2021

あ~、packedNotificationSchemaを書いたとしてもrefをよしなに解読してくれるわけではないので結局エラーは出るのか

@tamaina tamaina mentioned this pull request Sep 5, 2021
@tamaina tamaina mentioned this pull request Sep 17, 2021
9 tasks
CHANGELOG.md Outdated Show resolved Hide resolved
@syuilo
Copy link
Member

syuilo commented Oct 2, 2021

これもうrewuiredなpr全部マージされたっぽい?

Comment on lines 66 to 95
private onUserEvent(data: StreamMessages['user']['spec']) { // { type, body }と展開すると型も展開されてしまう
switch (data.type) {
case 'follow':
this.following.add(body.id);
this.following.add(data.body.id);
break;

case 'unfollow':
this.following.delete(body.id);
this.following.delete(data.body.id);
break;

case 'mute':
this.muting.add(body.id);
this.muting.add(data.body.id);
break;

case 'unmute':
this.muting.delete(body.id);
this.muting.delete(data.body.id);
break;

// TODO: block events

case 'followChannel':
this.followingChannels.add(body.id);
this.followingChannels.add(data.body.id);
break;

case 'unfollowChannel':
this.followingChannels.delete(body.id);
this.followingChannels.delete(data.body.id);
break;

case 'updateUserProfile':
this.userProfile = body;
this.userProfile = data.body;
Copy link
Contributor

Choose a reason for hiding this comment

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

複数行に渡るなら関数内部で const { type, body } = data しちゃってもいいのでは?と思ったり

Copy link
Member Author

@tamaina tamaina Oct 3, 2021

Choose a reason for hiding this comment

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

残念なお知らせ: constで展開してもtypeとbodyの型が分離してしまうのは変わらない

ジェネリックで関数を定義しても分離してしまうのでswitch-caseのTypeScriptの仕様なんだと思う

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
Member

Choose a reason for hiding this comment

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

this.publish('broadcast', type, typeof value === 'undefined' ? null : value);
}

public publishMainStream = (userId: User['id'], type: string, value?: any): void => {
public publishMainStream = <K extends keyof MainStreamTypes>(userId: User['id'], type: K, value?: MainStreamTypes[K]): void => {
Copy link
Contributor

Choose a reason for hiding this comment

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

よくなった?

@tamaina
Copy link
Member Author

tamaina commented Oct 3, 2021

これもうrewuiredなpr全部マージされたっぽい?

yes

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.

yosasou

@syuilo syuilo merged commit 69b56f6 into misskey-dev:develop Oct 20, 2021
@syuilo
Copy link
Member

syuilo commented Oct 20, 2021

🆒

syuilo added a commit that referenced this pull request Apr 30, 2022
* clean up

* ev => data

* refactor

* clean up

* add type

* antenna

* channel

* fix

* add Packed type

* add PackedRef

* fix lint

* add emoji schema

* add reversiGame

* add reversiMatching

* remove signin schema (use Signin entity)

* add schemas refs, fix Packed type

* wip PackedHoge => Packed<'Hoge'>

* 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

* update stream.ts

* #7769 (comment)

* fix lint

* clean up?

* add app

* fix

* nanka iroiro

* wip

* wip

* fix lint

* fix loginId

* fix

* refactor

* refactor

* remove follow action

* clean up

* Revert "remove follow action"

This reverts commit defbb41.

* Revert "clean up"

This reverts commit f94919c.

* remove fetch specification

* renoteの条件追加

* apiFetch => cli

* bypass fetch?

* fix

* refactor: use path alias

* temp: add submodule

* remove submodule

* enhane: unison-reloadに指定したパスに移動できるように

* null

* null

* feat: ログインするアカウントのIDをクエリ文字列で指定する機能

* null

* await?

* rename

* rename

* Update read.ts

* merge

* get-note-summary

* fix

* swパッケージに

* add missing packages

* fix getNoteSummary

* add webpack-cli

* ✌️

* remove plugins

* sw-inject分離したがテストしてない

* fix notification.vue

* remove a blank line

* disconnect intersection observer

* disconnect2

* fix notification.vue

* remove a blank line

* disconnect intersection observer

* disconnect2

* fix

* ✌️

* clean up config

* typesを戻した

* Update packages/client/src/components/notification.vue

Co-authored-by: Acid Chicken (硫酸鶏) <root@acid-chicken.com>

* disconnect

* oops

* Failed to load the script unexpectedly回避
sw.jsとlib.tsを分離してみた

* truncate notification

* Update packages/client/src/ui/_common_/common.vue

Co-authored-by: syuilo <Syuilotan@yahoo.co.jp>

* clean up

* clean up

* キャッシュ対策

* Truncate push notification message

* クライアントがあったらストリームに接続しているということなので通知しない判定の位置を修正

* components/drive-file-thumbnail.vue

* components/drive-select-dialog.vue

* components/drive-window.vue

* merge

* fix

* Service Workerのビルドにesbuildを使うようにする

* return createEmptyNotification()

* fix

* i18n.ts

* update

* ✌️

* remove ts-loader

* fix

* fix

* enhance: Service Workerを常に登録するように

* pollEnded

* URLをsw.jsに戻す

* clean up

Co-authored-by: Acid Chicken (硫酸鶏) <root@acid-chicken.com>
Co-authored-by: syuilo <Syuilotan@yahoo.co.jp>
syuilo added a commit that referenced this pull request May 1, 2022
* update stream.ts

* #7769 (comment)

* fix lint

* clean up?

* add app

* fix

* nanka iroiro

* wip

* wip

* fix lint

* fix loginId

* fix

* refactor

* refactor

* remove follow action

* clean up

* Revert "remove follow action"

This reverts commit defbb41.

* Revert "clean up"

This reverts commit f94919c.

* remove fetch specification

* renoteの条件追加

* apiFetch => cli

* bypass fetch?

* fix

* refactor: use path alias

* temp: add submodule

* remove submodule

* enhane: unison-reloadに指定したパスに移動できるように

* null

* null

* feat: ログインするアカウントのIDをクエリ文字列で指定する機能

* null

* await?

* rename

* rename

* Update read.ts

* merge

* get-note-summary

* fix

* swパッケージに

* add missing packages

* fix getNoteSummary

* add webpack-cli

* ✌️

* remove plugins

* sw-inject分離したがテストしてない

* fix notification.vue

* remove a blank line

* disconnect intersection observer

* disconnect2

* fix notification.vue

* remove a blank line

* disconnect intersection observer

* disconnect2

* fix

* ✌️

* clean up config

* typesを戻した

* Update packages/client/src/components/notification.vue

Co-authored-by: Acid Chicken (硫酸鶏) <root@acid-chicken.com>

* disconnect

* oops

* Failed to load the script unexpectedly回避
sw.jsとlib.tsを分離してみた

* truncate notification

* Update packages/client/src/ui/_common_/common.vue

Co-authored-by: syuilo <Syuilotan@yahoo.co.jp>

* clean up

* clean up

* キャッシュ対策

* Truncate push notification message

* クライアントがあったらストリームに接続しているということなので通知しない判定の位置を修正

* components/drive-file-thumbnail.vue

* components/drive-select-dialog.vue

* components/drive-window.vue

* merge

* fix

* Service Workerのビルドにesbuildを使うようにする

* return createEmptyNotification()

* fix

* i18n.ts

* update

* ✌️

* remove ts-loader

* fix

* fix

* enhance: Service Workerを常に登録するように

* pollEnded

* URLをsw.jsに戻す

* clean up

* wip

* wip

* wip

* wip

* wip

* wip

* ✌️

* use import

* fix

* install rollup

* use defineAsyncComponent.

* fix emojilist

* wip use defineAsyncComponent

* popup(import -> popup(defineAsyncComponent(() => import

* draggable?

* fix init import

* clean up

* fix router

* add comment

* ✌️

* ✌️

* ✌️

* remove webpack

* update vite

* fix boot sequence

* Revert "fix boot sequence"

This reverts commit e893dbf.

* revert boot import

* never make two app div

* ;

* remove console.log

* change clientEntry sequence

* fix

* Revert "fix"

This reverts commit 12741b3.

* fix

* add comment #8575 (comment)

* add log

* add comment

Co-authored-by: Acid Chicken (硫酸鶏) <root@acid-chicken.com>
Co-authored-by: syuilo <Syuilotan@yahoo.co.jp>
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.

Stream APIのEventEmitterに型定義する
5 participants