jQuery templating with ES6/ES2015 template strings

Question

I have been using a particular pattern over and over again when I want to keep my jQuery apps more safe from XSS. I do something like this. (It uses ES6 template strings)

function doSomething ($el, user) {
  var $html = $(`
  <div>
    <img src="${user.img}">
    <div class="username-text"></div>
    <div class="description"></div>
  </div>
`);
  $html.appendTo($el);
  $html.find('.username-text').append(document.createTextNode(user.name));
  $html.find('.description').append(document.createTextNode(user.description));

  return $html;
}

var user = {
  name: "John", 
  description: "<script>alert('XSS vulnerability');</script>", 
  img: 'http://placehold.it/100x100'
};

doSomething($('body'), user);

JS Bin

User defined fields become text nodes which means I need to create a jQuery element or some sort of DOM structure to hold them. I also use find which means at least in jQuery I have to append it to the actual DOM itself. So I was thinking it would be nice to have a more general way of nesting jQuery elements (I guess you could say "components") inside the template itself kind of like React's JSX but for jQuery.

This is what I came up with. (It uses a tagged template literal which returns a jQuery object).

var jQTemplate = (function ($) {

  var tempFiller = function (key) {
    return `<div id='jQT-jQTemplate-tempFiller-${key}'></div>`;
  };

  var jQTemplate = function (strings, ...values) {
      var $docFrag = $(document.createDocumentFragment());
      var nodeIndexes = [];
      var html = "";
    for(let i = 0; i < strings.length; ++i) {
      html += strings[i];

      var value = values[i];
      var allNodes = Array.isArray(value) && value.every(el=>el instanceof Node || el instanceof jQuery)
      if(value) {
        if(value instanceof Node || value instanceof jQuery || allNodes) {
          nodeIndexes.push(i);
          html += tempFiller(i);
        } else if(value) {
          html += value;// || '';
        }
      }
    }
    $docFrag.append(html);
    nodeIndexes.forEach(function (i) {
      var value = values[i];
      $($docFrag[0].getElementById(`jQT-jQTemplate-tempFiller-${i}`))
    .replaceWith(value);
    });

      return $docFrag;
    };

  return {
    template: jQTemplate,
    textNode: (text) => document.createTextNode(text)
  };

}($));

Here is some example usage: (not the best "app" but shows the different ways I intended it to be used)

/*
* Example usage
*
*/

var {template, textNode} =  jQTemplate;


console.log("started");

function bootstrapButton (type, text) {
  return $(`<a class="btn btn-${type || "default"} btn-lg" href="#" role="button">${text || ""}</a>`)
}

function jumbotron () {
  var $learnMore = bootstrapButton("primary", "Add Content");

  $learnMore.on('click', function (el) {
    $("body").append('<p>Lorem Ipsum. <i class="close fa fa-times js-deleteParent"></i></p>');
  })

  return template`
<div class="jumbotron">
  <h1>Hello, world!</h1>
  <p>...</p>
  <p>${$learnMore}</p>
</div>
`;
}

$("body").on("click", '.js-deleteParent', function () {
  $(this).parent().remove();
});

var userInput = ['<script>alert("hello world")</script>', "more user input"];

var trustedHtmlContent = [`<h1>Header</h1>`, `<h3>Sub Header</h3>`];

var extraClass ="container";

template`
  <div class="${extraClass}">
    <div>
      ${trustedHtmlContent.map(el=>$(el))}
      ${userInput.map(el=>textNode(el))}
     </div>
     ${jumbotron()}
  </div>
`.appendTo("body");

JS Bin

Any thoughts, suggestions, or comments? Is there any reason why this shouldn't be used in production?

Answer 1

I was reading through your code and I got to

  if(value) {
    if(value instanceof Node || value instanceof jQuery || allNodes) {
      nodeIndexes.push(i);
      html += tempFiller(i);
    } else if(value) {
      html += value;// || '';
    }
  }

and I had to read over it a few times, but I think that you can do away with the outside if statement or at least not check value a second time.

either one of these look like they are going to do about the same thing

if (value instanceof Node || value instanceof jQuery || allNodes) {
    nodeIndexes.push(i);
    html += tempFiller(i);
} else if (value) {
    html += value;
}

this would go through both checks, much the same way that the original code did, except you don't check the existence of value first, this might cause some errors, so I would probably do this instead

if (value) {
    if (value instanceof Node || value instanceof jQuery || allNodes) {
      nodeIndexes.push(i);
      html += tempFiller(i);
    } else {
      html += value;
    }
}

turned the else if into an else, because we already know that value is going to pass a truthy test, no need to check it a second time.