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

feat: Add 'connecting' event #34

Merged
merged 1 commit into from Oct 18, 2020
Merged

feat: Add 'connecting' event #34

merged 1 commit into from Oct 18, 2020

Conversation

tommoor
Copy link
Contributor

@tommoor tommoor commented Oct 15, 2020

As described in the comment here, a "connecting" event is useful for monkey-patching the authentication process in lieu of a more official API.

Seems like this is a no-brainer addition even if a more official auth API gets built on the client

Huly®: YJS-781

As described in the [comment here](yjs#7 (comment)), a "connecting" event is useful for monkey-patching the authentication process in lieu of a more official API.

Seems like this is a no-brainer addition.
@dmonad dmonad merged commit 82e0344 into yjs:master Oct 18, 2020
@tommoor
Copy link
Contributor Author

tommoor commented Oct 21, 2020

Could we get a release with this and the other couple of fixes? 🥺

@tommoor
Copy link
Contributor Author

tommoor commented Oct 29, 2020

I've published a fork for anyone that wants this functionality: @tommoor/y-websocket@1.3.6

@prosperi
Copy link

I tried to replicate authentication flow that is described here. I am trying to monkey patch onmessage and onopen functions. It works fine for the initial connection, but on a server crash it has a problem with reconnection. I believe since the connecting status gets emitted before onmessage and onopen initialization, the connecting event handler gets called with provider.ws having onmessage and onopen set to undefined. Seems to work fine if I were to move emit part at the end of setupWS.

@tommoor
Copy link
Contributor Author

tommoor commented Nov 18, 2020

Yep, I had the same thing – there's a PR in for it here:
#38

I published the fix to my fork already v1.3.7.

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.

None yet

3 participants