-
Notifications
You must be signed in to change notification settings - Fork 95
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Possible size reduction #359
Comments
Thanks @X-Ryl669 for the suggestion! I think lots of size optimization ideas can make the For instance, I tried to apply the ideas of aliasing the result of |
@X-Ryl669 is a useful tool: https://github.com/andrewiggins/gz-heatmap |
Also, dom.replaceWith('') is not equivalent to dom.remove() as the former will leave an empty text node underneath the DOM tree. |
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. |
Actually, I tried to use 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 |
Thanks a lot, when I looked before I noticed that |
In this code
you can avoid the
dom.remove()
by simply doingdom.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 :And finally,
k.startsWith("on")
appears twice in the same method, and is longer than an aliaslet o=k.startsWith('on')
so it can be further optimized away.Anyway, thanks for the library, it's very useful.
The text was updated successfully, but these errors were encountered: