Repository: buxlabs/boxwood (js)

Issue:
Local variables are overriden
```html
<foo bar="{['baz']} />
```

would not work if there was a `bar` variable passed to the parent component

Pull request:
Inlining arrays
Closes #590 

Patch:
diff --git a/src/Component/curlyTagReduction.js b/src/Component/curlyTagReduction.js
index b054f05..efde74a 100644
--- a/src/Component/curlyTagReduction.js
+++ b/src/Component/curlyTagReduction.js
@@ -11,7 +11,7 @@ const {
 } = require('astoptech')
 const { isCurlyTag, getTagValue, curlyTag } = require('../utilities/string')
 const { parse, stringify } = require('../utilities/html')
-const { inlineAttributesInIfStatement } = require('../utilities/inline')
+const { inlineLocalVariablesInTags } = require('../utilities/inline')
 const walk = require('himalaya-walk')
 const { addPlaceholders, removePlaceholders } = require('../utilities/keywords')
 const { GLOBAL_VARIABLE } = require('../utilities/enum')
@@ -172,7 +172,7 @@ function curlyTagReduction (string, variables) {
         return variable
       })
     ]
-    inlineAttributesInIfStatement(node, vars, true)
+    inlineLocalVariablesInTags(node, vars, true)
     if (node.type === 'text') {
       node.content = optimizeText(node.content, variables, newVariables)
     } else if (attributes && attributes.length > 0) {
diff --git a/src/utilities/collect.js b/src/utilities/collect.js
index 5802363..c2dfc41 100644
--- a/src/utilities/collect.js
+++ b/src/utilities/collect.js
@@ -8,7 +8,7 @@ const walk = require('himalaya-walk')
 const { SPECIAL_TAGS, SELF_CLOSING_TAGS } = require('./enum')
 const { join, dirname } = require('path')
 const { parse } = require('./html')
-const { inlineAttributesInIfStatement, inlineLocalVariablesInFragment, inlineExpressions } = require('./inline')
+const { inlineLocalVariables, inlineExpressions } = require('./inline')
 const { clone } = require('./object')
 const { isImportTag } = require('./string')
 const Component = require('../Component')
@@ -135,8 +135,7 @@ function collectComponentFromPath (path, fragment, assets, context, plugins, err
 function inlineData (htmlTree, content, path, assets, plugins, errors, localVariables, options) {
   walk(htmlTree, leaf => {
     leaf.context = path
-    inlineLocalVariablesInFragment(leaf, localVariables)
-    inlineAttributesInIfStatement(leaf, localVariables)
+    inlineLocalVariables(leaf, localVariables)
   })
   walk(htmlTree, leaf => {
     inlineExpressions(leaf, localVariables)
diff --git a/src/utilities/inline.js b/src/utilities/inline.js
index bab08f6..427e6fa 100644
--- a/src/utilities/inline.js
+++ b/src/utilities/inline.js
@@ -7,37 +7,7 @@ const { convertToExpression } = require('./convert')
 const { normalize } = require('./array')
 const { GLOBAL_VARIABLE } = require('./enum')
 
-const CONDITION_TAGS = ['if', 'elseif', 'unless', 'elseunless']
-
-function inlineAttributesInIfStatement (node, localVariables, remove) {
-  // TODO we should handle switch etc.
-  if (CONDITION_TAGS.includes(node.tagName)) {
-    const normalizedAttributes = normalize(node.attributes)
-    node.attributes = normalizedAttributes.map(attr => {
-      // TODO handle or remove words to numbers functionality
-      if (attr.type === 'Identifier' && !isCurlyTag(attr.key)) {
-        // TODO this does not handle computed values
-        // we should create a new abstract-syntax-tree and get the key that way
-        // instead of string manipulation
-        const key = attr.key.includes('.') ? attr.key.split('.')[0] : attr.key
-        if (key === GLOBAL_VARIABLE) { return attr }
-        const variable = localVariables && localVariables.find(variable => variable.key === key)
-        if (variable && variable.local) {
-          return attr
-        } else if (variable && isCurlyTag(variable.value)) {
-          attr.key = `${variable.value}`
-        } else if (!variable && remove) {
-          attr.key = '{void(0)}'
-        } else {
-          attr.key = `{${attr.key}}`
-        }
-      }
-      return attr
-    })
-  }
-}
-
-function inlineLocalVariablesInFragment (node, variables) {
+function inlineLocalVariablesInText (node, variables) {
   if (node.type === 'text') {
     variables.forEach(variable => {
       if (!isCurlyTag(variable.value)) {
@@ -45,6 +15,9 @@ function inlineLocalVariablesInFragment (node, variables) {
       }
     })
   }
+}
+
+function inlineLocalVariablesInAttributes (node, variables) {
   if (node.attributes && node.attributes.length > 0) {
     node.attributes.forEach(attribute => {
       if (isCurlyTag(attribute.key)) {
@@ -69,6 +42,56 @@ function inlineLocalVariablesInFragment (node, variables) {
   }
 }
 
+const CONDITION_TAGS = ['if', 'elseif', 'unless', 'elseunless']
+const LOOP_TAGS = ['for', 'each', 'foreach']
+function inlineLocalVariablesInTags (node, localVariables, remove) {
+  // TODO we should handle switch etc.
+  if (CONDITION_TAGS.includes(node.tagName)) {
+    const normalizedAttributes = normalize(node.attributes)
+    node.attributes = normalizedAttributes.map(attr => {
+      // TODO handle or remove words to numbers functionality
+      if (attr.type === 'Identifier' && !isCurlyTag(attr.key)) {
+        // TODO this does not handle computed values
+        // we should create a new abstract-syntax-tree and get the key that way
+        // instead of string manipulation
+        const key = attr.key.includes('.') ? attr.key.split('.')[0] : attr.key
+        if (key === GLOBAL_VARIABLE) { return attr }
+        const variable = localVariables && localVariables.find(variable => variable.key === key)
+        if (variable && variable.local) {
+          return attr
+        } else if (variable && isCurlyTag(variable.value)) {
+          attr.key = `${variable.value}`
+        } else if (!variable && remove) {
+          attr.key = '{void(0)}'
+        } else {
+          attr.key = `{${attr.key}}`
+        }
+      }
+      return attr
+    })
+  } else if (LOOP_TAGS.includes(node.tagName)) {
+    // TODO this approach is not great, e.g. would not work for items with whitespace in strings
+    // the variable will also be reused inside of the loop
+    // we should probably do a transpilation step of the source code first
+    // to host variables, give them a name and reuse
+    // this way we can utilize other code paths without needing to come here
+    const attribute = node.attributes[node.attributes.length - 1]
+    const variable = localVariables && localVariables.find(variable => variable.key === attribute.key)
+    if (variable) {
+      const value = getTagValue(variable.value)
+      if (isSquareTag(value)) {
+        attribute.key = value.replace(/\s+/g, '')
+      }
+    }
+  }
+}
+
+function inlineLocalVariables (node, variables) {
+  inlineLocalVariablesInText(node, variables)
+  inlineLocalVariablesInAttributes(node, variables)
+  inlineLocalVariablesInTags(node, variables)
+}
+
 function inlineExpressions (leaf, localVariables) {
   if (localVariables.length === 0) { return null }
   if (leaf.attributes) {
@@ -124,7 +147,7 @@ function inlineExpressions (leaf, localVariables) {
 }
 
 module.exports = {
-  inlineAttributesInIfStatement,
-  inlineLocalVariablesInFragment,
+  inlineLocalVariablesInTags,
+  inlineLocalVariables,
   inlineExpressions
 }
diff --git a/src/utilities/inline.spec.js b/src/utilities/inline.spec.js
index 12bc0cc..16466c4 100644
--- a/src/utilities/inline.spec.js
+++ b/src/utilities/inline.spec.js
@@ -1,25 +1,25 @@
 'use strict'
 
 const test = require('ava')
-const { inlineLocalVariablesInFragment } = require('./inline')
+const { inlineLocalVariables } = require('./inline')
 
-test('#inlineLocalVariablesInFragment inlines variables in the text', assert => {
+test('#inlineLocalVariables inlines variables in the text', assert => {
   const node = { type: 'text', content: 'hello, {foo}', attributes: [] }
   const variables = [{ key: 'foo', value: 'world' }]
-  inlineLocalVariablesInFragment(node, variables)
+  inlineLocalVariables(node, variables)
   assert.deepEqual(node.content, 'hello, world')
 })
 
-test('#inlineLocalVariablesInFragment inlines variables in the attribute keys', assert => {
+test('#inlineLocalVariables inlines variables in the attribute keys', assert => {
   const node = { type: 'text', content: 'hello', attributes: [{ key: '{foo}' }] }
   const variables = [{ key: 'foo', value: 'world' }]
-  inlineLocalVariablesInFragment(node, variables)
+  inlineLocalVariables(node, variables)
   assert.deepEqual(node.attributes, [{ key: 'foo', value: 'world' }])
 })
 
-test('#inlineLocalVariablesInFragment inlines variables in the attribute values', assert => {
+test('#inlineLocalVariables inlines variables in the attribute values', assert => {
   const node = { type: 'text', content: 'hello', attributes: [{ key: 'foo', value: '{foo}' }] }
   const variables = [{ key: 'foo', value: 'world' }]
-  inlineLocalVariablesInFragment(node, variables)
+  inlineLocalVariables(node, variables)
   assert.deepEqual(node.attributes, [{ key: 'foo', value: 'world' }])
 })


Test patch:
diff --git a/test/fixtures/acceptance/styleguide/actual.html b/test/fixtures/acceptance/styleguide/actual.html
new file mode 100644
index 0000000..ee8aca5
--- /dev/null
+++ b/test/fixtures/acceptance/styleguide/actual.html
@@ -0,0 +1,6 @@
+<import layout from="layouts/default.html"/>
+<import content from="./partials/content.html" />
+
+<layout>
+  <content/>
+</layout>
diff --git a/test/fixtures/acceptance/styleguide/components/header.html b/test/fixtures/acceptance/styleguide/components/header.html
new file mode 100644
index 0000000..807f6db
--- /dev/null
+++ b/test/fixtures/acceptance/styleguide/components/header.html
@@ -0,0 +1,5 @@
+<ul>
+  <for item of menu>
+    <li><a href="{item.href}">{item.text}</a></li>
+  </for>
+</ul>
diff --git a/test/fixtures/acceptance/styleguide/data.json b/test/fixtures/acceptance/styleguide/data.json
new file mode 100644
index 0000000..1fb9a2b
--- /dev/null
+++ b/test/fixtures/acceptance/styleguide/data.json
@@ -0,0 +1,6 @@
+{
+  "menu": [
+    { "href": "/", "text": "Start" },
+    { "href": "/login", "text": "Logowanie" }
+  ]
+}
diff --git a/test/fixtures/acceptance/styleguide/expected.html b/test/fixtures/acceptance/styleguide/expected.html
new file mode 100644
index 0000000..5aec28c
--- /dev/null
+++ b/test/fixtures/acceptance/styleguide/expected.html
@@ -0,0 +1,8 @@
+<html>
+  <body>
+    <ul>
+      <li><a href="/">Start</a></li>
+      <li><a href="/login">Login</a></li>
+    </ul>
+  </body>
+</html>
diff --git a/test/fixtures/acceptance/styleguide/layouts/default.html b/test/fixtures/acceptance/styleguide/layouts/default.html
new file mode 100644
index 0000000..eb80bcc
--- /dev/null
+++ b/test/fixtures/acceptance/styleguide/layouts/default.html
@@ -0,0 +1,5 @@
+<html>
+  <body>
+    <slot/>
+  </body>
+</html>
diff --git a/test/fixtures/acceptance/styleguide/partials/content.html b/test/fixtures/acceptance/styleguide/partials/content.html
new file mode 100644
index 0000000..30a35ee
--- /dev/null
+++ b/test/fixtures/acceptance/styleguide/partials/content.html
@@ -0,0 +1,10 @@
+<import header from="components/header.html"/>
+
+<header
+  menu="{
+    [
+      { href: '/', text: 'Start' },
+      { href: '/login', text: 'Login' }
+    ] 
+  }"
+/>
diff --git a/test/spec/acceptance.spec.js b/test/spec/acceptance.spec.js
index edf52e4..18981cb 100644
--- a/test/spec/acceptance.spec.js
+++ b/test/spec/acceptance.spec.js
@@ -153,6 +153,11 @@ test('acceptance: globals-env', async assert => {
   assert.deepEqual(actual, expected)
 })
 
+test.only('acceptance: styleguide', async assert => {
+  const { actual, expected } = await suite('styleguide')
+  assert.deepEqual(actual, expected)
+})
+
 test.skip('acceptance: slot as a variable', async assert => {
   const { actual, expected } = await suite('slot-as-variable')
   assert.deepEqual(actual, expected)
diff --git a/test/spec/tags/for.spec.js b/test/spec/tags/for.spec.js
index 1e7c52c..687fda8 100644
--- a/test/spec/tags/for.spec.js
+++ b/test/spec/tags/for.spec.js
@@ -362,3 +362,23 @@ test('for: inlining with an object', async assert => {
   `)
   assert.deepEqual(template({}, escape), 'quxquux')
 })
+
+test('for: inline array', async assert => {
+  const { template } = await compile(`
+    <template foo>
+      <for car and index of ['BMW','Hyundai']>#{index + 1}{car}</for>
+    </template>
+    <foo />
+  `)
+  assert.deepEqual(template({}, escape), "#1BMW#2Hyundai")
+})
+
+test('for: inline object', async assert => {
+  const { template } = await compile(`
+    <template foo>
+      <for key and value in {qux:'quux'}>{key}{value}</for>
+    </template>
+    <foo />
+  `)
+  assert.deepEqual(template({}, escape), "quxquux")
+})