gloo icon indicating copy to clipboard operation
gloo copied to clipboard

fix(gloo-net): remove bug-abling EventSource Clone implementation

Open vpochapuis opened this issue 1 year ago • 1 comments

PR Content

Remove the Clone derive/impl for the EventSource .

Reasoning

After discussing https://github.com/rustwasm/gloo/discussions/409, it seems that the implementation of Clone for EventSource is a bug.

To summarize the situation:

EventSource implements Drop and will disconnect the internal connection upon triggering this mechanism. However, when cloning this structure, if any instance of EventSource is going out of scope, all of the instances will have their internal connection disconnected.

An example of a situation where this can be unexpected to the user and not straight forward:

  • Using yewdux to manage a mutable global state. yewdux's state will need to have the structs field that impl Clone. Hence here using EventSource in its previous version, would be allowed, without any additional wrapper. However, internally yewdux clone's and drops the state, hence destroying the precious connection and rendering the EventSource virtually unusable, but without any explanation to the user.

vpochapuis avatar Dec 01 '23 16:12 vpochapuis

Thanks for the PR! Can you also update CHANGELOG.md accordingly

I will do my best to follow what is in CONTRIBUTING.md however I do not understand well how it works, as the CHANGELOG.md says that net is at version 0.5.0 while the Cargo.toml of net is at 0.4.0.

Please let me know if this is correct and how I could improve for the next time.

Thank you!

vpochapuis avatar Dec 02 '23 02:12 vpochapuis