wavesoft / dot-dom

.dom is a tiny (512 byte) template engine that uses virtual DOM and some of react principles
Apache License 2.0
806 stars 53 forks source link

Missing return annotation on callLifecycleMethods() #58

Closed mindplay-dk closed 4 years ago

mindplay-dk commented 4 years ago

Looks like an @returns annotation is missing on callLifecycleMethods?

It seems the closure and .map() call aren't just returning something here because the syntax is shorter, but the return-value is actually being used for something during reconciliation?

mindplay-dk commented 4 years ago

It looks like the return value of this function is treated as truthy just to enable saving some bytes - it's return value isn't actually used for anything.

You may be able to save 1 byte in expressions that rely on this, by using the comma-operator, e.g. callLifecycleMethods(), somethingElse() instead of callLifecycleMethods() && somethingElse() which should make those call sites a little less fragile as well? 🙂

mindplay-dk commented 4 years ago

During the unmount phase at the end of render, you can shave some bytes by unrolling it from an expression to a good ol'fashioned loop:

  for (let key in _old_cache) {
    if (!_new_cache[key]) {
      callLifecycleMethods(
        _old_cache[key].u,
        _old_cache[key]
      );
      render(
        [],
        dom.removeChild(_old_cache[key])
      )
    }
  }

Easier on the eyes, right? 😏

That snippet goes down from 142 -> 135 bytes (after terser) and you'll have to add curly braces and a semicolon to separate it from the previous expression, so 138 bytes, 4 bytes saved.

But it'll also perform better - you won't be coercing the useless array to a boolean, and for..in is faster than .map, so worth while? 🙂

wavesoft commented 4 years ago

Oh, I love how you dig into this 😄

Looks like an @ returns annotation is missing on callLifecycleMethods? ... It looks like the return value of this function is treated as truthy

Yes, that is correct.

That snippet goes down from 142 -> 135 bytes (after terser) and you'll have to add curly braces and a semicolon to separate it from the previous expression, so 138 bytes, 4 bytes saved.

Can you also pass it through the compression and check? If the compressed result is smaller, please open a PR with your changes, targeting the devel/0.4.0 branch. It would be nice to have your contribution 😉

You can also enable brotli with:

ENABLE_BROTLI=1 yarn build

I am asking because the .map trick is actually used to help the compression algorithm (at a cost to the performance). By using .map as my only array iteration routine I increase the expression entropy and the compression algorithm will use fewer bits to store its value, hence achieving better result.

mindplay-dk commented 4 years ago

I might submit a PR later, but tbh I'm kind of terrified of touching this code 😂

I'm on a different mission today - I'm actually trying to unroll all your minified code and turn this into regular, readable code.

I know that's probably not very interesting to you, but the size is a secondary concern for me - I just really like your approach to stateful components, and I'm curious to understand the code better. So I might uncover a few things in the process.

wavesoft commented 4 years ago

Nice! Looking forward to your comments 😄