Skip to content

fix: remove awareness stats on page unload instead of beforeunload#658

Merged
janthurau merged 1 commit intoueberdosis:mainfrom
iowxy:main
Jul 29, 2023
Merged

fix: remove awareness stats on page unload instead of beforeunload#658
janthurau merged 1 commit intoueberdosis:mainfrom
iowxy:main

Conversation

@iowxy
Copy link
Contributor

@iowxy iowxy commented Jul 25, 2023

I notice the code in file packages/provider/src/HocuspocusProvider.ts

window.addEventListener('beforeunload', this.boundBeforeUnload)

I must say that what if I prevent the page to be close? consider this logic below

// https://developer.mozilla.org/en/docs/Web/API/Window/beforeunload_event
window.addEventListener('beforeunload', (event) => {
  event.preventDefault()
  event.returnValue = ''
})

Develop can write intercept logic and users may cancel to close this page.

Copy link
Contributor

@janthurau janthurau left a comment

Choose a reason for hiding this comment

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

Totally makes sense. Thanks for this!

I guess we added beforeUnload to make sure the awareness message can actually arrive at the server, but seems this works fine (yjs/y-websocket#119).

@janthurau janthurau merged commit e1bd3f1 into ueberdosis:main Jul 29, 2023
janthurau pushed a commit to YousefED/hocuspocus that referenced this pull request Aug 12, 2023
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.

2 participants