Skip to content

Inline variable assignment in mustache tags #5087

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

Closed
ElianCordoba opened this issue Jun 29, 2020 · 7 comments
Closed

Inline variable assignment in mustache tags #5087

ElianCordoba opened this issue Jun 29, 2020 · 7 comments

Comments

@ElianCordoba
Copy link

In my experience I frecuently encounter the following code pattern:

<button on:click={() => menuOpen = !menuOpen}>Menu</button>

I propose the following code shorthand:

<button on:click={menuOpen = !menuOpen}>Menu</button>

The outputted compiled code should be the same in both cases. I think that the implementation could be if the parser sees an assignment inside moustache tags, it wraps it up in an anonymous function.

Not entirely sure if there are edge cases that I should consider. If it's a reasonable enhancement I would like to give it a shot. Someone pointing me out where to start would be much appreciated :)

On a personal note, I think this aligns nicely with existing code sugar/shorthands that svelte offers.

(I call the {} moustache tags because I've seen it the source code although I'm not really sure if that is the right technical name)

@antony antony added the proposal label Jul 1, 2020
@antony
Copy link
Member

antony commented Jul 1, 2020

My personal feeling is that this increases the surface API area with no discernible benefit. right now it's very simple to say that a handler is a method, and that's that.

This introduces a branch into the API which for one, doesn't receive the event itself.

I also think there'll be a case whereby it's unclear whether the content of the expression is a method to be called, or returns a method to be called, or has a side-effect of calling a method, but I can't bring one to mind right now.

So it'd be a a no from me. Leaving open for others to weigh in.

@dimfeld
Copy link
Contributor

dimfeld commented Jul 1, 2020

Yes, the case of on:click={createClickHandler(item)} is the one that worries me with this change. (The “returns a method to be called” case mentioned by Antony.)

I use a pattern like this in some of my charting code where the individual chart components share a lot of logic across their event handlers, and so it makes sense to use factory functions to create the event handlers. I could adapt without much trouble, but this would definitely be a breaking change if it were to be added.

@ElianCordoba
Copy link
Author

@antony
Mmm the benefit is small, that's true, but Svelte already has this type of syntactic sugar, like:

<input bind:value={value}>

// Same as

<input bind:value>

Another example is the forwarding all events, although that one also affects functionality so it not just syntax sugar.

right now it's very simple to say that a handler is a method, and that's that.

To be honest, I'm not that familiar with the internals but, in my mind, we would just have to introduce a check to see if the code inside the {} it's an assignment of not, the output of the two examples I shared above would be equal

@dimfeld

Yes, the case of on:click={createClickHandler(item)}

Since that is not an assignment the should follow the existing compilation path

but this would definitely be a breaking change if it were to be added.

No really, because the existing code will produce the same output.

Maybe I've overseen something obvious, sorry if that's the case. I'll give it a shot this weekend

@dimfeld
Copy link
Contributor

dimfeld commented Jul 1, 2020

I don't think the bind:value case is an analogous case, as it is simply syntactic sugar, while this does change the behavior of the expression.

The problem with a "does this expression have an assignment" heuristic is less a technical issue than a developer experience issue. Currently an expression in a handler is always evaluated at the time that the component/element is created. Changing it would mean that the expression is evaluated either at creation time or when the event occurs, depending on the contents of that expression.

Here's an assortment of things you might want to do in a handler when the expression can be interpreted as a function contents.

on:click={x = 5}
on:click={list.push(5)}
on:click={doIt(5)}
on:click={abc.def(5)}
on:click={() => abc.def(5)}
on:click={set(abc, 'def', 5)}
on:click={save}
on:click={save()}
on:click={save(); x = 5;}

I don't think the additional layer of complexity where sometimes you can just write the code and other times need to wrap it in a function is worth the convenience, and I guarantee it will confuse newcomers to Svelte, many of whom are also new to Javascript development completely.

Edit: All that said, I have at times wished I could do this too. I do sympathize with the desire to enable this sort of syntax. I just haven't come up with an appropriate way to do it that wouldn't increase the complexity unnecessarily. The best one I thought of is something like on:click|immediate={x = 5} but that isn't really better than just on:click={() => x = 5}.

@mjadobson
Copy link

mjadobson commented Jul 3, 2020

The best syntax I could think of was: run:click={x=5}, which I think captures the idea that it's running the expression when the user clicks, rather than wiring up event listeners with on:click. Vue has a magic $event variable for accessing the event object, but this shouldn't be necessary as this is already pretty terse: on:click={e => x = e.target.value}.

However, I am concerned about bloating the API surface area for this convenience, and while it's nice to put state changes in your templates when you're quickly prototyping, I think components are more maintainable when event handlers are explicitly written in <script>...</script>, and then passed by reference.

@stephane-vanraes
Copy link
Contributor

Not entirely related to the overall problem, but retracing the initial example of

<button on:click={() => menuOpen = !menuOpen}>Menu</button>

This is indeed a very common pattern I ran into several times, to the point I made a separate store-like object for it svelte-toggleable that abstracts away this kind of operation to the point you can do

<button on:click={menuOpen.toggle}>Menu</button>

@antony
Copy link
Member

antony commented Jul 10, 2020

Closing this as I believe that whilst the sentiment is correct, and it's always nice to have a shorthand, the amount of overheads in compilation and comprehension for people, i.e:

on:click={set(abc, 'def', 5)}

if perhaps the store returned a method as part of it's state change, would make this feature a bit of a non-starter. There's a huge benefit to be had by keeping things predictable.

@antony antony closed this as completed Jul 10, 2020
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

5 participants