cleanup, switch value and key positions for object loop

Author: Jeff Escalante

README.md

@@ -192,7 +192,7 @@ Output:
 And an example using an object:
 
 ```html
-<each loop="key, value in anObject">
+<each loop="value, key in anObject">
   <p>{{ key }}: {{ value }}</p>
 </each>
 ```

lib/index.js

@@ -125,13 +125,16 @@ function walk (opts, nodes) {
 
     // parse loops
     if (node.tag === loops[0]) {
+      // handle syntax error
       if (!(node.attrs && node.attrs.loop)) {
         throw new Error(`the "${conditionals[1]}" tag must have a "loop" attribute`)
       }
+
       // parse the "loop" param
       const loopParams = parseLoopStatement(node.attrs.loop)
       const target = vm.runInContext(loopParams.expression, ctx)
 
+      // handle additional syntax errors
       if (typeof target !== 'object') {
         throw new Error('You must provide an array or object to loop through')
       }
@@ -140,41 +143,19 @@ function walk (opts, nodes) {
         throw new Error('You must provide at least one loop argument')
       }
 
+      // run the loop, different types of loops for arrays and objects
       if (Array.isArray(target)) {
         for (let index = 0; index < target.length; index++) {
-          const value = target[index]
-          // add value and optional index loop locals
-          const scopedLocals = {}
-          scopedLocals[loopParams.keys[0]] = value
-          if (loopParams.keys[1]) scopedLocals[loopParams.keys[1]] = index
-          // merge nondestructively into existing locals
-          const scopedOptions = merge(opts, { locals: scopedLocals })
-          // provide the modified options to the content evaluation
-          // we need to clone the node because the normal operation modifies
-          // the node directly
-          const content = cloneDeep(node.content)
-          const res = walk(scopedOptions, content)
-          m.push(res)
+          m.push(executeLoop(loopParams.keys, target[index], index, node, opts))
         }
-        return m
       } else {
         for (let key in target) {
-          const value = target[key]
-          // add item and optional index loop locals
-          const scopedLocals = {}
-          scopedLocals[loopParams.keys[0]] = key
-          if (loopParams.keys[1]) scopedLocals[loopParams.keys[1]] = value
-          // merge nondestructively into existing locals
-          const scopedOptions = merge(opts, { locals: scopedLocals })
-          // provide the modified options to the content evaluation
-          // we need to clone the node because the normal operation modifies
-          // the node directly
-          const content = cloneDeep(node.content)
-          const res = walk(scopedOptions, content)
-          m.push(res)
+          m.push(executeLoop(loopParams.keys, target[key], key, node, opts))
         }
-        return m
       }
+
+      // return directly out of the loop, which will skip the "each" tag
+      return m
     }
 
     // return the node
@@ -196,6 +177,10 @@ function getNextTag (nodes, i, nodeCount) {
   return [i, { tag: undefined }]
 }
 
+/**
+ * Given a "loop" parameter from an "each" tag, parses out the param names and
+ * expression to be looped through using a mini text parser.
+ */
 function parseLoopStatement (input) {
   let current = 0
   let char = input[current]
@@ -250,3 +235,22 @@ function parseLoopStatement (input) {
     return res
   }
 }
+
+/**
+ * Creates a set of local variables within the loop, and evaluates all nodes
+ * within the loop, returning their contents
+ */
+function executeLoop (loopParams, p1, p2, node, opts) {
+  // two loop locals are allowed
+  // - for arrays it's the current value and the index
+  // - for objects, it's the value and the key
+  const scopedLocals = {}
+  scopedLocals[loopParams[0]] = p1
+  if (loopParams[1]) scopedLocals[loopParams[1]] = p2
+  // merge nondestructively into existing locals
+  const scopedOptions = merge(opts, { locals: scopedLocals })
+  // walk through the contents and run replacements with modified options
+  // we need to clone the node because the normal operation modifies
+  // the node directly
+  return walk(scopedOptions, cloneDeep(node.content))
+}

test/fixtures/loop_object.html

@@ -1,5 +1,5 @@
 <p>x</p>
-<each loop='key, value in items'>
+<each loop='value, key in items'>
   <p>{{key}}: {{value}}</p>
 </each>
 <p>x</p>

test/index.js

@@ -75,6 +75,10 @@ test('loop object', (t) => {
   return matchExpected(t, 'loop_object', { locals: { items: { a: 'b', c: 'd' } } })
 })
 
+test.todo('loop with other locals included')
+test.todo('loop with conflicting locals')
+test.todo('nested loops')
+
 //
 // Utility
 //