Better on syntax

[NullVoxPopuli]

Propose better on syntax

Rendered

Alternative to RFC#997

Summary

This pull request is proposing a new RFC.

To succeed, it will need to pass into the Exploring Stage), followed by the Accepted Stage.

A Proposed or Exploring RFC may also move to the Closed Stage if it is withdrawn by the author or if it is rejected by the Ember team. This requires an "FCP to Close" period.

An FCP is required before merging this PR to advance to Accepted.

Upon merging this PR, automation will open a draft PR for this RFC to move to the Ready for Released Stage.

Exploring Stage Description

This stage is entered when the Ember team believes the concept described in the RFC should be pursued, but the RFC may still need some more work, discussion, answers to open questions, and/or a champion before it can move to the next stage.

An RFC is moved into Exploring with consensus of the relevant teams. The relevant team expects to spend time helping to refine the proposal. The RFC remains a PR and will have an Exploring label applied.

An Exploring RFC that is successfully completed can move to Accepted with an FCP is required as in the existing process. It may also be moved to Closed with an FCP.

Accepted Stage Description

To move into the "accepted stage" the RFC must have complete prose and have successfully passed through an "FCP to Accept" period in which the community has weighed in and consensus has been achieved on the direction. The relevant teams believe that the proposal is well-specified and ready for implementation. The RFC has a champion within one of the relevant teams.

If there are unanswered questions, we have outlined them and expect that they will be answered before Ready for Release.

When the RFC is accepted, the PR will be merged, and automation will open a new PR to move the RFC to the Ready for Release stage. That PR should be used to track implementation progress and gain consensus to move to the next stage.

Checklist to move to Exploring

• [ ] The team believes the concepts described in the RFC should be pursued.
• [ ] The label S-Proposed is removed from the PR and the label S-Exploring is added.
• [ ] The Ember team is willing to work on the proposal to get it to Accepted

Checklist to move to Accepted

• [ ] This PR has had the Final Comment Period label has been added to start the FCP
• [ ] The RFC is announced in #news-and-announcements in the Ember Discord.
• [ ] The RFC has complete prose, is well-specified and ready for implementation.
  • [ ] All sections of the RFC are filled out.
  • [ ] Any unanswered questions are outlined and expected to be answered before Ready for Release.
  • [ ] "How we teach this?" is sufficiently filled out.
• [ ] The RFC has a champion within one of the relevant teams.
• [ ] The RFC has consensus after the FCP period.

[SergeAstapov]

what do we actually get with the new syntax? reading through, I'm missing why we can't just keep on built-in and what limitations it has. what other possibilities new syntax would introduce? will all modifiers as well as custom use new syntax?

[NullVoxPopuli]

as well as custom use new syntax?

Naw, just on

what other possibilities new syntax would introduce?

Idk, the main thing is event modifiers

what do we actually get with the new syntax?

Shorter event binding, event modifiers, having builtin event binding look different from userspace modifiers, potentially some perf for cleanup, as usage is more constrained, so we don't need to worry about teardown

[Techn1x]

Feels like a lot of work throughout the ecosystem for little gain. Whole new syntax, would need guides, linting updates..

I like that on being presented as just a modifier makes it clear to devs that it's nothing really special. It's just another modifier, like any we may write for other purposes

[jelhan]

I see it as a benefit of Ember that even basic functionality like {{on}} modifier can be rebuild using available primitives. That makes it much easier to reason about them. I don't get the benefit of introducing another mental complex for them.

Keeping it aligned with the primitives also eases migration story. No one expects {{on}} modifier to go away any time soon. But I feel 10 years ago we would have thought the same about {{action}}.

[NullVoxPopuli]

Counterpoint (to myself) svelte is getting rid of this syntax in favor of normal 'onclick' , 'onkeypress', https://svelte-5-preview.vercel.app/docs/event-handlers#why-the-change

And we can already use those, which i forgot about

[a13o]

It's easy to teach new Ember devs the {{on}} syntax because it's composed of existing Ember and JS concepts.

The proposed syntax will introduce a new Emberism to memorize, and I don't think it adds enough value to justify its expense on the cognitive load of Ember learners.

The native onclick and proposed on:click are pretty close, I worry about this distinction being too subtle.

The existing {{on}} modifier already supports capture, once, and passive via named args. This makes it easy to teach the {{on}} modifier as a thin wrapper for addEventListener. And with that mental model, there's no expectation that event.preventDefault() was called for you. {{action}} did that, and it was an Octane benefit to drop that and have a little less hamster magic to memorize.

<form on:submit|preventDefault={{@onSubmit}}>

The proposed syntactic sugar for adding the event.preventDefault() logic will live in the template rather than the JS, which is something an Ember developer could grow accustomed to looking for, but would be a gotcha to a Javascript developer trying to learn or evaluate Ember. I don't think Ember in general takes a strong enough position on what logic does or doesn't belong in the template, but I think if the position there were firmed up, it would be a violation to allow 1 specific bit of event logic to live in the template.

---

With all that said, I do share the concern at the root of the proposal. While template imports are fantastic for the node processes living in the build system, they're a pain for humans authoring components. It's hard to remember the 20 different locations that common things need to be imported from.

I consult this reference almost daily because it just doesn't ever seem to click if one thing comes from @ember/helper, or @ember/modifier, or where things like Input and Textarea live.

Many authors also carry ember-truth-helpers, ember-math-helpers, etc into their gjs/gts out of habit from the split js/hbs days. These libs IMO are not worth the weight of their import statement in the Polaris edition.

Taken in sum, the top of a gjs/gts starts to look like the top of a java file; and without a strong intellisense system to hand wave the problem away. I do think this is an adoption hurdle for Polaris, and reducing the cognitive burden would be a boon for Ember. I'd prefer a holistic approach to this problem rather than a hyperfocus on each specific template import statement.

[evoactivity]

I pretty much agree with what everyone has written.

• New syntax to teach and work to update all docs
• Familiarity is good actually
• It's using primitives and helps people understand it's just a modifier
• I think preventDefault et al should be dealt with in the JS side, not the template
• Svelte is moving away from similar syntax as you mentioned

I would like to not have to import the built ins, it makes sense for me to import my user land code but I don't see why things like on, fn, array, hash etc should be imported, they should just be part of the language. I'd support an RFC that takes that approach over one that creates a new syntax.

[flashios09]

Sorry but i don't like the on:event syntax, i prefer the mustache syntax {{on event doSomething}}, the mustache syntax make it easier to see/capture all the dynamic parts/sequences of the template, anything outside of {{}} is just text/html:

just text here
<button {{on 'click' itsClearThatThisIsAFunctionCall}} id="just-html">Do Something</button>
<span class="text">{{ aDynamicProperty }}</span>

So even we just use the native js event syntax onclick={{doSomething}} or we keep the {{on}} modifier !

I would go more for transforming the native js events behind the scenes to the {{on}} modifier(to make it compatible with EMBER FastBoot), IMO that would be the best solution:

Before:
<button {{on 'click' increment}}>Increment</button>
After:
<button onclick={{increment}}>Increment</button>

[jelhan]

The onclick HTML property has the major trade-off that it can be only set once. Unless we introduce some Ember-specific syntax, there is no straight forward way converting <button {{on "click" foo}} {{on "click" bar}}> to onclick property.

Additionally, I see it as one of the major problems of Glimmer template syntax that it is unclear if it is an HTML attribute property. This currently depends on logic implementing in the Glimmer VM as far as I remember. We should not make that more complex by introducing more properties, which look like attributes but being handled as properties by the Glimmer VM.

[NullVoxPopuli]

Thanks for all the feedback, based on all this, I've opened:

• https://github.com/krausest/js-framework-benchmark/pull/1608
  • demonstrates significant performance improvement by not using the on modifier
• demo showing onclick works how we want
  • demonstrates that onclick can be set more than once
• Reminder to myself to update interactive tutorial learning materials to use on{event} for non-dynamic event listeners
  • https://github.com/NullVoxPopuli/limber/issues/1665
• Issue for discussion for changing the learning materials on the ember guides as well:
  • https://github.com/ember-learn/guides-source/issues/2009
  • if we teach folks to use native event properties, then we probably don't need to necessarily make {{on}} a keyword

[jelhan]

demo showing onclick works how we want

* demonstrates that `onclick` can be set more than once

Maybe I'm missing something but that demo does not seem to show that you could use onclick twice on the same element. Of course you can use it on multiple elements. Staying with your demo, I was concerned about the case in which you want to increment and bump the increment on a click event on one button. With {{on}} modifier you can do <button {{on "click" this.increment}} {{on "click" this.bumpIncrement}}>. The onclick attribute would require another function, which call both of them.

We could accept that trade-off for sure. Especially if it has a noticeable performance benefit. Likely primarily an issue for upgrade path and less for new code.

[NullVoxPopuli]

twice on the same element.

ah, that's not how I interpreted "it can be only set once." in the demo, changing incrementBy, re-sets the onclick function, thus setting it more than once :sweat_smile:

n which you want to increment and bump the increment on a click event on one button.

in this demo, that would have indeterminate behavior cross-browser, because the order of these specific event handlers needs to be, 1. increase the incrementBy, then 2., do the other handler.

it should only be recommended to have multiple event handlers listening to the same event if the event handlers are totally independent in behavior and don't intend to cause the other handlers to behave differently (which is what's happening in my demo :sweat_smile: )

We could accept that trade-off for sure. Especially if it has a noticeable performance benefit.

yeah, I think what I'm trying to narrow in on, is a sort of decision tree for when to use which thing:

stateDiagram-v2
  state "use onclick" as useOnclick 
  state "use {{on}}" as useOn
  state "need cleanup" as cleanup
  state "need conditional {{on}}" as cond
  state "need multiple {{on}} the same event name" as multiple
  state "handlers are independent" as indMultiple
  state "combine the handlers to make serial" as serial

  [*] --> cleanup
  cleanup --> cond: no
  cleanup --> useOn: yes
  cond --> useOn: yes
  cond --> multiple: no
  multiple --> indMultiple: yes
  multiple --> useOnclick: no
  indMultiple --> serial: no
  indMultiple --> useOn: yes
  serial --> useOnclick

  

[lifeart]

@NullVoxPopuli seems it may be compiler feature - if on is used without extra args and no on modifiers with same event name and no ...attributes compiler could apply prop assign opcode instead of on

[NullVoxPopuli]

@lifeart yeah, I like that idea -- we could improve the performance of everyone's apps with a minor update with no change to anyone's code.

and compiler changes like that don't need an RFC, because the wireformat is totally private to ember users

[lifeart]

@NullVoxPopuli seems main performance benefit gained not from attribute usage, it depends on do we have destructor or not. And generally on modifier may not have destructors (if properly used), but to get it we need to bypass modifier manager, and this is seems doable because on modifier is an internal implementation.

• GXT has skip destructors flag enabled by default (for on modifier) https://github.com/lifeart/glimmer-next/issues/9#issuecomment-1879814751
• Svelte also don't use destructors for on https://github.com/sveltejs/svelte/blob/main/packages/svelte/src/internal/client/render.js#L323

[kategengler]

I agree completely with @evoactivity' s comment here, and want to add that microsyntaxes are hard to remember.

[SolPier]

Are there other use cases where this syntax could be used ? So it wouldn't be for one feature only.

[jelhan]

@NullVoxPopuli seems main performance benefit gained not from attribute usage, it depends on do we have destructor or not.

Do I understand correctly that performance benefit is gained from omitting removeEventListener? That's indeed not needed if destroying the element immediately afterwards. But that's likely not only the case for {{on}} modifier. Also other modifiers may only need to run destructor if modifier is removed but element is not destroyed.

generally on modifier may not have destructors (if properly used)

I don't think that's true. Destructor is critical if the modifier is applied conditionally. Even though that's not that often needed, it's a proper usage.

[NullVoxPopuli]

Are there other use cases where this syntax could be used ?

not really, it was only meant for on

. But that's likely not only the case for {{on}} modifier. Also other modifiers may only need to run destructor if modifier is removed but element is not destroyed.

yes, see the decision tree here: https://github.com/emberjs/rfcs/pull/1007#issuecomment-1950275078

I don't think that's true. Destructor is critical if the modifier is applied conditionally. Even though that's not that often needed, it's a proper usage.

static case is more common than conditional case. I personally cannot be ok with "proper" meaning 'less performant' :upside_down_face: (hence decision tree! 🥳)

[jelhan]

. But that's likely not only the case for {{on}} modifier. Also other modifiers may only need to run destructor if modifier is removed but element is not destroyed.

yes, see the decision tree here: #1007 (comment)

That decision tree is only working for {{on}} modifier. As there are the on* properties which serves as an alternative. That might be a mitigation if the issue but not a solution.

It sounds as if modifiers need the capability to optionally skip destructor if element is destroyed in the same render cycle. One of the many performance related features still missing for modifiers...

[NullVoxPopuli]

That might be a mitigation if the issue but not a solution.

What gap do you see as remaining?

It sounds as if modifiers need the capability to optionally skip destructor if element is destroyed in the same render cycle.

If an element is destroyed, we don't need to worry about cleanup (with on), because an element would be garbage collected, as would any data / event listeners / etc attached to that element (provided no other handles on that data exist in userland).

(for on, specifically) The case where we worry about cleanup is in dynamic modifiers. For all other modifiers, such as using the resize observer service, cleanup is very important)

One of the many performance related features still missing for modifiers...

yeah having more features in modifiers is nice - but is unrelated to how we use modifiers (in any syntax!).

The feature I think we can implement today without any RFC or worry about public API is @lifeart's suggestion, but it is specifically about optimizing DOM cleanup (needed when navigating away from a page).

The feature you want, (and I see you linked from the style-modifier repo), is a pre-paint timing, and that would be amazing, but requires manager changes.

[jelhan]

The feature you want, (and I see you linked from the style-modifier repo), is a pre-paint timing, and that would be amazing, but requires manager changes.

style modifier is likely affected in the same way as on modifier. It needs to run cleanup only if conditionally applied. Without me noticing it seems that style modifier taken a different decision on the trade-off: it does not support conditional usage and skips cleanup. I'm afraid that supporting conditional usage of style modifier would have negative performance impact on majority of use cases per your performance investigation for on modifier.

[NullVoxPopuli]

'm afraid that supporting conditional usage of style modifier would have negative performance impact

Why not have folks do:

<div style="css here {{if cond 'other CSS'}}">

</div>

or

<div class="foo {{if cond 'bar'}}">

</div>
<style>
  .foo {
  
    &.bar {
    
    }
  }
</style>

via https://github.com/cardstack/glimmer-scoped-css ?

[runspired]

imo: the compiler opt isn't worth it for the additional complication and confusion it can lead to around event ordering and cleanup. Events added by a modifier/modifier syntax should all agree on what event delegation timing they have.

[johanrd]

Counterpoint (to myself) svelte is getting rid of this syntax in favor of normal 'onclick' , 'onkeypress', https://svelte-5-preview.vercel.app/docs/event-handlers#why-the-change

And we can already use those, which i forgot about

Very intriguing. – Why teach to use a modifier when there is support by HTML natively? As we write in the very beginning of our guides:

Core concept: "Templates are HTML" At its core, Ember's UIs are HTML driven

onclick seems to me to work well in practice. Yet, currently, Glint complains: Only primitive values are assignable as HTML attributes:

function hello() { 
  alert('hello') 
}

<template>
  <button onclick={{hello}}>Hello</button>
</template>

// Only primitive values (see `AttrValue` in `@glint/template`) are assignable as HTML attributes. 
// If you want to set an event listener, consider using the `{{on}}` modifier instead.
// Type 'void' is not assignable to type 'AttrValue'.glint(2322)

[jelhan]

onclick seems to me to work well in practice. Yet, currently, Glint complains: Only primitive values are assignable as HTML attributes

I think this is correct. Because we teach that <button onclick={{hello}}> sets the onclick HTML attribute. For historical reasons it is setting the onclick property of the element in this case. That's why it's working. To be clear, it's only working because it is not following HTML syntax in this case.

[NullVoxPopuli]

Glint complains: Only primitive values are assignable as HTML attributes

I think this is an oversight, tbh

[NullVoxPopuli]

so after talking with a few people it seems the use case for onclick is kind of very specific, so maybe it shouldn't be a default at all and just "a tool you know about for that one specific situation"