Marko v3: Better handling of attribute placeholders

compiler/Builder.js

@@ -46,7 +46,6 @@ var Scriptlet = require('./ast/Scriptlet');
 
 var parseExpression = require('./util/parseExpression');
 var parseJavaScriptArgs = require('./util/parseJavaScriptArgs');
-var removeEscapeFunctions = require('./util/removeEscapeFunctions');
 var isValidJavaScriptIdentifier = require('./util/isValidJavaScriptIdentifier');
 
 var DEFAULT_BUILDER;
@@ -362,9 +361,6 @@ class Builder {
     parseExpression(str, options) {
         ok(typeof str === 'string', '"str" should be a string expression');
         var parsed = parseExpression(str, DEFAULT_BUILDER);
-        if (options && options.escapeXml === false) {
-            parsed = removeEscapeFunctions(parsed);
-        }
         return parsed;
     }
 

compiler/CodeGenerator.js

@@ -467,6 +467,8 @@ class Generator {
     }
 
     incIndent(count) {
+        this._flushBufferedWrites(true /* add separator */);
+
         if (count != null) {
             for (let i=0; i<count; i++) {
                 this.currentIndent += ' ';

compiler/CompileContext.js

@@ -12,6 +12,7 @@ var path = require('path');
 var Node = require('./ast/Node');
 var macros = require('./util/macros');
 var extend = require('raptor-util/extend');
+var Walker = require('./Walker');
 
 function getTaglibPath(taglibPath) {
     if (typeof window === 'undefined') {
@@ -370,6 +371,10 @@ class CompileContext {
     isPreserveComments() {
         return this._preserveComments === true;
     }
+
+    createWalker(options) {
+        return new Walker(options);
+    }
 }
 
 module.exports = CompileContext;

compiler/Parser.js

@@ -1,5 +1,6 @@
 'use strict';
 var ok = require('assert').ok;
+var AttributePlaceholder = require('./ast/AttributePlaceholder');
 
 var COMPILER_ATTRIBUTE_HANDLERS = {
     'preserve-whitespace': function(attr, context) {
@@ -16,6 +17,25 @@ function isIEConditionalComment(comment) {
     return ieConditionalCommentRegExp.test(comment);
 }
 
+function replacePlaceholderEscapeFuncs(node, context) {
+
+    var walker = context.createWalker({
+        exit: function(node, parent) {
+            if (node.type === 'FunctionCall' &&
+                node.callee.type === 'Identifier') {
+
+                if (node.callee.name === '$noEscapeXml') {
+                    return new AttributePlaceholder({escape: false, value: node.args[0]});
+                } else if (node.callee.name === '$escapeXml') {
+                    return new AttributePlaceholder({escape: true, value: node.args[0]});
+                }
+            }
+        }
+    });
+
+    return walker.walk(node);
+}
+
 class Parser {
     constructor(parserImpl) {
         ok(parserImpl, '"parserImpl" is required');
@@ -110,17 +130,19 @@ class Parser {
             selfClosed: el.selfClosed === true,
             pos: el.pos,
             attributes: attributes.map((attr) => {
-                var isLiteral = false;
-
+                var attrValue;
                 if (attr.hasOwnProperty('literalValue')) {
-                    isLiteral = true;
+                    attrValue = builder.literal(attr.literalValue);
+                } else if (attr.value == null) {
+                    attrValue = undefined;
+                } else {
+                    let parsedExpression = builder.parseExpression(attr.value);
+                    attrValue = replacePlaceholderEscapeFuncs(parsedExpression, context);
                 }
 
                 var attrDef = {
                     name: attr.name,
-                    value: isLiteral ?
-                        builder.literal(attr.literalValue) :
-                        attr.value == null ? undefined : builder.parseExpression(attr.value)
+                    value: attrValue
                 };
 
                 if (attr.argument) {

compiler/ast/AttributePlaceholder.js

@@ -0,0 +1,32 @@
+'use strict';
+
+var Node = require('./Node');
+
+class AttributePlaceholder extends Node {
+    constructor(def) {
+        super('AttributePlaceholder');
+        this.value = def.value;
+        this.escape = def.escape;
+    }
+
+    generateCode(codegen) {
+        codegen.generateCode(this.value);
+    }
+
+    walk(walker) {
+        this.value = walker.walk(this.value);
+    }
+
+    isCompoundExpression() {
+        return this.value.isCompoundExpression();
+    }
+
+    /**
+     * "noOutput" should be true if the Node.js does not result in any HTML or Text output
+     */
+    get noOutput() {
+        return this.value.noOutput;
+    }
+}
+
+module.exports = AttributePlaceholder;

compiler/ast/CustomTag.js

@@ -2,7 +2,6 @@
 
 var HtmlElement = require('./HtmlElement');
 var removeDashes = require('../util/removeDashes');
-var removeEscapeFunctions = require('../util/removeEscapeFunctions');
 var safeVarName = require('../util/safeVarName');
 var ok = require('assert').ok;
 
@@ -129,8 +128,7 @@ function buildInputProps(el, context) {
             return; // Skip over attributes that are not supported
         }
 
-        var attrValue = removeEscapeFunctions(attr.value);
-        handleAttr(attrName, attrValue, attrDef);
+        handleAttr(attrName, attr.value, attrDef);
     });
 
     // Imported variables are used to add input properties to a custom tag based on data/variables

compiler/ast/HtmlAttribute.js

@@ -3,7 +3,6 @@ var Node = require('./Node');
 var Literal = require('./Literal');
 var ok = require('assert').ok;
 var escapeXmlAttr = require('raptor-util/escapeXml').attr;
-var removeEscapeFunctions = require('../util/removeEscapeFunctions');
 var compiler = require('../');
 
 function isStringLiteral(node) {
@@ -11,14 +10,8 @@ function isStringLiteral(node) {
 }
 
 function isNoEscapeXml(node) {
-    return node.type === 'FunctionCall' &&
-        node.callee.type === 'Identifier' &&
-        node.callee.name === '$noEscapeXml';
-}
-
-function isStringExpression(node) {
-    return node.type === 'FunctionCall' && node.callee.type === 'Identifier' &&
-        (node.callee.name === '$noEscapeXml' || node.callee.name === '$escapeXml');
+    return node.type === 'AttributePlaceholder' &&
+        node.escape === false;
 }
 
 function flattenAttrConcats(node) {
@@ -46,7 +39,7 @@ function flattenAttrConcats(node) {
         }
 
         return {
-            isString: isStringLiteral(node) || isStringExpression(node),
+            isString: isStringLiteral(node) || node.type === 'AttributePlaceholder',
             concats: [node]
         };
     }
@@ -75,12 +68,8 @@ function generateCodeForExpressionAttr(name, value, escape, codegen) {
             } else if (part.type === 'Literal') {
 
             } else if (isNoEscapeXml(part)) {
-                part = removeEscapeFunctions(part);
                 part = codegen.builder.functionCall(codegen.builder.identifier('str'), [part]);
             } else {
-
-                part = removeEscapeFunctions(part);
-
                 if (escape !== false) {
                     var escapeXmlAttrVar = codegen.getEscapeXmlAttrVar();
                     part = codegen.builder.functionCall(escapeXmlAttrVar, [part]);
@@ -99,7 +88,6 @@ function generateCodeForExpressionAttr(name, value, escape, codegen) {
             escape = false;
         }
 
-        value = removeEscapeFunctions(value);
         let attrArgs = [codegen.builder.literal(name), value];
 
         if (escape === false) {

compiler/ast/Node.js

@@ -66,7 +66,7 @@ class Node {
 
     insertSiblingBefore(newNode) {
         ok(this.container, 'Node does not belong to a container: ' + this);
-        this.container.insertChildAfter(newNode, this);
+        this.container.insertChildBefore(newNode, this);
     }
 
     insertSiblingAfter(newNode) {

compiler/util/removeEscapeFunctions.js

@@ -1,17 +0,0 @@
-var compiler = require('../');
-
-function removeEscapeFunctions(node) {
-    var walker = compiler.createWalker({
-        exit: function(node, parent) {
-            if (node.type === 'FunctionCall' &&
-                node.callee.type === 'Identifier' &&
-                (node.callee.name === '$noEscapeXml' || node.callee.name === '$escapeXml')) {
-                return node.args[0];
-            }
-        }
-    });
-
-    return walker.walk(node);
-}
-
-module.exports = removeEscapeFunctions;

test/fixtures/compiler/autotest/attr-placeholder-escapeXml/expected.js

@@ -0,0 +1,17 @@
+function create(__helpers) {
+  var str = __helpers.s,
+      empty = __helpers.e,
+      notEmpty = __helpers.ne,
+      escapeXml = __helpers.x,
+      escapeXmlAttr = __helpers.xa;
+
+  return function render(data, out) {
+    out.w("<div foo=\"Hello " +
+      escapeXmlAttr(data.name) +
+      "\"></div>");
+
+    var foo = "Hello " + data.name;
+  };
+}
+
+(module.exports = require("marko").c(__filename)).c(create);

test/fixtures/compiler/autotest/attr-placeholder-escapeXml/foo-transformer.js

@@ -0,0 +1,6 @@
+module.exports = function(el, context) {
+    var builder = context.builder;
+    var fooValue = el.getAttributeValue('foo');
+
+    el.insertSiblingAfter(builder.var('foo', fooValue));
+};

test/fixtures/compiler/autotest/attr-placeholder-escapeXml/marko-taglib.json

@@ -0,0 +1,5 @@
+{
+    "<div>": {
+        "transformer": "./foo-transformer.js"
+    }
+}

test/fixtures/compiler/autotest/attr-placeholder-escapeXml/template.marko

@@ -0,0 +1 @@
+<div foo="Hello ${data.name}"></div>