kbrsh / moon

🌙 The minimal & fast library for functional user interfaces
https://moonjs.org
MIT License
6k stars 200 forks source link

Moon compiler does not apply DOM event listeners or attributes to created element by for #256

Closed LeviSchuck closed 5 years ago

LeviSchuck commented 5 years ago

The for element creates an element in its place, but unfortunately the other non-for-like attributes and events are not planted on the created element.

For example, a click event and id are not replicated to the ul which is created in the place of the for when compiled.

A minimal example, which can be demonstrated in the playground is included below.

const inc = ({ data, view }) => {
    const dataNew = {
        count: data.count + 1,
        list: [...data.list, data.count],
    };

    return {
        data: dataNew,
        view: (<Demo data={dataNew}/>)
    };
};

const dec = ({ data, view }) => {
    if (data.count <= 0) return {};

    let newList = [...data.list];
    newList.pop();
    const dataNew = {
        count: data.count - 1,
        list: newList,
    };

    return {
        data: dataNew,
        view: (<Demo data={dataNew}/>)
    };
};

const Demo = ({ data }) => (
    <div>
        <h1>Demo {data.count}</h1>
        <button @click={inc}>Push</button>
        <for={entry}
            of={data.list}
            name="ul"
            @click={dec}
            id="test-id"
        >
            <li>{entry}</li>
        </for>
        <button @click={dec}>Pop</button>
    </div>
);

Moon.use({
    data: Moon.data.driver({
        count: 0,
        list: [],
    }),
    view: Moon.view.driver("#root")
});

Moon.run(({ data }) => ({
    view: (<Demo data={data}/>)
}));

I ran into this when trying to use a web component, per this component's design, it emits events (which moon is able to bind to outside of for) but it's input is child divs, using for inside of it doesn't seem to work out.

<for={option} of={data.options} name="elix-filter-combo-box" @selected-index-changed={updateOption}>
      <div>{option.label}</div>
</for>

Perhaps, a DOM fragment approach for the for? Let the user decide what's outside and inside, as well as before and after?

kbrsh commented 5 years ago

Hey!

Thanks for the detailed testing and bug reports by the way, it really helps :)

for takes a data prop as an object which it will apply as data, and it doesn't apply DOM properties with ...props or something similar to avoid unexpected behavior.

Try something like:

<for={entry} of={data.list} name="ul" data={{
   id: "test-id",
   "@click": dec
}}></for>

I've considered DOM fragments before, but came across issues with the logic to remove them from the DOM. There is no equivalent of fragment.parentNode.removeChild(fragment) AFAIK. That's why I went with the custom parent element options route. Let me know what you think.

LeviSchuck commented 5 years ago

Wonderful! I have verified this gives the intended behavior. Although, I found this to be surprising from an API standpoint.

I can see why fragments would complicate dom operations.

kbrsh commented 5 years ago

Awesome! It can be a little surprising, but I personally think that it's better than having conflicts with other DOM property names. For example, you wouldn't be able to set a name attribute on an element because it it used for the name of the element instead.

kbrsh commented 5 years ago

Closing as it works as documented — let me know if you have any suggestions to change the API.