[Feature Request] React: Provide event name in useEcho callback#432
[Feature Request] React: Provide event name in useEcho callback#432toitzi wants to merge 1 commit intolaravel:2.xfrom
Conversation
e390264 to
b019bda
Compare
|
I started down this path while working on this hooks and it got a bit messy, I realized that I would probably just set up multiple hooks if the callback behavior is different. Wouldn't this be cleaner? useEcho(
"channel",
["UserCreated", "UserUpdated"],
(payload, event) => {
// Same action regardless of the event
},
);
useEcho(
"channel",
"UserDeleted",
(payload, event) => {
// Different action for this event
},
); |
|
Yes, using multiple hooks is definitely possible, of course. Personally, I’d prefer having support for both approaches, but that might just be a matter of taste. In my current project, I’m working on an app that makes fairly heavy use of WebSockets for real-time collaboration. I found it cleaner to wrap the useEcho call and extend it with some custom state and event bindings—mainly to track connection status and simplify UI logic for things like showing connection issues or offering a reconnect option. Doing this across multiple hooks would get a bit redundant, so having a central mechanism has worked better for me. That said, I’m not entirely sure if I’m veering into a bad design pattern here—it probably just comes down to individual use cases and preferences. I’d love to see this implemented, but totally understand if you decide not to include it. |
Signed-off-by: Tobias Oitzinger <tobiasoitzinger@gmail.com>
|
I can think of a scenario which it makes sense to just reuse one instance of |
|
This feature request makes a lot of sense when you think of private channels. Each call to useEcho with private visibility will send out an HTTP request to the server for authentication. So right now the options are:
Here is some real code we have in our codebase: function SiteTagEventsListener({ site, children }: { site: Site, children: ReactNode }) {
// We listen for tag updates on a per-site basis because users aren't always subscribed to all sites in a job contract,
// and we want to avoid receiving events for tags that don't belong to sites we don't have access to.
useEcho<TagUpdated | GatewayValueWritten | GatewayValueWriteErrored>(
`site.${site.id}`,
['.App\\Events\\TagUpdated', '.App\\Events\\GatewayValueWritten', '.App\\Events\\GatewayValueWriteErrored'],
(event) => {
switch (event.type) {
case 'App\\Events\\TagUpdated':
normalizedTagEventStore.setState(prev => {
console.debug('TagUpdated event received', event);
return { ...prev, [event.tag.id]: event };
});
break;
case 'App\\Events\\GatewayValueWritten':
case 'App\\Events\\GatewayValueWriteErrored':
gatewayValueEventStore.setState(prev => ({ ...prev, [event.requestID]: event }));
break;
}
},
[site],
'private',
);
return children;
}In the meantime, what I've had to do is manually put a "type" field in each payload. |
Currently, the
useEchohook allows subscribing to multiple events with a single callback:While functional, this makes it impossible to determine which specific event was triggered, especially when handling multiple events that share similar payload structures.
This PR enhances the useEcho hook by passing the event name as a second argument to the callback:
Benefits to end users
Enables precise event handling when listening to multiple events.
Reduces the need for workarounds such as including the event name in the payload manually.
Backward compatibility
The change is backward-compatible: existing usage with a single payload parameter remains functional.
Additional data (event) is non-breaking and optional for the callback.
NOTE: I had to adapt the tests since the registered function is now anonymous. Verifying that the original mockCallback was passed directly no longer works as before. I'm not entirely sure how you'd prefer this to be tested—if you have a specific approach in mind, I'm happy to implement it accordingly. I can also add support for this feature in Vue if you think this is something you would accept as a feature.