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

Events in spread props #5112

Closed
fbasso opened this issue Jul 8, 2020 · 21 comments
Closed

Events in spread props #5112

fbasso opened this issue Jul 8, 2020 · 21 comments

Comments

@fbasso
Copy link

fbasso commented Jul 8, 2020

Is your feature request related to a problem? Please describe.
At the moment, there is no way to manage component events for a dynamic component, with spread props. Let consider this example:

<script>
	const props = {
		'label': 'Click me'
		'on:action': () => {}
	}
</script>
<svelte:component this={Comp} {...props} />

on:action will not be called.

REPL here

Describe the solution you'd like
To have the code above working 😊

Describe alternatives you've considered
The workaround is to consider callbacks instead of events, like this :

<script>
	const props = {
		'label': 'Click me'
		'onAction': () => {}
	}
</script>
<svelte:component this={Comp} {...props} />

But then the code in Comp must be different compared to a normal dispatch usage.

How important is this feature to you?
It's just annoying because the workaround works pretty well, but maybe it can be a useful feature to keep consistency with the spread props.

@antony
Copy link
Member

antony commented Jul 10, 2020

In cases like this I fire a single known event name such as event and then use the event's detail to hold the inner event name and properties.

I'm pretty sure what you're suggesting isn't possible, since the expectation would be that events could be computed at runtime and appended to components, but I'm not 100% sure of that.

@fbasso
Copy link
Author

fbasso commented Jul 16, 2020

Thanks for your answer @antony, and sorry for the late answer here !

Using a generic event is another way to do it, indeed. I didn't think about this solution.

So it's definitely not a blocking issue, but sure, if there is way to standardize the way dynamic components are used (compared to a static one), it would be a plus.

The on:myevent set as a prop seems the more natural way to do it, and it was my first try without reading the documentation, before defaulting to a workaround.

@devongovett
Copy link

This would be really useful to allow reusing behavior between multiple components, similar to React Hooks. For example, I'd like to try to port React Aria to Svelte. These hooks return DOM props that should be spread onto an element to provide some behavior. Part of the functionality that is returned are event handlers. I'd like to avoid needing to manually copy the events over one by one so the hook implementation details are hidden.

<button {...$buttonProps} on:pointerdown={$buttonProps.onPointerDown} on:pointerup={$buttonProps.onPointerUp}>
  Clicked {count} {count === 1 ? 'time' : 'times'}
</button>

Is there a good way to do this given the compiler won't know at build time what events are needed? Should I make a wrapper that does addEventListener myself with a bind:this? Would be nice if Svelte could handle dynamic events though.

@kevmodrome
Copy link
Contributor

I think you could probably use an action and pass in the buttonProps there,then manually add event listeners for each of the handlers.

@tanhauhau
Copy link
Member

@devongovett I believe use: is something you are looking for

@devongovett
Copy link

Ooh nice. Thanks for the super quick responses. Heading to sleep now but will try in the morning! 😊

@afaur
Copy link

afaur commented Aug 9, 2020

@devongovett Here is a use action example:

@devongovett
Copy link

Update: it worked! 🎉 https://github.com/devongovett/svelte-hooks

@TylerRick
Copy link

I was so excited to see that there was a decent workaround for this missing feature (using an action like use:applyEvents={eventProps})... but then when I tried to use this action to attach some event handler props on my component, I discovered I actually can't 😞 :

Actions can only be applied to DOM elements, not components

Does anyone know why actions (i.e., Svelte's closest equivalent to react hooks, which is the background where I'm coming from) can't be applied to components? Okay, I mean, I understand that it would have to work a little bit differently because we don't necessarily have a single root element (though #5218 could provide a way to mark which element(s) are considered the root), but I wish the node argument of an action could be polymorphic so you could easily apply some reusable pattern/concern/behavior/action to a component too (more thoughts in #5218 (comment)...).

And, does anyone have a good generic reusable solution for how to attach a bunch of event handlers (defined in an object in your component) to a child component used inside of your component?

Needless to say, I would like to see this feature implemented, so that we could just spread ("pass through") an object containing on: keys and have them automatically attached as event handlers — instead of having passed through as regular ("inert") props/attributes — which is probably never what you actually want to happen, so it's surprising when that's what actually happens instead.

Since that current behavior is surprising/useless, could that be an argument in favor of making on: keys in a spread object semantically equivalent to explicitly listing out/hard-coding each of those like <component on:something={handleSomething} on:else={handleElse}>.

Of course I know nothing about how hard this would be to implement; I'm speaking only in the context of developer experience as a Svelte user... But the fact that it's pretty easy to implement in a generic/reusable way for elements means I am at least hopeful that it could be easy to make something like this available for components too... 🤞

@LeanderG
Copy link

I also looked into how to create renderless components such as React Aria and Headless UI in Svelte.
Since actions only run in the browser, aria-attributes would not be present on the server-side rendered HTML, making actions a non-starter. I guess you could use a combination of spread props for aria-attributes and actions for event listeners, but that complicates the API quite a bit. Being able to spread event props would make it a lot easier to write renderless components.

@afaur
Copy link

afaur commented May 13, 2021

Relevent Issue: #5218

@TylerRick I put together something that works on a component at runtime, but it requires the component to provide access to its' owned elements. The component exposes references to the elements through bind:this in a tagRefs array (exposed via the accessors flag). This allows the Adjust component to spread aria properties, on events, and component props.

@LeanderG This doesn't use actions, but probably won't work with SSR.

This does break component encapsulation. However, there could be a case where someone created a component as part of an npm package, but they may have forgotten to support some aria property.

Typically, you would submit a PR, let it merge, and then either set your package.json to the SHA, or wait for the next release. This example would allow someone to patch things without altering the original authors npm package. (As long as the author implemented the tagRefs and accessors flag.)

@stale
Copy link

stale bot commented Dec 24, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale-bot label Dec 24, 2021
@frederikhors
Copy link

No stale.

@stale stale bot removed the stale-bot label Dec 24, 2021
@rgossiaux
Copy link
Contributor

I also find this annoying & a source of clunkiness in the framework. The ergonomics of on: are lacking in just about every way & it's hard to work with programmatically. I wish these issues got a lot more attention as they're clear areas where Svelte's DX is worse than React's.

@bruno-maruszczak
Copy link

Is anyone currently working on the issue? It's a shame that a feature that is meant to improve DX actually breaks some powerful design patterns. Couple of cross-framework libraries depend on similar features in Vue and React to append features to components in a composable fashion.

@brandonmcconnell
Copy link
Contributor

brandonmcconnell commented May 20, 2022

UPDATE 8/13/22: Since originally posting this comment, I've come to think there must be some sort of flag set on the element/component tag itself granting such "special" props the ability to be dynamically assigned. Otherwise, you leave your app open to an XSS-like attack.


I'd like to revive this Feature Suggestion, as I don't think the use: directive actually solves the issue of not being able to spread event handlers onto elements and components.

I opened another related issue—that I'll close—that duplicates this one. Here were the contents:

Describe the problem

On several occasions, I've found myself taking in the props for components in an array of objects and then building out the component instances by iterating over the array using an {#each} block.

Each object in the array will have unique directives such as event handlers (on: directive) and transition (transition: directive), and others.

It would be great if there were a way to naturally spread directive-props onto components as well.

Describe the proposed solution

I propose adding a new convention when spreading props where Svelte will accept event handlers by their natural HTML name and convert them to their on: directive counterparts.

For example:

<script>
  import MyComponent from './MyComponent.svelte';
  let data = [
    {
      id: 184638,
      postCount: 17,
      $$on_click: () => console.log('Hello world'),
      $$transition_fade: true,
    },
    {
      id: 473892,
      postCount: 4,
      $$on_click: doSomething,
      $$transition_fade: {
        y: 200,
        duration: 2000,
      },
    },
  ];
</script>

{#each data as child}
  <MyComponent
    {...child}
  />
{/each}

This would automatically spread the onclick handlers onto the components'/elements' on:click directive. This is my proposal, but I'd be interested in any naming or syntax that would make spreading event handlers in this way possible. It could be something more inline with the other magic Svelte $$ naming conventions, like the examples mentioned above, or just use strings (less preferable to me, like this):

{
  id: 184638,
  postCount: 17,
  'on:click': () => console.log('Hello world'),
}

Regarding the naming, I also considered wrapping the Svelte convention in quotes as you did to keep it exactly the same. If this passes, I keep going back and forth on what would make more sense between these two—

  1. '[[directive]]:[[value]]' (e.g. 'on:click', 'transition:fade')
    This option has the benefit of being pragmatic and sticking to the typical svelte naming conventions, just wrapped as a string because it contains the special character.

  2. $$[[directive]]_[[value]] (e.g. $$on_click, $$transition_fade)
    This option has the benefit of being a reserved Svelte convention and it less likely to get mixed up with the standard HTML onclick attribute.

Alternatives considered

Currently, to work around this, I am spreading the other non-event-handler props into a separate variable using a {@const} block in an intermediary wrapper component, like this:

<script>
  import MyComponent from './MyComponent.svelte';
  let data = [
    {
      id: 184638,
      postCount: 17,
      $$on_click: () => console.log('Hello world'),
    },
    {
      id: 473892,
      postCount: 4,
      $$on_click: doSomething,
    },
  ];
</script>

{#each data as child}
  {@const { $$on_click, ...props } = child}
  <MyComponent
    {...props}
    on:click={$$on_click}
  />
{/each}

While I do value the explicit nature of this method, it does often require creating an intermediary wrapper component as the component(s) I'm feeding into often forward the on:click directive and do not expect an onclick prop.

@flipkickmedia
Copy link

There are a number of open issues asking for this. Can a mod plz close these down so we can keep the focus in a single thread?

#7625
#5265

Also some work has been done to make this happen.
#6876

It need some work to make it pass tests but it should do what it being asked in this thread.

@ptrxyz
Copy link

ptrxyz commented Aug 2, 2022

This seems to also stop UI frameworks such as Chakra and Flowbite to port their stuff to Svelte.

Is there any info to this wrt what needs to be done to move this feature forward?

@flipkickmedia
Copy link

Id suggest for the above example there is little point in using spread props for events.

The idea behind spreading the events is so you can pass the event handler into the component dynamically. If you need to specify the event name, then passing in a dynamic function can already be achieved without additional syntax, simply by handling the implementation inside the function which handles the click handler.

Im not a fan of dumping logic into templates.

function clickHandlder() {
  if some condition
    run this
 else
   etc
}

@205g0
Copy link

205g0 commented Sep 30, 2023

This is super important, every UI that is slightly customizable by the user needs that, so it's a pretty basic requirement for any simple UI and would be also a no-brainer in other frameworks. Yeah, there are work-arounds but they are clunky/cumbersome. So, no need to block Svelte users here.

@dummdidumm dummdidumm removed the triage label Jan 19, 2024
@Rich-Harris
Copy link
Member

Closing as spreading events is supported in Svelte 5

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