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

feat: dynamic event bindings #6876

Closed

Conversation

flipkickmedia
Copy link

@flipkickmedia flipkickmedia commented Oct 22, 2021

This PR provides a means to pass a {...spread} operator with bindings so they can be handled dynamically.

It's highly likely this will need rework and some guidance. It's POC and my first contribution towards svelte.

Ill write tests once this is implemented with agreement.

test.svelte

<script lang="ts">
    import {createEventDispatcher} from 'svelte';

    const dispatch = createEventDispatcher();

    function sayHello() {
        dispatch('testevent', {
            test: 'food'
        });
    }
</script>

<div on:click={sayHello}>test</div>

App.svelte

<script lang="ts">
    import test from "./test.svelte";

    function testFn() {
        console.log('testing');
    }

    function testFn2() {
        console.log('testing2');
    }

    export let blah = {
        'event:bindings': {
            'testevent' : testFn
        }
    }

</script>

<svelte:component this={test} {...blah} on:testevent={testFn2} />

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with [feat], [fix], [chore], or [docs].
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with npm test and lint the project with npm run lint

@dummdidumm
Copy link
Member

Could you explain what the use case for this feature is and give some examples?

@ghost
Copy link

ghost commented Oct 23, 2021

@dummdidumm yeah sure...

Overview

At the company I'm working for, we are developing a dynamic UI/UX system to read data from a meta data system and data engine. We also have data roles/permissions/policies which provides a map of what the user can do with the data.

This presents a few problems.

  • an unknown data structure, but a known map between data models and components.
  • an unknown set of data policies for a given view until we navigate into it.
  • can't assume to know what components we need to render until we navigate into the system
  • roles/permissions/data policies may swap versions of components and these components might broadcast different event's depending on their functionality.
  • components have different functionality imported dynamically

Typically when you build anything dynamically the process of deriving the structure of the UI and the data and event flow means that everything is loaded just in time. The link between the parent/child components is done using a container component which maps the props, events and loads and get's needed for the child component to load.

Senario/Example

For example an admin might use DataEditAdmin.svelte and a normal user might use DataEditStd.svelte. DataEditAdmin.svelte has access to properties on the metadata system that the Std component does not. If the Admin user modifies their layout, those admin components broadcast events which the standard components do not in order to capture the modified child layout.

If svelte can do svelte:component this={instance} we assume that the instance is dynamic, the props are dynamic, we must also match that with event bindings that are dynamic in order to allow complete separation of the dynamic instance from hard coded values inside the instantiation of the component.

Code example

We have a window...
image

When we click the window, we want to show some options...
image

We don't know what those options are or will do, we also don't know what component those options are built from...but when we click one of them, we need to be able to pass this event into the container component so we can handle it.

reusableFunction.ts

export type MyDataStructure = {
    propertyA: string;
    someFn: CallableFunction;
}

function dynamicFunction(event: CustomEvent) {
    console.log('this is a dynamicFunctionA')
}

function dynamicFunctionHandlesData(event: CustomEvent, data: MyDataStructure) {
    console.log('this is a dynamicFunctionWithData')
    //in this example someFn is simply a dispatch object we created so we can dispatch the event but it could be anything
    data.someFn(event);
}

export {dynamicFunction, dynamicFunctionHandlesData}


AdjustmentsContainer.svelte

<script lang="ts">
    import {ComponentSchema} from "@/lib/dynamicLoad"
    import {onMount} from "svelte";
    import type {MyDataStructure} from "./reusableFunction"
    import {dynamicFunctionHandlesData} from "./reusableFunction";

    export let top
    export let right
    export let width = 100;

    export let height = 50;
    let order
    let resizeWidth
    let resizeHeight
    const someData: MyDataStructure = {} as MyDataStructure;

    function testFn() {
        console.log('testing testFn');
    }

    function testFn2(event: CustomEvent) {
        console.log('testing testFn2');
    }

    //assume this is imported dynamically
    function listItemClickHandler(event: CustomEvent) {
        console.log(event.detail);
        console.log('this is a test binding');
    }

    const dispatch = createEventDispatcher();

    const someData: MyDataStructure = {
        someFn: dispatch
    } as MyDataStructure;

    //assume this is loaded from an api/json schema and the listItemClickHandler is resolved using a dynamic module import
    export let properties: ComponentSchema[] = [
        {
            componentName: "./InteractableListItem.svelte",
            dataSource: 'someurl.com/data.json',
            resolvedComponent: undefined,
            loaded: false,
            props: {
                content: 'hello',
            },
            events: {
                'event:bindings': {
                    'test': [testFn, {data: someData}],
                    'testevent': [testFn2, {data: someData}],
                    '*': [dynamicFunctionHandlesData, {data: someData}],
                }
            },
        } as ComponentSchema
    ] as ComponentSchema[]

    onMount(async () => {
        for (let view in properties) {
            console.log('slowly loading component ' + properties[view].componentName);
            if (properties[view].resolvedComponent === undefined && properties[view].componentName) {
                let name = properties[view].componentName
                properties[view].resolvedComponent = (await import('../' + name)).default
                properties[view].loaded = true
                console.log('done loading')
                properties = properties;
            }
        }
    });

    $: cssWindowStyle = `--top-position:${top}px;--right-position:${right}px;--width:${resizeWidth}px;--height:${resizeHeight}px;`
</script>

<div class="adjustments-panel position" style={cssWindowStyle}>
    {#each properties as {componentName, dataSource, resolvedComponent, props, events},i}
        <svelte:component this={resolvedComponent} {...props} {...events} />
    {/each}
</div>

InteractableListItem.svelte

<script lang="ts">
    export let content = '';
    import {createEventDispatcher} from 'svelte';

    const dispatch = createEventDispatcher();

    function handleClick() {
        dispatch('listItemClicked', {test: 1});
    }
</script>

<div class="window-listitem selectable" on:click={handleClick}>
   {content}
</div>

@dummdidumm
Copy link
Member

Thanks for explanation! So this is kind of the other end of #4599 . I'm not sure about the API proposal but I definitely agree that this makes sense to be able to react to arbitrary events. If #4599 would be implemented as on:* or bubble:*, then a matching implementation could be on:*={handleEvents} where handleEvents would be handed the event which you can differentiate based on the type property which all DOM events and the custom events have in common.
Note that this is just me thinking out loud for now. I'll discuss this with the other maintainers soon hopefully.

@ghost
Copy link

ghost commented Oct 23, 2021

Precisely :)

During dev this morning tho, this method does present some problems. If I want data inside my handled event, if the functionality is dynamically imported this presents some interesting architecture challenges.

I would also assume that, passing a spread operator is always preferable to on:* - I can't really think of a time where I might want on:* if you could just pass config in. This might be useful for dev, to see which events are being broadcast but if you want to handle all events, you can simply pass in a * into the event config.

So here I'm working on the basis that my itemListClick handler is imported dynamically, we don't want to pollute the CustomEvent.

Ive looked at the use cases for on:*

Personally, I feel on: is special and Id downvote the on:* because it has some checking and consistency. CustomEvents however handled like this allow the dev to bypass the safety of sveltes event checking.

If you take this example...

<script lang="ts">
...
    const dispatch = createEventDispatcher();

    const someData: MyDataStructure = {
        someFn: dispatch
    } as MyDataStructure;

    //assume this is loaded from an api/json schema
    export let properties: SchemaViews = {
        ListItemA: {
            componentName: "./dynamic/InteractableListItem.svelte",
            dataSource: 'someurl.com/data.json',
            resolvedComponent: undefined,
            loaded: false,
            props: {
                content: 'hello',
            },
            events: {
                'event:bindings': {
                    'listItemClicked': ['clickableInteractions', 'listItemClickHandler', {data: someData}]
                }
            },
        } as ComponentSchema,
        ListItemB: {
            componentName: "./dynamic/InteractableListItem.svelte",
            dataSource: 'someurl.com/data.json',
            resolvedComponent: undefined,
            loaded: false,
            props: {
                content: 'hello',
            }
            events:{}
        } as ComponentSchema
    }

    //components are then resolved dynamically
...

Container.svelte

    {#each properties as {componentName, dataSource, resolvedComponent, props, events},i}
        <svelte:component this={resolvedComponent} {...props} {...events}/>
    {/each}

If you were to add on:* this would then add in bubbling and event handling when there wasn't any for the second list item. I would argue an assumption that in most cases, svelte:component is being used in a similar manner to handle multiple items and in cases where you handle a list you end up hardcoding a value into a dynamic component. This is generally a bad idea AFAIC - for DOM components, maybe it's fine but Im concerned with svelte:component here not other types of use so I might be wrong.

@flipkickmedia
Copy link
Author

You can pass in data using this method, just provide it in the binding.

@matthieujabbour
Copy link

matthieujabbour commented Jul 17, 2022

@flipkickmedia thank you so much for your work on this! Do you have any clue on whether it will be accepted and/or released soon?

@brandonmcconnell
Copy link
Contributor

brandonmcconnell commented Jul 17, 2022

@flipkickmedia Great work on this so far. As the author of one of the duplicate issues, #7548, I just had one question—

Allowing event keys to be included in JSON data as the string counterparts of their literal spelt notation, it certainly keeps the syntax familiar, but wouldn't is also open the door to some dangerous situations where 3rd parties could pass in "event props" and thus take control of your app to some capacity? To avoid this, a user would have to sanitize all JSON data being used for props.

The two options I see are…

  • going the route you suggested, but only evaluating event props if an additional flag is used on the element/component, something like ALLOW_EVENT_PROPS
  • similar to the first option, exposing a helper function from the svelte package like evalEventProps that wraps the props like this:
    <MyComponent {evalEventProps(...propsWithEvents)} />

@ptrxyz
Copy link

ptrxyz commented Aug 2, 2022

In the example, the <svelte:component this={test} construct is used. Would this workaround also work when the component is used directly (as in <Test ....>)?

@Jojoshua
Copy link

Jojoshua commented Aug 8, 2022

Thanks for explanation! So this is kind of the other end of #4599 . I'm not sure about the API proposal but I definitely agree that this makes sense to be able to react to arbitrary events. If #4599 would be implemented as on:* or bubble:*, then a matching implementation could be on:*={handleEvents} where handleEvents would be handed the event which you can differentiate based on the type property which all DOM events and the custom events have in common. Note that this is just me thinking out loud for now. I'll discuss this with the other maintainers soon hopefully.

Any update on the discussions @dummdidumm ?

@flipkickmedia
Copy link
Author

flipkickmedia commented Aug 12, 2022

In the example, the <svelte:component this={test} construct is used. Would this workaround also work when the component is used directly (as in <Test ....>)?

If you import the component import Test from Test.svelte then you can use the imported component.

The only reason to use a the <sveltecomponent ...> method is so you can dynmically load the component at runtime to make use of the meta data.

If you want to pass spread props/events to a component <Test {...props} {...events}> I can't really see a good reason to do this since you are statically generating the component and therefore know what props/events you need.

@brandonmcconnell
Copy link
Contributor

@flipkickmedia Believe me— I want a method for spreading event props into a component as much as anyone. It's why I created an issue quite like this one).

However, allowing any arbitrary JSON to be spread into special event/transition props is too unsafe if always done. 3rd-party providers and packages could easily abuse this and include such props in their JSON export which would put your site at risk of an XSS-like attack. Even if you know the initial structure, there's no way to know that such props wouldn't be added later.

@flipkickmedia
Copy link
Author

@flipkickmedia Believe me— I want a method for spreading event props into a component as much as anyone. It's why I created an issue quite like this one).

However, allowing any arbitrary JSON to be spread into special event/transition props is too unsafe if always done. 3rd-party providers and packages could easily abuse this and include such props in their JSON export which would put your site at risk of an XSS-like attack. Even if you know the initial structure, there's no way to know that such props wouldn't be added later.

I think we possibly are thinking the same thing. The checking in svelte means when you import a component using a name e.g. <Test .....> Svelte will check to make sure the props/events passed are valid at compile time. If you use a spread operator, this needs to be done at runtime.

This process depends on validating the data at runtime to stop the things you mention. The reason for my comment about not using a spread operator when statically defining the component name is because Svelte can then do its checks when it compiles.

You could use the <svelte:component> method and just load your component into this to get the same result with this PR.

Svelte can then ignore the checks when compiling and the event/prop data can be (and should be) validated at runtime.

@dummdidumm dummdidumm added this to the one day milestone Feb 22, 2023
@benmccann benmccann changed the title [feat] added dynamic event bindings feat: dynamic event bindings Mar 14, 2023
@Rich-Harris
Copy link
Member

Closing as spreading events is supported in Svelte 5 — thank you

@Rich-Harris Rich-Harris closed this Apr 2, 2024
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.

7 participants