optimize: VueUse を用いてグローバルなイベント監視を効率化する by uni-kakurenbo · Pull Request #4890 · traPtitech/traQ_S-UI

uni-kakurenbo · 2025-10-28T05:58:21Z

なぜこの PR を入れたいのか

fix(channel-view): 短時間での通知設定の再変更がイベントログに複数の記録を残さないようにする #4826, fix: 日をまたいだ際に日時の表示を更新するように修正 #4750 あたりで使いたい / 使えそう
- 状態を持たない composable を手軽に使いまわしたりできると嬉しい
Shared Component のためだけに Pinia を持ち出すのは微妙な感じがする
onBeforeUnload みたいなのをもっと楽に書きたい
イベントリスナーの cleanup 忘れを防止したい

動作確認の手順

実際に使ってみて、現在の挙動と差異がないことを確かめる

PR を出す前の確認事項

動作確認ができている
自分で一度コードを眺めて自分的に問題はなさそう

github-actions · 2025-10-28T05:58:29Z

Preview (prod) → https://4890-prod.traq-preview.trapti.tech/
Preview (dev) → https://4890-dev.traq-preview.trapti.tech/

codecov · 2025-10-28T06:00:03Z

Codecov Report

❌ Patch coverage is 14.28571% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.81%. Comparing base (ecb0d1d) to head (c0cf29d).
⚠️ Report is 14 commits behind head on master.

Files with missing lines	Patch %	Lines
src/composables/dom/useInsertText.ts	0.00%	5 Missing ⚠️
src/lib/basic/array.ts	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4890      +/-   ##
==========================================
- Coverage   61.01%   60.81%   -0.20%     
==========================================
  Files         101      101              
  Lines        2970     2973       +3     
  Branches      615      616       +1     
==========================================
- Hits         1812     1808       -4     
- Misses       1061     1071      +10     
+ Partials       97       94       -3

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…き換える

- refactor: `MouseEvent` と `TouchEvent` を併用している部分を `PointerEvent` に統合する

… optimize/effect-scope

cp-20

ありがとう

いくつかコメントしたけど、概ね良いと思います

cp-20 · 2025-12-29T11:44:21Z

src/lib/utils/createSharedComposable.ts

+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    return state!


non-null というよりは as ReturnType<Fn> な気がしたけど、どうだろう (Fn に何が入るのか分からない以上 null が返ってくるかもしれない)

たしかにです
そうします

cp-20 · 2025-12-29T11:54:07Z

src/lib/dom/unrefElement.ts

+export function unrefElement<T>(
+  element: MaybeRefOrGetter<T>
+): UnrefElementReturn<T>


シグネチャを分けてる意味ある？ (関数オーバーロード？)

だいぶ前に書いた部分なので忘れてしまったのですが、たぶんない気がしたので直します

cp-20 · 2025-12-29T11:58:09Z

src/composables/dom/useEventListener.ts

+  _targets: MaybeRefOrGetter<MaybeArray<Target> | null | undefined>,
+  _events: MaybeRefOrGetter<MaybeArray<EventNames<Target>>>,
+  _handlers: MaybeRef<MaybeArray<EventListenerType<EventType>>>,
+  options?: MaybeRefOrGetter<boolean | AddEventListenerOptions>


_ の prefix をつけるより argment であることを示した方が良いかも (_ の prefix は未使用変数に付けるのが慣例的な気がしている)

cp-20 · 2025-12-29T12:05:32Z

src/composables/useResponsive.ts

+import useEventListener from './dom/useEventListener'
+
+const useResponsive = () => {
+  const queryList = window.matchMedia(`(max-width: ${mobileMinBreakpoint}px)`)


この変更の本質ではないけど、なんで queryList って命名なんだろう

ここは過去のコードから引き継いだ部分なので定かではないのですが、window.matchMedia が返すのが MediaQueryList だからな気がします

cp-20 · 2025-12-29T12:21:59Z

src/composables/dom/useEventListener.ts

+  _targets: MaybeRefOrGetter<MaybeArray<Target> | null | undefined>,
+  _events: MaybeRefOrGetter<MaybeArray<EventNames<Target>>>,
+  _handlers: MaybeRef<MaybeArray<EventListenerType<EventType>>>,


これらが MaybeArray を受け取るのはなんでですか？ (見た感じ array で受け取っている箇所はなさそう？)

VueUse の useEventListener が MaybeArray を受け付けていたので、使用感がそろっていたほうが嬉しいかと思ってそのようにしました。

なるほど、、、　合わせても良いけど、実装が複雑になるし実際使うことはほぼない気がするから (配列にするときはその分だけ useEventListener を呼び出せばよい) 配列を受け取らなくても良い気がするな

確かにです、そうします
(と思ったのですがそもそも VueUse を導入してしまってもよさそう....？)

まかせます

わざわざこっちでメンテナンスする理由が特に思い浮かばないのと、他にも VueUse で置換できそうな部分があったので VueUse を導入しました

cp-20 · 2025-12-29T12:24:23Z

src/components/UI/composables/useImageViewer.ts

-    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-    containerEle.value!.addEventListener(
-      'mouseup',
-      _upEvent => {
-        // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-        containerEle.value!.removeEventListener('mousemove', moveUpdate)
+    const stop = useEventListener(
+      containerEle,
+      'mousemove',
+      (moveEvent: MouseEvent) => {
+        stop()
+        moveUpdate(moveEvent)
      },
-      { once: true }
+      {
+        once: true
+      }
    )


これが消えてる？

あっ、ミスです直します

cp-20 · 2025-12-29T12:24:52Z

src/components/UI/ClickOutside.ts

-const startEventName = isIOS() ? 'touchstart' : 'mousedown'
-const endEventName = isIOS() ? 'touchend' : 'mouseup'


iOSで動作確認した方が良いかもね

cp-20

動作確認してないけど、動くならOKそう

cp-20 · 2025-12-29T15:09:08Z

src/lib/dom/unrefElement.ts

-  return isObjectAndHasKey(plain, '$el') ? plain.$el : plain
+  return (
+    isObjectAndHasKey(plain, '$el') ? plain.$el : plain
+  ) as UnrefElementReturn<T>


アサーションいらないんじゃない？ (普通にアノテーションするだけで十分)

その後 VueUse の unrefElement を利用するように書き換えたんですが、戻り値の型 (HTMLElement | SVGElement | undefined | null) がちょっと微妙だった (.$el の型を infer してほしかった) のでアサーションする形になってしまいました
https://github.com/traPtitech/traQ_S-UI/pull/4890/changes#diff-ad8f25153aa059247c28efc0e1a4b6aaae7d0020fea996985c41c9330ffa341a

微妙なのでアサーションするのは TypeScript の型安全性を破壊する行為なので非常に良くないです　その型が気に食わないなら気に食わない部分を自分でハンドリングする必要があるはずです (型には意味があるはずなので)

色々試した結果結局このような形にすることしかできなかったのですが、この as any を外すことってできるんでしょうか....？(関数オーバーロードも実質的には型アサーションと変わりないですよね)

export function unrefElement<T>( element: MaybeRefOrGetter<T> ): T extends { $el: infer E } ? E : T { const plain = toValue(element) if (isObjectAndHasKey(plain, '$el')) return plain.$el as any return plain as any }

最後の as any はいらないと思うけど、2行目のやつは外すの難しいかも

どっちを外しても Type *** is not assignable to type 'T extends { $el: infer E; } ? E : T' と言われてしまって困ってます

あ、ほんとだ　難しいな

as any するぐらいなら最初からアサーションした方がましだったかも　すいません

やっぱり無理そうだったので諦めました :komata-nya:

uni-kakurenbo changed the title ~~optimize & refactor: 参照カウントと Effect Scope を用いて window に対するイベント監視を効率化する~~ optimize & refactor: 参照カウントと Effect Scope を用いてグローバルなイベント監視を効率化する Oct 28, 2025

uni-kakurenbo force-pushed the optimize/effect-scope branch from 9a99922 to 7d77bf9 Compare October 28, 2025 07:12

uni-kakurenbo changed the title ~~optimize & refactor: 参照カウントと Effect Scope を用いてグローバルなイベント監視を効率化する~~ optimize: 参照カウントと Effect Scope を用いてグローバルなイベント監視を効率化する Oct 28, 2025

uni-kakurenbo self-assigned this Oct 28, 2025

uni-kakurenbo force-pushed the optimize/effect-scope branch 2 times, most recently from 14fa6b5 to 3dd0f11 Compare October 28, 2025 15:21

uni-kakurenbo added refactor optimize performance optimization labels Oct 29, 2025

uni-kakurenbo force-pushed the optimize/effect-scope branch from 3dd0f11 to 62ec88b Compare October 29, 2025 09:48

uni-kakurenbo added this to traQ Client Oct 29, 2025

github-project-automation bot moved this to Inbox in traQ Client Oct 29, 2025

uni-kakurenbo moved this from Inbox to In review in traQ Client Oct 29, 2025

uni-kakurenbo moved this from In review to In progress in traQ Client Oct 29, 2025

uni-kakurenbo force-pushed the optimize/effect-scope branch from 62ec88b to 72295bf Compare October 29, 2025 14:06

feat: 参照カウントによって effectScope を自動で作成/破棄するライブラリを追加

b6cc8c4

uni-kakurenbo force-pushed the optimize/effect-scope branch 3 times, most recently from 64e6323 to 18dcfc4 Compare October 29, 2025 15:03

uni-kakurenbo added 5 commits October 30, 2025 17:31

feat: useEventListener composable を追加

6c549b5

optimize: useInnerWindowSize を createSharedEffectScope を用いて書き換える

415cc97

optimize, refactor: responsive.ts を createSharedEffectScope を用いて書…

5385734

…き換える

refactor: useQallConfirmer を useBeforeUnload を用いて書き換える

b478e85

refactor: 既存の実装を useEventListner を用いて簡略化する

140e7c3

- refactor: `MouseEvent` と `TouchEvent` を併用している部分を `PointerEvent` に統合する

uni-kakurenbo force-pushed the optimize/effect-scope branch from 18dcfc4 to 03cee1d Compare October 30, 2025 10:26

refactor: 既存の実装を unrefElement を用いて簡略化する

142c21a

uni-kakurenbo force-pushed the optimize/effect-scope branch from 03cee1d to 142c21a Compare October 31, 2025 01:15

uni-kakurenbo force-pushed the optimize/effect-scope branch from b1908ed to bcd48e0 Compare December 25, 2025 14:34

uni-kakurenbo force-pushed the optimize/effect-scope branch 3 times, most recently from 10e8153 to 68df132 Compare December 29, 2025 07:37

uni-kakurenbo added 2 commits December 29, 2025 17:13

Merge branch 'master' of https://github.com/traPtitech/traQ_S-UI into…

7006300

… optimize/effect-scope

Merge branch 'master' of https://github.com/traPtitech/traQ_S-UI into…

a061e98

… optimize/effect-scope

uni-kakurenbo force-pushed the optimize/effect-scope branch from 68df132 to a061e98 Compare December 29, 2025 08:13

uni-kakurenbo mentioned this pull request Dec 29, 2025

fix: ドラッグの終了判定を緩和する #4998

Merged

2 tasks

uni-kakurenbo requested review from a team and cp-20 December 29, 2025 10:47

cp-20 approved these changes Dec 29, 2025

View reviewed changes

uni-kakurenbo added 2 commits December 29, 2025 22:18

refactor: 関数オーバーロードの代わりに型アサーションを使う

c2a8671

refactor: MediaQueryList の変数名を改善

9c1e3b7

uni-kakurenbo requested a review from cp-20 December 29, 2025 15:07

uni-kakurenbo changed the title ~~optimize: 参照カウントと Effect Scope を用いてグローバルなイベント監視を効率化する~~ optimize: VueUse を用いてグローバルなイベント監視を効率化する Dec 29, 2025

cp-20 approved these changes Dec 29, 2025

View reviewed changes

uni-kakurenbo force-pushed the optimize/effect-scope branch from 6c3dc38 to 262d84b Compare December 30, 2025 01:25

uni-kakurenbo added 2 commits December 30, 2025 10:32

fix: useEventListener 周りのバグを修正

f3566e9

refactor: VueUse を導入

c0cf29d

uni-kakurenbo force-pushed the optimize/effect-scope branch from 262d84b to c0cf29d Compare December 30, 2025 01:33

uni-kakurenbo merged commit 57d6ef2 into master Dec 30, 2025
11 checks passed

uni-kakurenbo deleted the optimize/effect-scope branch December 30, 2025 01:39

github-project-automation bot moved this from In progress to Done in traQ Client Dec 30, 2025

		// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
		return state!

		const startEventName = isIOS() ? 'touchstart' : 'mousedown'
		const endEventName = isIOS() ? 'touchend' : 'mouseup'

Conversation

uni-kakurenbo commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

なぜこの PR を入れたいのか

動作確認の手順

PR を出す前の確認事項

Uh oh!

github-actions bot commented Oct 28, 2025

Uh oh!

codecov bot commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

cp-20 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cp-20 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

uni-kakurenbo Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

uni-kakurenbo commented Oct 28, 2025 •

edited

Loading

codecov bot commented Oct 28, 2025 •

edited

Loading

uni-kakurenbo Dec 29, 2025 •

edited

Loading