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

provide useSocket() function #520

Open
wants to merge 6 commits into
base: alpha
Choose a base branch
from
Open

provide useSocket() function #520

wants to merge 6 commits into from

Conversation

e-tobi
Copy link

@e-tobi e-tobi commented Mar 1, 2021

This can be used in Vue's setup() function to inject the socket instance
where $socket is not available:

setup() {
const socket = useSocket()
}

Currently this breaks some tests - I will look into this later.

This can be used in Vue's setup() function to inject the socket instance
where $socket is not available:

setup() {
  const socket = useSocket()
}
The plugin now provides a default and named exports. So the tests now
must explicitly test the default export.
@codecov
Copy link

codecov bot commented Mar 1, 2021

Codecov Report

Merging #520 (5ccd96f) into alpha (df83685) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             alpha      #520   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            9        10    +1     
  Lines          124       131    +7     
  Branches        20        20           
=========================================
+ Hits           124       131    +7     
Impacted Files Coverage Δ
src/composables.js 100.00% <100.00%> (ø)
src/plugin.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df83685...5ccd96f. Read the comment docs.

@e-tobi
Copy link
Author

e-tobi commented Mar 2, 2021

I'm going to add another composable:

setup() {
  onSocketEvent("my-event', (data) => {
     ...
  }
}

...so you might want to wait before pulling

@e-tobi
Copy link
Author

e-tobi commented Mar 2, 2021

onSocketEvent() is now working too and I have also added this to the README.md.

@probil
Copy link
Owner

probil commented Mar 2, 2021

Wow! That looks nice. 👍 Will check it on weekend and merge

FYI I am thinking about sunsetting the $subscribe/$unsubscribe API in v5 in favour of directly socket usage. The main reason for this is the fact that it's not working properly with options api after component mount/unmount anyway #431 It will probably be available internally so your approach will definitely work but the mention of $subscribe/$unsubscribe in docs might be redundant. Thanks anyway!

BTW, you don't have to commit dist folder - it's supposed to be generated by CI later in the process

@e-tobi
Copy link
Author

e-tobi commented Mar 2, 2021

Sorry, missed that dist slipped in with the last commit. I've fixed it. Might be a good idea to add dist to .gitignore.

This will subscribe to a socket.io event before the component is mounted
and will unsubscribe before the component is unmounted.

Usage:

setup() {
  onSocketEvent('my-event', (data) => {
      ...
    })
}
@e-tobi
Copy link
Author

e-tobi commented Mar 2, 2021

Ok - I've added dist to .gitignore so I can't make the same mistake again.

@e-tobi
Copy link
Author

e-tobi commented Mar 2, 2021

Mmm... it still doesn't work like it should. In my little toy project unsubscribing fails, because removeListenersByLabel doesn't detect the unsubscribe coming from the same vm as the subscribe.
I'll try to sort this out tomorrow.

@probil
Copy link
Owner

probil commented Mar 22, 2021

@e-tobi Any updates on this? I'll try to take a look at removeListenersByLabel maybe there is some internal changes in Vue 3

BTW thanks for spotting dist/ missing in .gitignore. I thought it was there. Backported to the master branch.

@alew3
Copy link

alew3 commented Apr 27, 2021

@e-tobi what's the best way for me to try out your PR? I can't find it in the 5.0.0-alpha.4

@sQuarecoW
Copy link

Is there any movement on this?

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.

4 participants