Code review – what the heck is that?!

Code-reviewing a proposed merge request from a guy who doesn’t usually do web development, but pitches in on minor bugs.  However, today, he surprised me with something I’d never seen before….

ng-show-start and ng-show-end.  I saw it in the code, saw what he intended, and nearly passed it by.  But then realized I’d never seen an AngularJS directive like that.  ng-show’s documentation didn’t mention any such type of use, and besides, that would be a new directive.  A hunt through our source code didn’t turn it up in either our angular.js source file (patched by the guys before me) or in any custom directive.  (Yes, I did look for ngShowStart, rather than ng-show-start…).  A hunt through Google showed a guy asking on StackOverflow how to do an ng-show-start, and getting told to wrap an ng-show in an outer element rather than do a -start / -end combination.

So I was about to bust his chops in the code review, but then realized he’d only repeated a pattern he saw in the code.  Pulled it up in Chrome using the Batarang plugin to see what more I could figure out.  All I discovered is that the code did what he intended, but I had no idea how it actually worked.  As in, to my knowledge, there was no way that code should be useful, but here it was doing what he intended……

I finally found my answer, nowhere near the ng-show documentation.  It turns out that _any_ directive can have this -start/-end behavior, thus making specific Google searches very non-useful.  There’s a multiElement attribute available, which

When this property is set to true, the HTML compiler will collect DOM nodes between nodes with the attributes <span class="pln">directive</span><span class="pun">-</span><span class="pln">name</span><span class="pun">-</span><span class="pln">start </span>and <span class="pln">directive</span><span class="pun">-</span><span class="pln">name</span><span class="pun">-</span><span class="kwd">end</span>, and group them together as the directive elements. It is recommended that this feature be used on directives which are not strictly behavioural (such as <span class="pln">ngClick</span>), and which do not manipulate or replace child nodes (such as <span class="pln">ngInclude</span>).

Not sure what words I can add to this posting to help make this easier to find for the next coder, but this counted as a couple of hours of my afternoon today…


Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <s> <strike> <strong>