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

fix(frontend): 通知が重複して受信される問題を修正 #13531

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

kakkokari-gtyih
Copy link
Contributor

@kakkokari-gtyih kakkokari-gtyih commented Mar 6, 2024

What

Partially cherry-picked from MisskeyIO#504

Why

Fix #13517
治るかはわからない

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

@github-actions github-actions bot added the packages/frontend Client side specific issue/PR label Mar 6, 2024
Comment on lines +93 to +99
if (!connection) {
connection = useStream().useChannel('main');
if (connection) {
connection.on('notification', onNotification);
connection.on('notificationFlushed', reload);
}
}
Copy link
Contributor Author

@kakkokari-gtyih kakkokari-gtyih Mar 6, 2024

Choose a reason for hiding this comment

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

deactivatedされた後に通信が復活しないのもなんかアレな気がしたのでこれは型定義を良さそうな感じにした上で復活した

Copy link
Contributor Author

Choose a reason for hiding this comment

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

connectionの詳細な仕様をよくわかってないので必要無い説はある

Comment on lines +60 to 70
function onNotification(notification: Misskey.entities.Notification) {
const isMuted = props.excludeTypes ? props.excludeTypes.includes(notification.type) : false;
const isAlreadyShown = pagingComponent.value?.items.has(notification.id);
if (isMuted || document.visibilityState === 'visible') {
useStream().send('readNotification');
}

if (!isMuted) {
if (!isMuted && !isAlreadyShown) {
pagingComponent.value?.prepend(notification);
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

その上でUIに流すかどうかをリアルタイム受信時に確認させるようにした

Copy link

codecov bot commented Mar 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.67%. Comparing base (08d618b) to head (7097dbe).

Additional details and impacted files
@@             Coverage Diff              @@
##           develop   #13531       +/-   ##
============================================
+ Coverage    64.69%   77.67%   +12.97%     
============================================
  Files          986      185      -801     
  Lines       110809    25028    -85781     
  Branches      5692      484     -5208     
============================================
- Hits         71691    19441    -52250     
+ Misses       37682     5580    -32102     
+ Partials      1436        7     -1429     

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

@kakkokari-gtyih
Copy link
Contributor Author

これコネクションを捨てるんではなく単にリスナーを外すだけにしたほうか良いやつ?

connection = useStream().useChannel('main');
connection.on('notification', onNotification);
connection.on('notificationFlushed', reload);
if (!connection) {
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
if (!connection) {
if (connection == null) {

});

onActivated(() => {
if (!connection) {
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
if (!connection) {
if (connection == null) {

connection.on('notificationFlushed', reload);
if (!connection) {
connection = useStream().useChannel('main');
if (connection) {
Copy link
Member

Choose a reason for hiding this comment

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

connectionが無いことはなさそう

});

onActivated(() => {
if (!connection) {
connection = useStream().useChannel('main');
if (connection) {
Copy link
Member

Choose a reason for hiding this comment

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

connectionが無いことはなさそう

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/frontend Client side specific issue/PR
Projects
Development

Successfully merging this pull request may close these issues.

通知が増殖する
3 participants