woutdp / live_svelte

Svelte inside Phoenix LiveView with seamless end-to-end reactivity
https://hexdocs.pm/live_svelte
MIT License
1.01k stars 38 forks source link

Remove knowledge of LiveView from Svelte components #31

Closed markevans closed 11 months ago

markevans commented 1 year ago

Hi there

This is great - thanks for this - I was actually going to create something similar but no need now - what you've done is great.

One thing though - I think it would be better if the Svelte components had no idea they were in a LiveView app - currently you're using

<button class="plus" phx-click="increment">
    +1
</button>

from inside the Svelte component, i.e. the Svelte component uses phx-click attribute, which seems wrong to me.

I think the better solution would be for the component to dispatch an event, e.g.

<script>
    import { createEventDispatcher } from 'svelte';

    const dispatch = createEventDispatcher();

    function handleClick() {
        dispatch('increment');
    }
</script>

<button class="plus" on:click={handleClick}>
    +1
</button>

or just using event forwarding,

and for your LiveSvelte.render component to also take an events argument or something, which in turn send a message to LiveView, e.g.

<LiveSvelte.render name="SimpleCounter" props={%{number: @number}} events={%{increment: "increment"}} />

One slightly tricky thing is that ideally you'd be able to map the payload between what gets emitted ----> what you send to the LiveView handler - there would be various approaches for this - I guess sending event.detail wouldn't be a bad default.

Anyway, this way your Svelte components would have no knowledge of LiveView/Phoenix and you could more easily import third-party components, etc.

Just an idea - what do you think?

gevera commented 1 year ago

I see where you coming from. Making it more vanilla Svelte. Easy switching from existing projects and importing third party libs.

woutdp commented 1 year ago

We have something similar already with pushEvent, an example of that is here: https://github.com/woutdp/live_svelte#create-a-svelte-component

<script>
    export let number = 1
    export let pushEvent

    function increase() {
        pushEvent("set_number", {number: number + 1}, () => {})
    }

    function decrease() {
        pushEvent("set_number", {number: number - 1}, () => {})
    }
</script>

<p>The number is {number}</p>
<button on:click={increase}>+</button>
<button on:click={decrease}>-</button>

Note that you can also just do pusEvent("set_number") in case you don't have a value. The last parameter is an optional callback.

So instead of phx-click you can use pushEvent and it would work in similar ways as you outlined, would that be sufficient for this usecase?

gevera commented 1 year ago

I believe @markevans meant that instead of using "custom" pushEvent function in Svelte components, it might be more intuitive for Svelte folks to use Svelte's own dispatch from createEventDispatcher. Not sure if Its even possible. Here is an example https://svelte.dev/tutorial/component-events

woutdp commented 1 year ago

@gevera Hmm I see, it's an interesting idea. I'd definitely want to keep the current functionality with pushEvent and phx-click working, but I wouldn't be opposed to also support dispatch if there's a demand for it, although it's hard for me to see what the benefit over pushEvent would be. I understand it's more inline with the Svelte way of doing things, but I think dispatch is more of a client side thing where events bubble up to other components, instead of sending an event over a websocket, I think those 2 are different. But please correct me if I'm wrong!

gevera commented 1 year ago

Agreed. So for clarification:

markevans commented 1 year ago

hi - sorry for delay getting back - yes as @gevera mentioned I was thinking you'd push the event to live view outside the Svelte component (pushEvent is cool but it still requires the svelte component to implicitly know that it's in a Phoenix app)

One way of thinking about it is that components can (should) be "black-boxes" with "props-in, events-out" as their interface.

So just as in Svelte you'd use a component by passing props in and getting events out:

<EditUserForm user={user} on:submit={handleSubmit} />

the same component could be used in a "props-in, events-out" fashion by LiveSvelte.render (imagining the LiveView has a def handle_event("submit_user", ...)):

<LiveSvelte.render name="EditUserForm" props={%{user: @user}} events={%{submit: "submit_user"}} />

where the slight difference is that instead of passing a javascript function handleSubmit as the "submit" handler, you're passing the name of the message ("submit_user" in this case) that will be sent to the LiveView with pushEvent.

The slightly tricky thing is that you haven't specified here how to map the payload, so you either need to send the whole JS event object (yuck - LiveView shouldn't care about JS event objects) or maybe event.detail.

Another thing you could do, which I'm sure someone will point out has some security implications or something, would be to use a function that yields both the event and the pushEvent function from the phoenix hook like so:

<LiveSvelte.render name="EditUserForm" props={%{user: @user}} events={%{submit: "(event, pushEvent) => pushEvent('submit_user', event.detail)"}} />

That way you'd have proper control over how the Svelte event gets mapped to the LiveView handler.

Sorry I've not had time to look into your code so all these suggestions are what it would look like only without suggesting how it might be done(!) but happy to contribute when I have time.

Basically the whole point in this is that the Svelte components have no knowledge about Phoenix - they just bubble events up in the usual way and LiveSvelte.render handles those events and turns them into push events to the LiveView. That way you could reuse any 3rd-party components simply with LiveSvelte.render rather than creating a Svelte component of your own to wrap them and push to LiveView. However pragmatically-speaking maybe just having the top-level Svelte component know about Phoenix is not so bad if the rest are pure Svelte. All just food for thought so if it's too much of a pain to incorporate that's cool

woutdp commented 1 year ago

That makes sense actually. I'm not sure about the mapping if a client side event should map to a server side push, but it's not that bad imo. I'd happily merge PR's that add this feature

dev-guy commented 1 year ago

It would be great to be able to copy vanilla Svelte examples. There are other gotchas with