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

Use actions on Svelte components #6771

Open
hgiesel opened this issue Sep 25, 2021 · 9 comments
Open

Use actions on Svelte components #6771

hgiesel opened this issue Sep 25, 2021 · 9 comments

Comments

@hgiesel
Copy link
Contributor

hgiesel commented Sep 25, 2021

Describe the problem

There's some things I often find myself doing:

  • dispatching events when component is mounted:
import { onMount, createEventDispatcher } from "svelte";
const dispatch = createEventDispatcher();
onMount(() => dispatch("mount"));
  • dispatching events when destroying:
import { onMount, createEventDispatcher } from "svelte";
const dispatch = createEventDispatcher();
onDestroy(() => dispatch("destroy"));

E.g. I have one component WithShortcut, which exports a let: binding to create a shortcut, e.g. like this:

<WithShortcut shortcut="Control+C" let:createShortcut let:shortcutLabel>
    <Button title={shortcutLabel} on:click={doThing} on:mount={(event) => createShortcut(event.detail.button)}>Click me!</Button>
</WithShortcut>

That's quite the boilerplate, imo, and it leads to code which doesn't feel good, as we're using events, just to be able to listen to the lifecycle event outside of the component. Also, using on: feels more correct, if you're reacting to events, but what we're doing here is extending the component, for which use: would feel better.

There's other issues here. The cleanup for the button happens in Button, but the cleanup code for the shortcut happens in WithShortcut.

Describe the proposed solution

What would be much nicer imo, is if we could use use: actions on components.
The semantics would be similar to HTML elements: the action'smount corresponds to onMount, and destroy corresponds to onDestroy.

This means that the whole thing is basically just syntax sugar. I've built an example here, where I'm recreating a Component use: action with props:

UseAction.svelte REPL

I'll also copy it over here. <MyComponent use:action={param} /> translates to:

// MyComponent.svelte
<script>
  import { onMount } from "svelte";
  import { get_current_component } from "svelte/internal"
  
  export let action;
  export let param;
  
  let elementUpdate;
  
  function update() {
    if (elementUpdate) {
      elementUpdate(param);
    }
  }
  
  $: param, update();
  
  const component = get_current_component();
  
  onMount(() => {
    const { destroy, update } = action(component, param);
    elementUpdate = update;
    return destroy;
  })
</script>

The above code could then be rewritten as (I know {@const is not a thing yet, but it's also a nice feature):

<WithShortcut shortcut="Control+C" let:createShortcut let:shortcutLabel>
    {@const createButtonShortcut = component => createShortcut(component.state.button)}
    <Button title={shortcutLabel} use:createButtonShortcut on:click={doThing}>Click me!</Button>
</WithShortcut>

See here for a REPL of how I wish I could do it.

I believe this makes it more semantic.

This would also be easy to teach new users, as it mirrors how they behave on elements:

<div bind:this={myElement} use:actionOnElement />
<Component bind:this={myComponent} use:actionOnComponent />

(This feature seems quite natural to me, and I was surprised to have not found this feature suggestion. If it has been suggested before, I'd appreciate a link.)

Alternatives considered

  1. As I mentioned above, I currently use onMount(() => dispatch("mount", { button }));. See here for a REPL.

  2. One might say, I could import registerShortcut from Button.svelte and use it there directly, but I don't want to teach every component that might get a shortcut how to create them, and eventually how to assign tooltips (title).

Importance

would make my life easier

@Prinzhorn
Copy link
Contributor

Prinzhorn commented Sep 26, 2021

Why don't you do this:

<WithShortcut shortcut="Control+C" action={doThing} let:shortcutLabel>
    <Button title={shortcutLabel} on:click={doThing}>Click me!</Button>
</WithShortcut>

Sure, it's a tiny bit of duplication, but it works and is easier to understand than the back and forth spaghettiing. Remove duplication by re-exposing the function to the button:

<WithShortcut shortcut="Control+C" action={doThing} let:shortcutLabel let:shortcutAction>
    <Button title={shortcutLabel} on:click={shortcutAction}>Click me!</Button>
</WithShortcut>

This also allows WithShortcut to spice up the function before passing it to the button (e.g. add a confirmation modal or something like that). In both cases <Button> is completely unaware of shortcut and doesn't need anything special.

I think #3617 could also solve this in a different way, e.g. by passing the component back via <Button bind:this={shortcutComponent}> and exposing a click/trigger method on the button component. No more need to have events.

@hgiesel
Copy link
Contributor Author

hgiesel commented Sep 26, 2021

The issue is that shortcut does not execute the function but rather it gets the element and simulates a click: button.click(). This way, the button can also be disabled:

<WithShortcut shortcut="Control+C" let:createShortcut let:shortcutLabel>
    <Button {disabled} title={shortcutLabel} on:click={doThing} on:mount={(event) => createShortcut(event.detail.button)}>Click me!</Button>
</WithShortcut>

I guess I could also pass {disabled} to WithShortcut, but I also want the shortcut to notify the button that it has been pushed, so it can update its button state (e.g. a "Bold" formatting button in an editor will show that the editor is in "Bold" state now).

This is basically a perfect case for use: and it would work fine, if I just inlined button:

<WithShortcut shortcut="Control+C" let:shortcutLabel let:createShortcut>
    <button use:createShortcut [...] />
</WithShortcut>

@Prinzhorn
Copy link
Contributor

so it can update its button state (e.g. a "Bold" formatting button in an editor will show that the editor is in "Bold" state now).

Sounds a lot like manually and imperatively managing state and hoping it's consistent. Shouldn't this happen automatically, e.g. via some state in a store or something?

This is basically a perfect case for use: and it would work fine, if I just inlined button:

What would you need WithShortcut for then? Why not have a single action that you can add to elements you want to react to shortcuts?

I might not see the bigger picture yet, but this seems way more complicated than it should be.

@hgiesel
Copy link
Contributor Author

hgiesel commented Sep 26, 2021

Sounds a lot like manually and imperatively managing state and hoping it's consistent. Shouldn't this happen automatically, e.g. via some state in a store or something?

In fact that's exactly what I am doing. Here's an actual example from our code:

<WithShortcut {shortcut} let:createShortcut let:shortcutLabel>
    <WithState
        {key}
        update={() => document.queryCommandState(key)}
        let:state={active}
        let:updateState
    >
        <IconButton
            tooltip="{tooltip} ({shortcutLabel})"
            {active}
            {disabled}
            on:click={(event) => {
                document.execCommand(key);
                updateState(event);
            }}
            on:mount={(event) => createShortcut(event.detail.button)}
        >
            <slot />
        </IconButton>
    </WithState>
</WithShortcut>

What would you need WithShortcut for then? Why not have a single action that you can add to elements you want to react to shortcuts?

But I don't want to inline the button, because I want to bundle the HTML with styling and functionality. That's why I'm using on:mount (and on:destroy).

@Prinzhorn
Copy link
Contributor

Prinzhorn commented Sep 27, 2021

In fact that's exactly what I am doing. Here's an actual example from our code:

To me this does not look like Svelte at all, but I guess people do things a lot different than I do (we had this discussion in #6255 already, I'd never use a context that way).

From the top of my head I would:

  1. Abstract the whole queryCommandState / execCommand system into a store (or some wrapper that also holds a store) that has state like isBold and canMakeBold and a makeBold() function. It also observes the current cursor/selection and makes sure these things are up-to-date (probably what your WithState.update does). Since the document already is a global state object this would just neatly organize it into the Svelte world (I do that with a lot of things like prefers-color-scheme or document.visibilityState). Once you've done this your entire application is detached from this imperative and arguably ugly DOM interface.
  2. Remove WithState, see 1
  3. Potentially remove WithShortcut, I don't see a point tying it to a button. You could still have a Shortcut component to declaratively add/remove shortcuts. But to me these things are tightly coupled for no reason. What if you later also want a context-menu to make the current selection bold? Do you want to tie all three things to simulating a button click and magically relying on its disabled state?
  4. Now button and shortcut can both independently call editor.makeBold(). It's the job of makeBold to decide if you can make things bold (via canMakeBold), that's not the job of the button[disabled] state preventing the shortcuts el.click() call. That's super backwards to me.
  5. The button disabled state is just disabled={$editor.canMakeBold} and does the usual Svelte magic (calling makeBold makes things bold in the DOM, your store will react to the new document state and your button magically becomes disabled without doing all this manual work)

I know that this has nothing to do with your feature request, but most feature requests I see can be solved by doing things differently. In my experience Svelte components can be very lightweight and independent if abstractions happen at the right place. I've never once had the need to use sth. like WithState, with the exception of maybe some router logic to highlight the current nav item (and I was in a rush and due to tailwind I couldn't just use a simple action with a toggleClass).

I will refrain from hijacking your feature request any further, maybe this was helpful though.

@hgiesel
Copy link
Contributor Author

hgiesel commented Sep 27, 2021

Thanks a lot for your comments, @Prinzhorn, I think I might just have gotten a few good ideas out of this.

However, I am still standing by this idea. This might have been a bad example to argue for it, so here's another go (actually my original motivation, but I thought it was too akward to explain):

We want our Svelte app to be extensible. Doing so is quite the challenge, and having use: actions on components would make it certainly easier.

Here's an example. I have a simple List component displaying some Item components. So we (the app developer) might have our own reasons for showing or hiding some of the items, for which we would change the visible prop. But we also want to provide a way for add-on devs to dynamically show or hide other names, which is why we want to expose APIs alongside the components, that can be used from outside. In the REPL I put this code into setTimeout.

(This would also require that you can define inline :use actions)

@Prinzhorn
Copy link
Contributor

Prinzhorn commented Sep 27, 2021

I think this sounds more like mixins than actions 🤔, not sure what others are doing in this space (you can get pretty far with <svelte:component> and passing component constructors around and spicing up props along the way).

We want our Svelte app to be extensible. Doing so is quite the challenge, and having use: actions on components would make it certainly easier.

Here's an example. I have a simple List component displaying some Item components. So we (the app developer) might have our own reasons for showing or hiding some of the items, for which we would change the visible prop. But we also want to provide a way for add-on devs to dynamically show or hide other names, which is why we want to expose APIs alongside the components, that can be used from outside. In the REPL I put this code into setTimeout.

Now this is getting too complex for discussing this here and these type of architectural decisions require more in depth understanding than what I can provide here. I will say this: I personally would think of the extensions in terms of data, not in terms of UI elements. My UIs are data/store driven. The UI is just a way to visualize the data. Your data could flow through all of of the extensions and the extensions can make decisions (e.g. setting visible to false). Like middlewares in a Connect/Express/Polka app. And the UI doesn't even know about all this, it just updates with the current state and makes sure it's consistent.

@hgiesel
Copy link
Contributor Author

hgiesel commented Sep 29, 2021

I think this sounds more like mixins than actions.

I always thought of actions as mixins. Even the actions used in the tutorial could be aptly described as Pannable and LongPressable.

you can get pretty far with svelte:component and passing component constructors around and spicing up props along the way

Yep, we experimented with this, but what we found is that you loose most of Svelte's niceness like slots, and instead you'll pass around deeply nested objects.

@Phaqui
Copy link
Contributor

Phaqui commented Mar 20, 2022

Guess I'm late to the party, but I have another use case.

I made an action that can be applied to input and textarea elements, and listens for input events on them. The action basically just starts a setTimeout when that happens, and when the timer fires, a method that the user submitted when making the action would be called. A very simplified demonstration of what it basically does:

<script>
  const { autosave } = make_autosaver(my_save_fn);

  async function my_save_fn() {
    console.log("autosaver told us to save now");
    // save to local/session storage, call an api, do whatever...
  }
</script>

<input type="text" use:autosave>

Now, I may want to use this on components that are wrapper- or custom built components that mimics native ones (Button wrappers, custom SVG checkboxes, or whatever). So how would my action know what to do with any given Component that I might use it on? In general it can't know what to do - but what it could do, is say that the action requires these and these behaviors of the Component. Maybe for the action to make sense to use on a component, it requires that the Component dispatches custom input events (with the data set to whatever data makes sense - could be a boolean in the case of a checkbox, or a Date in the case of a date picker). I don't think it's any more work for the author of the action to define the interface for Components, than it is to determine the type of and handle support for all the various kinds of native elements that the action could also be applied to.

I envision just passing the component (the object you get when you bind:this to a component) as the first parameter to the action, instead of the dom element (which is the case for native DOM elements). But that's just an idea, of course.

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

No branches or pull requests

3 participants