Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Broadcast custom Events on IPC #367

Merged
merged 3 commits into from
Sep 10, 2024
Merged

Conversation

gwleuverink
Copy link
Contributor

@gwleuverink gwleuverink commented Sep 10, 2024

Summary

This PR implements a api/broadcasting endpoint to handle all custom events dispatched on the nativephp channel. (See #295 (comment))

Whenever a Laravel event is dispatched on the nativephp channel, a native-event event will be dispached on IPC so we can both listen to these events on the frontend using ipcRenderer.on('native-event') and using Livewire listeners without using a separate websocket server.

Please also check out NativePHP/electron-plugin#37 which implements the endpoint in the Electron backend

For example, whenever the following event is dispatched:

class FooBar
{
    use Dispatchable;

    public $foo = 'Bar';

    public function broadcastOn(): array
    {
        return [
            new Channel('nativephp'),
        ];
    }
}

You may listen to it within Livewire using a listener:

#[On('native:\App\Events\FooBar')]
public function listenOnIpc(FooBar $event)
{
    // dd($event);
}

Or directly int the frontend using the ipcRenderer

<!-- 
Assuming you have ipcRenderer attached to the window scope in your bundle:
window.ipcRenderer = require('electron').ipcRenderer
-->

<script>
    window.ipcRenderer.on('native-event', (native, { event, payload }) => {

        console.dir(event)
        console.dir(payload)
    })
</script>

For your consideration

  1. I'm using the existing native-event so all events are automatically forwarded to Livewire. I'm unsure about the naming of this, strictly speaking this isn't a Native event, but rather a custom one.

  2. I've looked around for a existing path to testing this, but it seems similar features (like the debug endpoint) aren't covered. If you wan't me to cover this PR with tests I'll have to do some more digging. I'm not very experienced with jest & TS

  3. While exploring the codebase I found an existing notifyLaravel function in the electron plugins' utils.ts. It looks like this function doesn't emit events to any menubar window like was recently added to the debug enpoint. Should I create a separate issue for this?

Closing

I've got all this working locally without issues. If you'd like me to change anything please let me know and I'll update this asap 👍🏻

@simonhamp
Copy link
Member

This is great! Regarding your questions:

I'm using the existing native-event so all events are automatically forwarded to Livewire. I'm unsure about the naming of this, strictly speaking this isn't a Native event, but rather a custom one.

This is fine for now. If it becomes apparent that we should split them out, we can.

I've looked around for a existing path to testing this, but it seems similar features (like the debug endpoint) aren't covered. If you wan't me to cover this PR with tests I'll have to do some more digging. I'm not very experienced with jest & TS

This specific PR (on the Laravel side) could do with a simple test that just asserts that the correct client endpoint will be called when the event is the right type and not when it isn't. I don't think you'd need to dig into Jest etc...

That said, I'm happy with this as-is... I plan to revisit tests closer to or even after v1.

While exploring the codebase I found an existing notifyLaravel function in the electron plugins' utils.ts. It looks like this function doesn't emit events to any menubar window like was recently added to the debug enpoint. Should I create a separate issue for this?

Yes please. I think that makes sense for them all to be consistent.

@simonhamp simonhamp merged commit 91afa37 into NativePHP:main Sep 10, 2024
21 checks passed
@gwleuverink
Copy link
Contributor Author

Great! I'll also add it to the broadcasting docs this evening 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants