vanjs-org / van

🍦 VanJS: World's smallest reactive UI framework. Incredibly Powerful, Insanely Small - Everyone can build a useful UI app in an hour.
https://vanjs.org
MIT License
3.77k stars 87 forks source link

Possible size reduction #359

Open X-Ryl669 opened 2 weeks ago

X-Ryl669 commented 2 weeks ago

In this code you can avoid the dom.remove() by simply doing dom.replaceWith('') So it can be replaced as let update = (dom, newDom) => newDom === dom || dom.replaceWith(newDom||'')

Also, there is many time the document variable in the script. Maybe it's worth creating an alias, since it won't be renamed by the minifier code, like this :

let dom = ns ? document.createElementNS(ns, name) : document.createElement(name)
// instead:
let d=document,dom = ns ? d.createElementNS(ns, name) : d.createElement(name)
// Or better:
let n = ns&&'NS', doc = document['createElement'+n](ns||name,name)

And finally, k.startsWith("on") appears twice in the same method, and is longer than an alias let o=k.startsWith('on') so it can be further optimized away.

Anyway, thanks for the library, it's very useful.

Tao-VanJS commented 2 weeks ago

Thanks @X-Ryl669 for the suggestion!

I think lots of size optimization ideas can make the .min.js bundle smaller, but will indeed increase the gzipped bundle size. This is because gzip compression will compress the repetitive patterns in the input text and some aliasing optimization will eliminate the repetitive patterns in the code.

For instance, I tried to apply the ideas of aliasing the result of k.startsWith("on"), it will indeed increase the gzipped bundle size by 1 byte to 1049 bytes. Thus we need to try out every optimization idea to assess its ultimate impact on the gzipped bundle size.

sirenkovladd commented 2 weeks ago

@X-Ryl669 If you have enough time you can play around with these suggestions, I was able to find a couple of bytes of optimization by changing the order of variables or variable names but I don't think it's worth it.

is a useful tool: https://github.com/andrewiggins/gz-heatmap

Tao-VanJS commented 2 weeks ago

Also,

dom.replaceWith('')

is not equivalent to

dom.remove()

as the former will leave an empty text node underneath the DOM tree.

X-Ryl669 commented 2 weeks ago

Is it really a problem ?

Tao-VanJS commented 1 week ago

Is it really a problem ?

It can be a problem. Think about the use case like a TODO list, where lots of items are added to the list and then deleted. Eventually it will leave lots of dummy nodes in the DOM tree. Thus this is basically a kind of memory leak.

sirenkovladd commented 1 week ago

Actually, I tried to use replaceWith('') for chromium based browser and this element does not stay in the DOM tree children elements are reduced

Do you have any example of a memory leak being possible?

Tao-VanJS commented 1 week ago

Do you have any example of a memory leak being possible?

Please see this example: https://jsfiddle.net/qsvz2abc/2/ (a modified TODO list).

Basically, if we change .remove() to .replaceWith(""). The total count of the child nodes of the TODO list won't be decreased when a TODO item is removed.

sirenkovladd commented 1 week ago

Thanks a lot, when I looked before I noticed that replaceWith reduces the number of children but didn't notice that it doesn't reduce the number of childNodes