search

SvenKirschbaum / react-stomp-hooks

This repository contain a react library which enables simple access to stomp subscriptions via hooks.
MIT License
59 stars 12 forks source link

Fix scope issue #4

Closed JeffreySoriano5 closed 2 years ago

JeffreySoriano5 commented 2 years ago

Hello, The issue I am having is if I do the following:

useSubscription([`/topic/user-${user.id}-notifications`], onMessage)

If onMessage uses any value that changes, it does not update inside the function. The onMessage function stores the first instance of itself. I see the reason is setting the onMessage function into a useRef. Why was the reason this was done?

This PR should fix this if there is no underlying reason for this

SvenKirschbaum commented 2 years ago

Hello!

Thanks for your contribution! I have used a ref there because I wanted to prevent unsubscribing and resubscribing to the topics on each change of the onMessage function. Implementing your suggested change would cause every change of the function to clean up the effect (unsubscribe) before afterwards running the effect again with the new function (resubscribing), which is in my eyes undesirable.

However, the ref should of course be updated to point to the new function every time the hook runs, to not cause the exact issue you are currently experiencing. The reason that is currently not done is simply an oversight, which happened during the typescript rewrite I had done. In the original commit which implemented this, the ref was updated, while it currently is not.

I therefore will start updating the ref again, and not merge your pull request. Please let me know tough if you disagree with me or have any further feedback!

Sven