laravel / reverb

Laravel Reverb provides a real-time WebSocket communication backend for Laravel applications.
https://reverb.laravel.com
MIT License
942 stars 63 forks source link

Enhance library for extensibility #220

Closed vesko2624 closed 1 week ago

vesko2624 commented 1 week ago

I respect that you want to use every design pattern on the planet, but it is a pain in the ass. If i want to extend your package in any way i need to overwrite half of it.

I want to log if a connection is closed due to message being too large, the only way I can think of is as follows:

Overwrite the ReverbInstallCommand so that I can overwrite the Server Factory so I can overwrite the Router so that I can overwrite the attemptUpgrade method in order to overwrite the ReverbConnection so I can overwrite the control method. Seems simple enough doesn't it. And if for some reason you change just a tiny bit of these 4 classes I have named above, I will have to go though my implementation and rewrite it. I know that extending a package comes with a maintenance, but cmon, that could've been much easier if reverb connection is not created with new. That server factory could've easily been a singleton which is managed by the app. Router could have easily been bound with the app so that i can change it with my implementation.

There might be a better way, just haven't found it yet. Been looking for over an hour for this. I realize that the problem you are trying to solve is not that simple, and the package is still in beta. But nevertheless I believe it is quite overengineered and has limited extension points.

Excuse me for the angry review, just a bit frustrated, if there is a better way for doing this please let me know.

Cheers, vgeorgiev :)

driesvints commented 1 week ago

You're free to fork the package and make any adjustments you want 👍