search

marcelklehr / vdom-virtualize

Virtualize a DOM node
MIT License
131 stars 27 forks source link

fromHTML adds nodes to the document, but doesn't delete them #6

Closed jfsiii closed 8 years ago

jfsiii commented 10 years ago

See this RequireBin.

250+ nodes were created in ~10 seconds: screen shot 2014-07-22 at 7 24 37 pm

I believe this is because DOM nodes are continually created but not reused or removed.

Please let me know if you see something wrong with my test case.

Raynos commented 10 years ago

I tried it with h() instead ( http://requirebin.com/?gist=4a0f6e2084efdd826cc6 ).

It looks like diff & patch do not leak.

The leak is probably here ( https://github.com/marcelklehr/vdom-virtualize/blob/9e2da942c787cd9f9c1c745f3a74a08d8158d471/index.js#L111-L112 ). I'm curious whether we are copying such properties as elem.firstChild and elem.previousSibling

marcelklehr commented 10 years ago

I guess that line's the problem. I just tried wrapping that value in a String(), but that didn't seem to help.

What's weird is that the nodes seems to be GC'd just fine the first few seconds for me, but then after about 6secs the count increases uncintrolled.

marcelklehr commented 10 years ago

The list at the end of the file (props) is the whitelist of all properties that are copied.

marcelklehr commented 10 years ago

Mh. GC'ing manually works for me, so I think it's not a memory leak.

paulocheque commented 8 years ago

I tried window.virtualize.fromHTML('<h1>teste</h1') and ir returns an html and body nodes with the h1 node inside. Is that necessary? Why dont return just the h1 node?

marcelklehr commented 8 years ago

parsing HTML correctly is hard, so I've removed fromHTML and recommend using https://github.com/TimBeyer/html-to-vdom instead -- If you want HTML parsing, use that. If you already have a DOM tree, virtualize will probably be more performant.

paulocheque commented 8 years ago

Thanks!