Some improvements... #95
Conversation
| /** Return channel */ | ||
| return connectedChannels.current[channelName][0] as T; | ||
| }, | ||
| [connectedChannels, client] |
There was a problem hiding this comment.
probably don't need connectedChannels in dependencies as it's a ref. Why did you opt for a ref rather than useState? Wouldn't useState refresh all the subscribers to get the updated channels?
There was a problem hiding this comment.
I've tried with useState but subscribe and unsubscribe reference changes every time connectedChannels changes, this makes useChannel useEffect to runs infinitely, tried with useCallbacks, useMemos, even using the callback of the setState instead of using the value of the state. Found the most stable option using the ref.
Probably I'm missing something, but couldn't get it work with useState :(
There was a problem hiding this comment.
Also, with useState if some parent context changes, will lose all the connections right?
We'll do now, just pushed a github action that runs the test suite ;) |
|
Let's see if we can get this merged/reviewed soon @mayteio don't want to introduce you hurry, we're using my fork meanwhile, but I'll prefer to use yours :) |
|
Thanks @semoal, sorry I'm not seeing the GitHub action in the checks? |
GitHub only runs actions that are on main/master branch. If you want cherry pick the action to main I can update the branch and the CI will run 👍 They made that cuz the crypto mining, if you haven’t any ci first has to be on main to run on branches, and then when on main if the pull request is open and first contributor, needs approval from you to run 🥵 |
|
Nice, thanks @semoal! Approved and merged. |
|
Hi im still seeing that it's incompatible with react 18... is that the case? or should i use --legacy-peer-deps (i don't like to do that...) $ npm install @harelpls/use-pusher |
|
@mayteio could you publish a new version with these fixes? |
Will fix:
#89 Incompatible with React versions >16
#40 is this hooks automatically disconnect channels?
#43 Introduce to hoist channel connection
#44 useChannel(false) to prevent eager channel connection