dom-expressions icon indicating copy to clipboard operation
dom-expressions copied to clipboard

Potential savings in attributes transpilation

Open Exelord opened this issue 2 years ago • 8 comments

Is there any reason why empty attributes <div empty /> are getting transpiled into empty=""? As I understand by default HTML understands empty as "" value. Getting rid of ="" could reduce the build size while still persisting the same effect.

https://github.com/ryansolid/dom-expressions/blob/62bd00e61c2cf217cae818be21ace06a32c8dfd0/packages/babel-plugin-jsx-dom-expressions/src/dom/element.js#L528

- results.template += value ? `="${value.value}"` : `=""`; 
+ results.template += value ? `="${value.value}"` : ""; 

I also noticed another case that could prevent generating more templates. In many cases the templates differs only by attributes.

    <>
      <div>
        <div class="bang">
          Hello
          <span>!</span>
        </div>
      </div>
      <div>
        <div class="h1">
          Hello
          <span>!</span>
        </div>
      </div>
    </>

Currently this syntax would generate 2 the same templates with just a different attribute value:

const _tmpl$ = template(`<div><div class="bang">Hello<span>!</span></div></div>`, 6),
      _tmpl$2 = template(`<div><div class="h1">Hello<span>!</span></div></div>`, 6);

We could optimize it by treating attributes always as dynamic:

setAttribute(_el$2, "my-attr", "value");

That why we could reduce build size drastically reducing the amount of templates.

I am aware that currently this can be achieved by wrapping each attr with template literals but this is inconvenient for DX

Exelord avatar Aug 28 '21 14:08 Exelord

We use innerHTML to generate template elements and sometimes it corrects bad HTML my previous mechanism was to string equal the template vs the result in the innerHTML in dev mode. The browser adds the ="" so I had done that to match. Now I just count the number of tags which is less precise but means that probably doesn't matter to do anymore.

I've always optimized for runtime speed over size, even if negligible. I wonder how much we get back from gzip anyway which should be able to reduce duplication in the template strings pretty handedly. While calling helpers definitely can be reduced more by the minifier I think it would depend a lot on the type of templates you are using.

ryansolid avatar Aug 29 '21 05:08 ryansolid

Thanks Ryan for your answers! :)

The browser adds the ="" so I had done that to match.

Hmm, when I set empty string attribute on the element using pure browser API, it does not add ="". Did I miss some something?

I've always optimized for runtime speed over size, even if negligible. I wonder how much we get back from gzip anyway which should be able to reduce duplication in the template strings pretty handedly. While calling helpers definitely can be reduced more by the minifier I think it would depend a lot on the type of templates you are using.

I think in bigger apps, with a lot of templates this make a huge difference in delivery. Do you have an access to some mature Solid app, or some OSS project? We could do some benchmarking to validate :)

Exelord avatar Aug 30 '21 12:08 Exelord

Hmm, when I set empty string attribute on the element using pure browser API, it does not add ="". Did I miss some something?

Yes try setting the boolean attribute in an innerHTML and then look what is generated. Like create template with just the boolean property naked. The HTML it generates will have the ="" or at least it did in every browser I checked about 2 years ago.

Yeah I don't have anything of that size to make a comparisn. But I do know that cloning is faster than separately setting from microbenchmarks.

ryansolid avatar Aug 30 '21 15:08 ryansolid

That's correct. The inner html as text will contain the ="" however the nodes will not have it. Screen Shot 2021-08-30 at 22 23 58

Setting innerHTML as '<div empty string=""></div>' is exactly the same as '<div empty="" string=""></div>'

I think this could save some size without any side effect. It's still a valid HTML to pass just the attribute without assignment so browsers will interpret it anyway as ="". I guess it's like const test = undefined vs const test. Implicit value assignment.

But I do know that cloning is faster than separately setting from microbenchmarks.

That's true. Im just wondering about the impact the duplicated templates will have on the bundled size. I know that is some apps the templates can be like 80% of the size with very complex structure. It might be the case that the cost of setting attributes can be justified in order to trim the bundle by eg. 50%.

There is also one more option instead of using setAttribute we could interpolate the template with attributes to create the final template. I think that would lead to template compilation where structure is shared and all static data is precompiled- not sure what do you think about that.

Exelord avatar Aug 30 '21 20:08 Exelord

An example of how we could use template literals for that:

function template(strings, ...keys) {
  return (function(...values) {
    let dict = values[values.length - 1] || {};
    let result = [strings[0]];
    keys.forEach(function(key, i) {
      let value = Number.isInteger(key) ? values[key] : dict[key];
      result.push(value, strings[i + 1]);
    });
    return result.join('');
  });
}

const genericTemplate = template`<button ${0}><div>${1}</div><span></span></button>`

const html = genericTemplate('my-attr="value"', 'Hello there!')

Exelord avatar Aug 30 '21 20:08 Exelord

Yeah for the ="" as I was saying it was mostly because I needed the string comparison to be equal. Now that I don't that's probably an easy reduction. So I see no issue with us removing that.

The template thing is trickier. I looked at stuff like bytecodes etc doing my own encoding and I wasn't happy with the performance. And mostly that I needed to bring in the code to do it which was larger than the gains I was shaving from the template. Strings really have the benefit of being easily Gzippable. And I felt all I was doing was implementing my own compression algorithm.

I think there are some heuristics that make sense. Like if templates are sufficiently small like a single element we probably could look at using document.createElement and attribute helpers assuming that those helpers would be there anyway and wouldn't add more code size. Right now template sharing would only be applicable within the same file. And I don't see it applying to partials but whole sections. Like if the initial example here is in a div instead of fragment no gain as it would be in a single template. But it also seems complicated for a scenario that I'm not sure how often you'd hit or would get reasonably handled by GZip. I'm not saying I can't picture this scenario but I suspect the wins aren't clear cut.

ryansolid avatar Aug 30 '21 21:08 ryansolid

I've removed the ="" from 0.29.15.

ryansolid avatar Sep 01 '21 05:09 ryansolid

Awesome! Thank you Ryan for tackling this so quickly! 🎉

The template thing is trickier. I looked at stuff like bytecodes etc doing my own encoding and I wasn't happy with the performance. And mostly that I needed to bring in the code to do it which was larger than the gains I was shaving from the template. Strings really have the benefit of being easily Gzippable. And I felt all I was doing was implementing my own compression algorithm.

Yes, I know what you are talking about :/ Ember.js (glimmer) is doing the template compilation to bytecode. Although it trimmed down the bundle size in bigger apps it increased the runtime drastically. That's why I suggested the template literals as they seams like a good compromise between compilation and runtime performance.

Let me do some benchmarks with a few ideas that we have here, and get back with results on small components, a website and small app.

Exelord avatar Sep 02 '21 08:09 Exelord

Seems like this issue can be closed

orenelbaum avatar Dec 19 '22 14:12 orenelbaum

Well I only ever did half of the suggestion and the OP was going to investigate the other half. But I agree this one is stale.

ryansolid avatar Dec 20 '22 18:12 ryansolid