search

BorisMoore / jsviews

Interactive data-driven views, MVVM and MVP, built on top of JsRender templates
http://www.jsviews.com/#jsviews
MIT License
856 stars 130 forks source link

removeView performance #447

Closed johan-ohrn closed 4 years ago

johan-ohrn commented 4 years ago

(I found this issue in the WIP cache version of jquery.views.js but I'm pretty sure it's in the current version as well)

at line 2920 in jquery.views.js:

try {
  prevNode.parentNode.removeChild(prevNode); // (prevNode.parentNode is parentElem, except if jQuery Mobile or similar has inserted an intermediate wrapper
  nextNode.parentNode.removeChild(nextNode);
} catch (e) { }

Executing this code with one of my templates take ~140ms

//try {
  prevNode && prevNode.parentNode.removeChild(prevNode); // (prevNode.parentNode is parentElem, except if jQuery Mobile or similar has inserted an intermediate wrapper
  nextNode && nextNode.parentNode.removeChild(nextNode);
//} catch (e) { }

Executing this code with the same templates take ~40ms

prevNode and? nextNode is undefined at times.

BorisMoore commented 4 years ago

Yes, you are right about the perf cost, and indeed I think we can simply replace that code by:

if (!viewToRemove._elCnt && prevNode) {
    prevNode.parentNode.removeChild(prevNode); // ...
    nextNode.parentNode.removeChild(nextNode);
}
BorisMoore commented 4 years ago

This has been fixed in new release v1.0.7