-
Notifications
You must be signed in to change notification settings - Fork 414
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
Using DOM Nodes instead of strings #80
base: master
Are you sure you want to change the base?
Conversation
I had also felt the need of storing actual dom nodes. however I'm also curious to understand why string approach has taken. @STRd6 Can you check how much memory your browser tab is consuming? |
Hello Thanks! This may be convenient for someone. I remember someone had already requested such feature. Compare this two tabs in chrome's task manager Because of that I can't replace main library with your implementation @STRd6, but we definitely can move it to separate branch and maintain there. I am guessing there are may be very specific situations when speed is more important than concerns about the memory p.s. I like how easily you went through sources. changes with minimal interference 👍 regarding height |
Using the same node |
content_elem.innerHTML = data; | ||
|
||
this.empty(content_elem); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey!
How about the following?
var fragment = document.createDocumentFragment();
var dataLength = data.length;
for (var i = 0; i < dataLength; i++) {
fragment.appendChild(data[i]);
}
content_elem.appendChild(fragment);
The document fragment should be faster
I think this would be nice to merge with the main branch. Would there be a reason why we couldn't support both Strings and actual Dom nodes? |
yeah, there is a reason, read previous comments |
Hey @STRd6 sounds like we're working on similar apps (here's the one I'm working on). I was hoping to do the same thing, as I'm using choo/bel/yo-yo, which creates dom elements. Curious that it's 5x slower though. |
@timwis did you manage to make it faster actually? |
I'm afraid I have absolutely no memory of this. Sorry! |
I'm making a spreadsheet app and rendering input elements inside table rows. The input values and event listeners were getting clobbered by being converted to strings and back.
In this PR I'm caching the real nodes and inserting/removing them. It works great for my purposes and may be of use to others but I understand if there may be reasons why the strings approach was taken.
It seems to work well in my limited test, but there's still an issue with the change on line 174 where we compute the height... I'm assuming the first three rows exist.
Anyway, thought I'd put up these changes in case you found them of interest!