Prevent including unneeded vars when in iterator with status var

marko-js · Jan 29, 2019 · 565dc73 · 565dc73
1 parent adee5b1
commit 565dc73
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 18 deletions.
diff --git a/src/taglibs/migrate/for-tag.js b/src/taglibs/migrate/for-tag.js
@@ -34,12 +34,15 @@ function migrateForLoop(elNode, context) {
         case "ForEach": {
             let varNamePrefix = "loop";
             let needsParams;
+            let isLegacyGenerator;
+
             elNode.params = [parsed.varName];
 
             if (parsed.iterator) {
                 if (parsed.statusVarName) {
                     // When we have both a status var and an iterator it's impossible to convert to a modern for loop.
                     // Instead we convert to the deprecated `<for of=generatorFunction>`.
+                    isLegacyGenerator = true;
                     elNode.params.push(parsed.statusVarName);
                     parsed.in = builder.functionCall(
                         builder.memberExpression(
@@ -110,23 +113,25 @@ function migrateForLoop(elNode, context) {
                 }
             }
 
-            if (parsed.separator) {
-                needsParams = true;
-                elNode.appendChild(
-                    builder.htmlElement(
-                        "if",
-                        undefined,
-                        [builder.text(parsed.separator, false, true)],
-                        `${varNamePrefix}Index !== ${varNamePrefix}All.length - 1`
-                    )
-                );
-            }
+            if (!isLegacyGenerator) {
+                if (parsed.separator) {
+                    needsParams = true;
+                    elNode.appendChild(
+                        builder.htmlElement(
+                            "if",
+                            undefined,
+                            [builder.text(parsed.separator, false, true)],
+                            `${varNamePrefix}Index !== ${varNamePrefix}All.length - 1`
+                        )
+                    );
+                }
 
-            if (needsParams) {
-                elNode.params.push(
-                    builder.identifier(`${varNamePrefix}Index`),
-                    builder.identifier(`${varNamePrefix}All`)
-                );
+                if (needsParams) {
+                    elNode.params.push(
+                        builder.identifier(`${varNamePrefix}Index`),
+                        builder.identifier(`${varNamePrefix}All`)
+                    );
+                }
             }
 
             elNode.setAttributeValue("of", parsed.in);

diff --git a/test/migrate/fixtures/legacy-for-syntax/snapshot-expected.marko b/test/migrate/fixtures/legacy-for-syntax/snapshot-expected.marko
@@ -49,12 +49,11 @@ $ input.iterator(colors, function(item) {
     <if(loopIndex !== loopAll.length - 1)>,</if>
 </for>
 <!-- Array | status-var iterator separator -->
-<for|color, loop, loopIndex, loopAll| of=input.iterator.bind(null, colors)>
+<for|color, loop| of=input.iterator.bind(null, colors)>
     ${color}${loop.getIndex() + 1}) of ${loop.getLength()}<if(loop.isFirst())>
         - FIRST
     </if>
     <if(loop.isLast())>- LAST</if>
-    <if(loopIndex !== loopAll.length - 1)>,</if>
 </for>
 <!-- Properties -->
 <for|color, code| in=colors>