linkedin / dustjs

Asynchronous Javascript templating for the browser and server
http://dustjs.com
MIT License
2.91k stars 479 forks source link

Bug with parser and JavaScript. #445

Closed trusktr closed 10 years ago

trusktr commented 10 years ago

I found a situation where a single commented line inside a code block causes the parser to incorrectly assume an unintentional {dust} tag.

e.g. Writing Dust.js/JavaScript like

    {<footer}
                    $("#voteTopics .voteTopic").each(function() {
                        // if id is empty, ti
                    });
     {/footer}

will get compressed and look like

{<footer}$("#voteTopics .voteTopic").each(function() {// if id is empty, ti});{/footer}

which I think is causing the {// if id is empty, ti} to be considered as a closing tag when it's not supposed to be, so Dustjs complains about not finding the actual closing tag:

Error: Expected end tag for footer but it was not found. At line : 161, column : 65
jimmyhchan commented 10 years ago

This is caused by the line comment. We have a bug for this already I believe

Workaround for now is to use the other comment style /* */

Another workaround is to avoid using inline script tags. Instead use script src

trusktr commented 10 years ago

I'm using dustjs-linkedin with Node.js, and I'm not sure if it's Dust.js that's compressing stuff, but if I remove that comment line or put some actual JavaScript there, then it works, and looking at the output in my browser shows all JavaScript is compressed (minified), so that's why I guessed compression might have to do with the problem. Either that or Dust.js ignores white space.

However, if I replace the comment with /footer and delete the last tag, I still get the same error:

    {<footer}
                    $("#voteTopics .voteTopic").each(function() {
                        /footer
                    });
trusktr commented 10 years ago

Oh thanks @jimmyhchan. Any idea why replacing the line comment with /footer still doesn't work?

Maybe you guys can ignore tags of the form {//tag} when it has two slashes?

jimmyhchan commented 10 years ago

note sure what you mean. what is /footer?

I'm going to close this as a duplicate of #300. Feel free to continue the conversation here or in #300.

trusktr commented 10 years ago

@jimmyhchan

The /footer was referring to my error. Dust couldn't find the {/footer} closing tag so I thought if I tried putting /footer in place of the comment like

{
/footer
}

that Dust would compile it to {/footer}. Didn't work though.

Does Dust minify my template into one line? Or is it something else that's doing it before it gets parsed by Dust?

jimmyhchan commented 10 years ago

Dust strips white space. If you need a new line use {~n}

kate2753 commented 10 years ago

I'm not sure if this is related to whitespace stripping. I've tried using /* style comments without success. Template below can't be parsed by Dust and throws Syntax error

Error: Expected end tag for footer but it was not found. At line : 2, column : 47

{<footer}
  $("#voteTopics .voteTopic").each(function() {
    /*if id is empty, ti*/
  });
{/footer}

It looks like { at the end of the 2nd line is considered a reference start.

Interesting to note is that adding a line of code after comment fixes things:

{<footer}
  $("#voteTopics .voteTopic").each(function() {
    /*if id is empty, ti*/
    var i = 2;
  });
{/footer}

Using Dust comments works fine:

{<footer}
  $("#voteTopics .voteTopic").each(function() {
    {!if id is empty, ti!}
  });
{/footer}

Another workaround is to use {~lb} and {~rb} instead of { and }

{<footer}
  $("#voteTopics .voteTopic").each(function() {~lb}
    /*if id is empty, ti*/
  {~rb});
{/footer}
trusktr commented 10 years ago

@kate2753 Thanks for the tips. I end up having to use {~lb} and {~rb} inside regexes, which is a little inconvenient. I'm also getting the error using block comments, e.g.

        $(document).ready(function() {
            /*Scripts here*/
        });

Hopefully a fix for this gets realized in issue #300!