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

7fc1d77 以降に更新するとタイムラインが正常に表示・操作できなくなる #13219

Closed
1 task
okinjp opened this issue Feb 9, 2024 · 24 comments · Fixed by #13248
Closed
1 task
Labels
🐛Bug Unexpected behavior 🔥high priority packages/frontend Client side specific issue/PR

Comments

@okinjp
Copy link

okinjp commented Feb 9, 2024

💡 Summary

7fc1d77 以降に更新するとタイムラインが正常に表示・操作できなくなる

🥰 Expected Behavior

すべてがうまくいく

🤬 Actual Behavior

TLが表示できなくなりTL切り替えやページ遷移なども作動せずコンソールにエラーが残る
ユーザーページや個別投稿のURLを直接開くことは可能

コンソールログ
a1.miclear.casa-1707482555884.log

_.Ver.A1.37.-.-.Microsoft_.Edge.2024-02-09.mp4

📝 Steps to Reproduce

  1. 7fc1d77 に更新する
  2. TLが空になりまともに操作できなくなる

💻 Frontend Environment

* Model and OS of the device(s): Windows 11 23H2
* Browser: Microsoft Edge 121.0.2277.83 (公式ビルド) (64 ビット)
* Server URL: https://a1.miclear.casa
* Misskey: 2024.2.0-beta.10 (7fc1d77893631f7bc792c7d6d5398aba65a331c9)

🛰 Backend Environment (for server admin)

* Installation Method or Hosting Service: docker compose
* Misskey: 2024.2.0-beta.10 (7fc1d77893631f7bc792c7d6d5398aba65a331c9)
* Node: 20.10.0
* PostgreSQL: 15
* Redis: 7
* OS and Architecture: Ubuntu 22.04.3 LTS amd64

Do you want to address this bug yourself?

  • Yes, I will patch the bug myself and send a pull request
@okinjp okinjp added the ⚠️bug? This might be a bug label Feb 9, 2024
@kakkokari-gtyih
Copy link
Contributor

なんか無限ループが走ってそう

@tamaina
Copy link
Member

tamaina commented Feb 9, 2024

too much recursion (firefox

image

@tamaina
Copy link
Member

tamaina commented Feb 9, 2024

vue 3.4.16以降がだめ

@tamaina tamaina added 🐛Bug Unexpected behavior packages/frontend Client side specific issue/PR and removed ⚠️bug? This might be a bug labels Feb 9, 2024
@tamaina
Copy link
Member

tamaina commented Feb 9, 2024

vuejs/core#10214 / vuejs/core#10232 に関係がありそう

@tamaina
Copy link
Member

tamaina commented Feb 9, 2024

Vue Landに投げてみた(

https://discord.com/channels/325477692906536972/1205559733101010984

@tamaina
Copy link
Member

tamaina commented Feb 9, 2024

  • Vueのバージョンを下げれば動きはするが、今後一切vueのバージョンが上げられないのは相当まずいのでこの問題は後回しにできない
  • 私はVueの内部構造に詳しいわけではないので何が起きているのかわからない(→解決策が出せない
    • (読み解けばいい話だけど私が読むより詳しい人に聞いた方が早そう)

@tamaina
Copy link
Member

tamaina commented Feb 10, 2024

tasukete

@syuilo
Copy link
Member

syuilo commented Feb 10, 2024

3.4.15にするか

miyakogi added a commit to miyakogi/misskey that referenced this issue Feb 11, 2024
@kakkokari-gtyih
Copy link
Contributor

withRepliesonlyFilesが悪さしてそう

@kakkokari-gtyih
Copy link
Contributor

kakkokari-gtyih commented Feb 11, 2024

image

computed内で別のcomputedの値をお互いに参照し合うと無限ループに入るっぽい(ここを消したら普通に動いた)

@syuilo
Copy link
Member

syuilo commented Feb 11, 2024

あーそれはアカンわね

@kakkokari-gtyih
Copy link
Contributor

なんで今まで動いてたんだろう()

@syuilo
Copy link
Member

syuilo commented Feb 11, 2024

むしろなぜ今まで動いてたのか

@kakkokari-gtyih
Copy link
Contributor

じゃあどう対応すればいいのかしら(これ構造的にお互いに影響し合う値なので…)(watch使うとか?)

@tamaina
Copy link
Member

tamaina commented Feb 11, 2024

computedじゃなくてただのfunctionを作って見れば良いんじゃないかしら

@kakkokari-gtyih
Copy link
Contributor

kakkokari-gtyih commented Feb 11, 2024

// computed内での無限ループを防ぐためのフラグ
const localSocialTLFilterSwitchStore = ref<'withReplies'|'onlyFiles'|false>('withReplies');

const withReplies = computed<boolean>({
	get: () => {
		if (!$i) return false;
		if (['local', 'social'].includes(src.value) && localSocialTLFilterSwitchStore.value === 'onlyFiles') {
			return false;
		} else {
			return defaultStore.reactiveState.tl.value.filter.withReplies;
		}
	},
	set: (x: boolean) => saveTlFilter('withReplies', x),
});
const onlyFiles = computed<boolean>({
	get: () => {
		if (['local', 'social'].includes(src.value) && localSocialTLFilterSwitchStore.value === 'withReplies') {
			return false;
		} else {
			return defaultStore.reactiveState.tl.value.filter.onlyFiles;
		}
	},
	set: (x: boolean) => saveTlFilter('onlyFiles', x),
});

watch([withReplies, onlyFiles], ([withRepliesTo, onlyFilesTo]) => {
	if (withRepliesTo) {
		localSocialTLFilterSwitchStore.value = 'withReplies';
	} else if (onlyFilesTo) {
		localSocialTLFilterSwitchStore.value = 'onlyFiles';
	} else {
		localSocialTLFilterSwitchStore.value = false;
	}
});

これでいけた

@tamaina
Copy link
Member

tamaina commented Feb 11, 2024

↑functionが無限ループするだけだわ

@syuilo
Copy link
Member

syuilo commented Feb 11, 2024

じゃあどう対応すればいいのかしら(これ構造的にお互いに影響し合う値なので…)(watch使うとか?)

そのような構造にしないのが正攻法

@kakkokari-gtyih
Copy link
Contributor

kakkokari-gtyih commented Feb 11, 2024

これでいけた

(#13219 (comment)) の方法はどう?

@tamaina
Copy link
Member

tamaina commented Feb 11, 2024

{ withReplies: boolean; onlyFiles: boolean; }を吐き出す関数を作る方がスマートな気がした

@tamaina
Copy link
Member

tamaina commented Feb 11, 2024

(もしくはそういうcomputedにしちゃうとか

@kakkokari-gtyih
Copy link
Contributor

kakkokari-gtyih commented Feb 11, 2024

{ withReplies: boolean; onlyFiles: boolean; }を吐き出す関数を作る方がスマートな気がした

なるほど(リアクティブにやってくれるのかしら)

(もしくはそういうcomputedにしちゃうとか

これはMkPopupMenuから値のセットができなくなる(そのために煩雑な処理を追加する必要がありそう)のであかんかも

@kakkokari-gtyih
Copy link
Contributor

とりあえずなおした #13248

@tamaina
Copy link
Member

tamaina commented Feb 11, 2024

これはMkPopupMenuから値のセットができなくなる(そのために煩雑な処理を追加する必要がありそう)のであかんかも

(MkSwitch、popupが呼ばれるごとに新しくref/computedを作るのがベターではあったりする)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛Bug Unexpected behavior 🔥high priority packages/frontend Client side specific issue/PR
Projects
Status: Done
4 participants