ryansolid / dom-expressions

A Fine-Grained Runtime for Performant DOM Rendering
MIT License
850 stars 122 forks source link

Prefer to set property when property & attribute are identical #319

Open qeleb opened 2 months ago

qeleb commented 2 months ago

Changes

  1. It saves 5+ bytes for every single attribute that's instead set as a property, so I've added a list of reflected attributes to the Properties list.
  2. Fix 1 typo in a comment

Bundle size win

Example code

render(
  () => (
    <>
      <a id={Math.random() > 0.5 ? 'linkA' : 'linkB'}>link</a>                   {/* id */}
      <button name={Math.random() > 0.5 ? 'name a' : 'name b'}>name btn</button> {/* name */}
      <link rel={Math.random() > 0.5 ? 'preload' : 'preconnect'}>link</link>     {/* rel */}
      <img src={logo} alt={Math.random() > 0.5 ? 'a img' : 'b img'} />           {/* src */}
      <input type={Math.random() > 0.5 ? 'text' : 'password'} />                 {/* type */}
    </>
  ),
  document.body
);

OLD output

(() => {
  return [
    ((c = G()), e(() => a(c, 'id', Math.random() > 0.5 ? 'linkA' : 'linkB')), c),
    ((r = k()), e(() => a(r, 'name', Math.random() > 0.5 ? 'name a' : 'name b')), r),
    ((o = z()), e(() => a(o, 'rel', Math.random() > 0.5 ? 'preload' : 'preconnect')), o),
    ((n = E()),
    a(n, 'src', 'data:image/svg+xml,somesvg'),
    e(() => a(n, 'alt', Math.random() > 0.5 ? 'a img' : 'b img')),
    n),
    ((t = B()), e(() => a(t, 'type', Math.random() > 0.5 ? 'text' : 'password')), t),
  ];
}, document.body);

NEW output (-0.1KB)

(() => {
  return [
    ((c = A()), e(() => (c.id = Math.random() > 0.5 ? 'linkA' : 'linkB')), c),
    ((r = G()), e(() => (r.name = Math.random() > 0.5 ? 'name a' : 'name b')), r),
    ((o = k()), e(() => (o.rel = Math.random() > 0.5 ? 'preload' : 'preconnect')), o),
    ((n = O()),
    (n.src = 'data:image/svg+xml,somesvg'),
    e(() => (n.alt = Math.random() > 0.5 ? 'a img' : 'b img')),
    n),
    ((t = E()), e(() => (t.type = Math.random() > 0.5 ? 'text' : 'password')), t),
  ];
  var t, n, l, o, r, c, f;
}, document.body);

Notes

  1. There are more reflected properties to be added in the future for further gains, although some properties like maxlength (string)/maxLength (number) will require slightly more effort to coerce types
ryansolid commented 2 months ago

Trying to think if there is a downside. When I started Solid I actually used all properties but we switched to attributes for consistency reasons. We still use properties for webcomponents. So trying to remember why we made this choice. I might have to do some digging to remind myself of why.

qeleb commented 2 months ago

Trying to think if there is a downside. When I started Solid I actually used all properties but we switched to attributes for consistency reasons. We still use properties for webcomponents. So trying to remember why we made this choice. I might have to do some digging to remind myself of why.

Ah please do. I’ve been writing prop:src=, prop:alt=, & prop:href= for months at work and I was hoping this is something the compiler could take on specifically for the cases where the attribute & property have the exact same behavior.

We definitely cannot do this for all of them though— there are some tricky ones like checked where the attr:checked === prop:defaultChecked (but not prop:checked)

there is a similarly weird case for the value property.

To the best of my understanding, this optimization is safe for all IDL attribute reflection properties so I made a very careful list of only a few.

qeleb commented 2 months ago

@ryansolid Have you gotten a chance to review this? I’ve been taking a look at how Preact handles this and it’s interesting. I may follow their approach

It seems to specially handle value/defaultValue, checked/defaultChecked, then set everything as properties except for href because of this quirk (although this can still be handled with some consideration) & a few others

qeleb commented 2 months ago

@ryansolid Would you be willing to merge this optimization if it were opt-in only like omitNestedClosingTags?

ryansolid commented 1 month ago

@ryansolid Would you be willing to merge this optimization if it were opt-in only like omitNestedClosingTags?

That might be a good way to start it. Mostly I don't want to break in preparation for SolidStart release.