rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

HTML Attribute and Property Rationalization

Open chadhietala opened this issue 6 years ago • 9 comments

Rendered

chadhietala avatar Mar 22 '18 00:03 chadhietala

Sooo.... we're bringing back a variant of {{bind-attr}}?? 😕 (In regards to DX, no fun).

Panman82 avatar Mar 22 '18 13:03 Panman82

Seems like an implementation of core element modifiers https://github.com/emberjs/rfcs/pull/112

The old way still exist, but works in a different way under the hood. The above would only be in certain situations. @chadhietala maybe an example is required of when the above would be required and when not?

knownasilya avatar Mar 22 '18 13:03 knownasilya

@knownasilya Yes this is introducing more modifiers like #112 outlined. We are actively working on revising that RFC. This RFC may make more sense when #112 is updated.

@Panman8201 Not quite. {{bind-attr}} was used for introducing dynamic attributes to any element and still had the implicit semantics as described in this RFC. There are only a handful of places where we think {{prop}} would need to be used. Below is an outline of some unexpected consequences of how we do things today, along with some use cases we feel like will most likely would be affected.

Weird Cases That Work Today

These cases would require the {{prop}} modifier. For simplicity sake the below examples use static strings, but the result and migration is the same if they are dynamic.

innerText

<div class="red" innerText="Inner Text">
- What in the World???
</div>

Renders:

<div class="red">
Inner Text - What in the World???
</div>

Would be re-written to:

<div class="red" {{prop innerText="Inner Text"}}>
- What in the World???
</div>

innerHTML

<div class="red" innerHTML="<h1>Inner HTML</h1>">
- What in the World???
</div>

Renders:

<div class="red">
<h1>Inner HTML</h1> - What in the World???
</div>

Would be re-written to:

<div class="red" {{prop innerHTML="Inner HTML"}}>
- What in the World???
</div>

textContent

<div class="red" textContent="Text Content">
- What in the World???
</div>

Renders:

<div class="red">
Text Content - What in the World???
</div>

Would be re-written to:

<div class="red" {{prop textContent="Text Content"}}>
- What in the World???
</div>

More Reasonable Cases

As mentioned in the RFC majority of the time you want what happens by default. These cases should only pertain to built in "user input" elements and custom elements. Consider the following html:

<input value="Chad" />

When the browser parses this and creates the DOM it will setAttribute of value to "Chad", however if the user types into the <input /> the HTML attribute is not updated but rather the property on the HTMLElement. Example. So in Ember's case when you have a dynamic value that is being used on a "user input" element you would need to use {{prop}}. This would include properties such as value on <input />, checked on <radio />, selected on <option />. Happy to give more examples.

chadhietala avatar Mar 23 '18 14:03 chadhietala

@chadhietala Would disabled and checked be other examples where you'll have to use {{prop}}?

balinterdi avatar Mar 23 '18 20:03 balinterdi

I dislike this for DX reasons mostly. It's pushing a lot of manual labor to developers and additional complexity to the templates. I'd argue that the templates will get unreadable with all of the sigils in them, it's already somewhat hard to read some templates when there is a lot of stuff going on.

Also, do we really need yet another way to define actions / events? I think our current setup is complex enough:

image

I guess my main issue is that the current setup mostly works and is pretty convenient to work with. It feels like this proposal optimizes for the edge cases instead of the "90%" use case. Maybe an alternative would be to go the other way, meaning leave the heuristics as they are (or improve if necessary) and provide tools (like {{prop}} or {{on}}) to specify what we want to do in the instances we need to.

piotrpalek avatar Mar 25 '18 14:03 piotrpalek

@balinterdi Yes.

@piotrpalek For what it's worth I help maintain an app that has 120,000 lines of handlebars (2589 templates). I did a quick audit of just <input /> and <option /> usage, of the 752 usages (268 <option /> and 484 <input />) identified roughly 54 places where {{prop}} would need to be used.

chadhietala avatar Mar 25 '18 16:03 chadhietala

@chadhietala I've tried to collect my thoughts about this proposal:

What I don't like:

  • it will force existing and new Ember.js users to learn the (muddy) attributes vs properties html semantics
  • it makes using Ember.js less convenient
  • it adds complexity to things we already have (event listeners/action handlers)
  • it increases (imo significantly) the learning curve of Ember

What I wish it would looks like:

  • I would expect of Ember to do the right thing for the common case by default (as it does now)
  • Maybe these element modifiers could be added, but we wouldn't deprecate/remove/discourage the current behavior? This would allow their usage for those special cases without forcing new/existing users to learn when to use which
  • If the current semantics are unclear perhaps I'd be good to specify them somewhere to clear it up (could also have the benefit of exposing uncosistencies / bugs)
  • If the Custom Components use case is the most important one, maybe we could use the same way Glimmer.js distinguishes between properties and attributes (@ sigil vs lack of it)

Useful discussions from the React ecosystem:

  • https://github.com/facebook/react/issues/7249
  • https://github.com/facebook/react/issues/11347

piotrpalek avatar Mar 25 '18 18:03 piotrpalek

While I'm not opposed to this RFC per-se, I think we're approaching it the wrong way.

I'd much rather see #112 advance and hold this back. Once we have element modifiers we should experiment with the proposed API in this RFC (which sounds reasonable to me) in addon-land and eventually promote it into core.

But in any case I'm agains adding more {{bind-attr}}-like magic until that power is generally available to users.

cibernox avatar Mar 27 '18 12:03 cibernox

Is there still interest in moving this forward? If so, I can help get it finalized.

wagenet avatar Jul 23 '22 17:07 wagenet