googlearchive / TemplateBinding

TemplateBinding Prolyfill
290 stars 61 forks source link

remove a templateIteratorTable.delete(this) that looks like no-op #114

Closed jmesserly closed 11 years ago

jmesserly commented 11 years ago

As far as I can tell, the templateIteratorTable only uses Nodes as keys. So .delete using the TemplateIterator itself as the key doesn't do anything. Maybe this code is intending to go through the nodes created by the TemplateIterator instead, and remove the pointers back to it?

I tried to see if this code was live by adding a if (templateIteratorTable.get(this)) { throw '!!! templateIteratorTable present'; }. It was not hit by the tests (in Chrome and Firefox). So it looks like these two lines can be removed, if I'm understanding this code correctly.

arv commented 11 years ago

These should be delete(this.templateElement_) since we associate the iterator with the template element instance.

rafaelw commented 11 years ago

Thanks for pointing this out. Fixed here: https://github.com/Polymer/mdv/commit/4a7088431903d710046299f1df39d3af73810b30

jmesserly commented 11 years ago

awesome, thanks!