Whitespace is removed from dust rendered HTML

[kate2753]

Dust removes new lines from output HTML as well as any spaces. This is a problem for cases when we have <pre></pre> tags with pre-formatted HTML .

This example code in pre tag

<div class="row">{~n}
 <div class="content">{~n}
    <div class="row">{~n}
      <div class="facets">6</div>{~n}
      <div class="main-content">8</div>{~n}
      <div class="aside">{~n}
        <div class="row">{~n}
          <div class="summary">4</div>{~n}
          <div class="details">6</div>{~n}
        </div>{~n}
        10{~n}
      </div>{~n}
    </div>{~n}
    24{~n}
  </div>{~n}
</div>

will be rendered as :

<div class="row">{~n}
<div class="content">{~n}
<div class="row">{~n}
<div class="facets">6</div>{~n}
<div class="main-content">8</div>{~n}
<div class="aside">{~n}
<div class="row">{~n}
<div class="summary">4</div>{~n}
<div class="details">6</div>{~n}
</div>{~n}
10{~n}
</div>{~n}
</div>{~n}
24{~n}
</div>{~n}
</div>

Note, if I do not include {~n} everything is rendered as one line of text and browser handles line breaks.

Spaces are used for indentation for the code examples. Removing whitespace is making code examples hard to read. It is pretty hard to figure out HTML nesting without indentation.

And example for script tag is:

sometimes there are single line comments within script tag like:

<script>
//this will alert "test"
alert("this is test");
</script>

After dust compilation it will become :

<script>//this will alert "test" alert("this is test");</script>

This will basically break the code by commenting out that line of JavaScript.

[vybs]

@kate2753 we should {! for comments, but I understand that we strip out comments for minification etc.

[vybs]

@bgaff what are your thoughts on this?

History of changes :
#232

[kate2753]

@vybs good point we should use dust comments where possible.

The issue with script tag is more of an edge case. Although it causes issues that are hard to debug\figure out what's going on.

If script tag is used inline in HTML it's probably just couple of lines long and it would not make much difference if it's minimized or not. It performance is a concern then such script tag should be made into external JS and properly minified by JS Minifier ( which strips out comments altogether). So I'd argue that it's better to keep the code in script tag as is, removing newlines may cause more damage than help.

[vybs]

@kate2753 i meant we strip the comments in dust templates for performance

[vybs]

talking to @bgaff we figured a much cleaner solution.

Today we have
format: function(context, node) { return ".write(" + escape(node[1] + node[2]) + ")"; }

and this is the grammar rule, that tries to remove whitespaces and the newline with the join

buffer "buffer" = e:eol w:ws* { return ["format", e, w.join('')] } / b:(!tag !comment !eol c:. {return c})+ { return ["buffer", b.join('')] }

But note : this always preserves leading whitespaces.

We propose adding a new pragma block, and then make a compiler change to support the 2 pragmas to not strip the white space and new lines

{%ws_preserve} blah blah blah {/ws_preserve} {%preserve_format} blah blah blah preserve both newlines and leading whitespaces {/preserve_format}

[rragan]

Glad to see we have circled back to pragmas (#195) . This seems like the most flexible way to deal with it. I assume the scope is a single .dust template (e.g. wrapping a partial call with {%ws_preserve} would have no effect on the code in the partial. Are there any interactions when nesting these pragmas we need to think about. Too bad we have to give up a nice {% single character form for a fairly low use feature. Our char set is getting thin without doing doubling up ({%%xxx} {##...} whatever those might mean

[jairodemorais]

Guys, I think that the original author had any idea of how to handle the ws and carriage return but he never implemented it. I am saying this because he parses the ws and carriage return as "format" and he created a format function in the compiler (@vybs mentioned it in the comment above).
This function is never called because in the dust.optimizers the value on the key format is "nullify".

dust.optimizers = {
  body:      compactBuffers,
  buffer:    noop,
  special:   convertSpecial,
  format:    nullify,        // TODO: convert format
  reference: visit,

Maybe we can continue his work in place of create new rules like ws_preserve or %preserve_format.
It is just my opinion.

[eoneill]

I think you can resolve this and other whitespace issues by updating the regex used for stripping whitespace (in compiler.js) e.g.

source = source.replace(/^\s+/mg, '').replace(/\n/mg, '');

should be something like...

// (<\/(\w+)[^>]*>)  -  match all closing tags
// <(?!\/)  -  match the following opening tag
// ([^\S\r\n]*[\r\n]+[^\S\r\n]*)+  -  match newlines that contain only whitespace
// [^\S\n]+  -  match redundant whitespace (this intentionally excludes newlines, so we can't use \s)
source = source.replace(/(<\/(\w+)[^>]*>)([^\S\r\n]*[\r\n]+[^\S\r\n]*)+<(?!\/)/g, '$1\n<').replace(/(<\/(\w+)[^>]*>)[^\S\n]+<(?!\/)/g, '$1 <');

[vybs]

is it backwards compatible ? I dont think we can afford not to be.

[eoneill]

doubt it, but what's there now is fundamentally broken

we shouldn't have to find tricks to get plain-old-html to render correctly

these are all broken:

<textarea>
this is a simple
example of some
multi-line text
</textarea>

<script>
  var x = 5
  ++x;
  // arguably, this is bad javascript, but it's completely valid and shouldn't fail
</script>

<pre>
  some indented
    text
</pre>

<div style="white-space:pre;">
  now imagine if we set white-space:pre in CSS?
</pre>

[eoneill]

that said, the back-compat issue should only impact anyone using the {~n} special char, or expecting all whitespace to be removed

bump the version number, document the change, and call it a day

if you were so inclined, you could keep the existing behavior and make the new behavior "opt-in" via config

[jimmyhchan]

{~n} is part of the original Dust language. That said your examples have been broken for a very long while
(see http://akdubya.github.com/dustjs/) however "By default, Dust compresses whitespace by eliminating newlines and indentation. This behavior can be toggled at compile time."

indenting is lost so that Dust source files look good. Mustache seems to do something similar. Handlebars preserves indents. Don't know what Hogan does.

[vybs]

@eoneill
I dont agree that there is a need to do this, when there is a alternative
-- bump the version number, document the change, and call it a day

opt-in via a config is not always easy to rollout. I have been through that here at LI, want to take a stab.?

Second, I dont see any reason why having a pragma to explicitly say you want to preserve all kinds of formatting does not work.

[eoneill]

I guess my point is, if you're templating engine can't handle plain-old-html, it's fail

From the words of the original author...

• Precise: The parser should be accurate enough that designers rarely have to use escape sequences to achieve the desired result. Similarly, templates shouldn't mysteriously generate or eliminate whitespace.

If I can't trust that basic premise, how do I trust anything else in the engine?

I don't see why a config option would be an issue. We made whitespace stripping a config option in 1.2.0 anyway, why not extend it?

dust.compile = function(source, name, strip) {
  try {
    if (strip) {
      // if strip was set to `safe`, only remove redundant whitespace that will not impact rendering
      if(strip === 'safe') {
        // (<\/(\w+)[^>]*>)  -  match all closing tags
        // <(?!\/)  -  match the following opening tag
        // ([^\S\r\n]*[\r\n]+[^\S\r\n]*)+  -  match newlines that contain only whitespace
        // [^\S\n]+  -  match redundant whitespace (this intentionally excludes newlines, so we can't use \s)
        source = source.replace(/(<\/(\w+)[^>]*>)([^\S\r\n]*[\r\n]+[^\S\r\n]*)+<(?!\/)/g, '$1\n<').replace(/(<\/(\w+)[^>]*>)[^\S\n]+<(?!\/)/g, '$1 <');
      }
      // otherwise, aggressively strip whitespace
      else {
        source = source.replace(/^\s+/mg, '').replace(/\n/mg, '');
      }
    }
    ...
  }
  ...
};

now only if you call dust.compile(..., ..., 'safe') will you get the new change

no back-compat issue until you opt-in. and the only "issue" is that we'll have to go back and remove anywhere we had to abuse {~n} (e.g. in pre tags) as a workaround in the first place

this won't break things and fixes a fundamental issue, so I don't see why we would hesitate on this

[vybs]

We both agree that what we have as default in dust is agressive.
Second if are going the big bang, why not change the grammar rather than add boolean strip flag with regex

[dominykas]

Documentation sez:

Dust pre 1.2.0: Execute this before doing the dust.compile operation to retain whitespace. dust.optimizers.format = function(ctx, node) { return node };

Dust 1.2.0 was changed to preserve whitespace in the original template by default. If you want the behavior prior to 1.2.0, call the compiler with the third parameter strip as "true":
dust.compile = function(source, name, strip)
https://github.com/linkedin/dustjs/wiki/Dust-Tutorial#controlling-whitespace-suppression

This is not true - to preserve whitespace it is still required in 1.2.1 to dust.optimizers.format = function(ctx, node) { return node };

I see some comments on other issues that 1.2.1 was restored to pre-1.2.0 behavior, which puzzles me, but docs still need an update...

[vybs]

have updated the wiki. kindly review

[rragan]

The text reads fine and covers the 1.2.0 vs pre 1.2.0 whitespace handling.

I’m still confused though. I thought 1.2.0 was “backed out” because the change was incompatible but I still see it mentioned on the new home page. I’ve also seen mention of 1.2.1 which, as I recall, reverts the whitespace change. Help, I’m confused about the versions, what is officially supported and the plan for whitespace handling (having also seen pragma threads in the Issues).

Rich

From: Veena Basavaraj [mailto:[email protected]]
Sent: Wednesday, March 27, 2013 11:16 AM
To: linkedin/dustjs
Cc: Ragan,Richard
Subject: Re: [dustjs] Whitespace is removed from dust rendered HTML (#238)

have updated the wiki. kindly review

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/238#issuecomment-15542385.

[vybs]

@rragan 1.2.0 @smfoote change was not remvoed since it configurable.

@jairodemorais made a change which had bugs and it was backed out ( this was 1.2.1)

the pragma is not done yet and when done this GH ticket will be updated

Does it help?

[vybs]

Again,

See my comment on
History of changes :
#232

[dominykas]

I'm not convinced with the changed doc. It mentions prior/post 1.2.0 behavior - but at least in 1.2.1, one must do BOTH - the pre1.2.0 (i.e. change optimizers.format) AND post1.2.0 (i.e. pass in a param), or, in other words, the param has no effect on "optimizers.format" behavior. The param only controls WS stripping before compiling: see https://github.com/linkedin/dustjs/blob/master/lib/compiler.js#L5 - "strip" is never used again.

Also, worth noting, that dustc does not pass anything, i.e. it uses the "default" value, so the "default" value must be noted.

Also, worth noting, that IMHO stripping whitespace from HTML using a regex is a horrible idea.

[vybs]

@dymonaz I am not sure why this is getting overly complicated

Let me update the wiki.

prior to 1.2.0 it is the same as what the original dust was. NO changes period

1.2.0 , yes the strip was added as a config, but default remains the same.
IMHO stripping whitespace from HTML using a regex is a horrible idea. - I agree, the best way to do that is change the compiler and fix the dus.optimizers.format method as the original author indicates.

Does this make sense?

Lastly, IMO, and oen of earlier comments in this ticket says the same:

Add a pragma support if we do not want to change the default behaviour, so the regex can be removed.

[dominykas]

@vybs I'll rephrase... regardless if you pass true or false as the last param, the whitespace will be stripped in 1.2.1

Stripping happens twice:

1. dust.compile function has if (strip) { ... }
2. dust.optimizers.format is set to nullify

[vybs]

dust.compile function has if (strip) { ... }

if (true) should strip, why otherwise?

So I agree that we should remove the strip flag.

I will update the wiki with your comments

[vybs]

@smfoote Also, is there any unit tests that verufy that passing false to the strip does not strip the WS

[smfoote]

I did not create any explicit test, because all of the current tests implicitly set strip to false, and none of them are failing.

[rragan]

dust.compile takes 3rd param of strip.

Looking at https://github.com/linkedin/dustjs/blob/master/lib/dust.js
I see two calls to dust.compile. Neither call passes strip so these will default to old behavior of not stripping.

dust.compileFn = function(source, name) {
var tmpl = dust.loadSource(dust.compile(source, name));
....
}

dust.load = function(name, chunk, context) {
...
if (!dust.cache[name]) dust.loadSource(dust.compile(src, name));
...
}

I think I finally understand what went on. The compiler grammar was changed to preserve whitespace instead of stripping it.
Then to restore the old behavior the strip parameter was added which strips out all the whitespace the compiler might be inclined to keep before running the compiler.
So is it fair to say that sites wanting the old behavior must modify these two places in the source to add a 3rd parameter of "true" to force stripping? If so, it might be good to document how to revert the behavior for a site since source code changes need to be made rather than an option to control it exposed higher up.

[codeability-ab]

Hello!
I can only agree with jvanasco, this is a bug! And it really needs to be fixed!

Please also refer to: #343

My suggestion is that trailing space + newline + leading space is stripped to one space character.

My intention when inserting space between two words is to get space between them. Same thing when I hit newline!

Dust doesn't remove space between words. It should not remove newlines either.

best regards Johan

[rragan]

Just adding a comment from one our devs who got bitten by the attribute per line html case.

dust gang, we need a plan for this. It will keep biting web developers ad infinitum and they are the primary audience. I know any change in behavior will cause backward incompatibility. Having to wrap all of one's templates in a pragma or {` or whatever feels very artificial. We could make a change and bump the major version provided we can agree on what the change should be.

[smfoote]

I think we need to think very carefully about what type of whitespace stripping we do in Dust. If we replace (trailing whitespace + new line + leading whitespace) with a single space, we fix the "attribute per line" issue.

<a href="#"
   data-a="a"
   data-b="b">click</a>

becomes

<a href="#" data-a="a" data-b="b">click</a>

but

<div>
    <span>some text</span>
    <button>a button</button>
</div>

becomes

<div> <span>some text</span> <button>a button</button> </div>

which may be unexpected, and is definitely backwards incompatible. The extra spaces entered between tags (whether they should have been there in the first place or not) will significantly impact the layout of many pages.

We can't fix the "attribute per line" problem without introducing the spaces between tags problem. Although it may not be ideal, I think that not stripping any whitespace (which is already an option) does solve both problems.

Finally, the `{`` syntax from #334 is only tangentially related to this issue. The two issues should be considered separately.

[mattiaskarlsson]

IMHO whitespace stripping should be an optimization and not change the behaviour of the rendered page. Since HTML is whitespace sensitive whitespace stripping should not remove all whitespace between elements. If I as a developer does not want whitespace between elements I should probably not add them in the first place. Right now the default behaviour for Dust is to create HTML that might render in a way that was not intenden and in some cases that is completely broken.

If there is a use-case for whitespace stripping that acutally removes all whitespace it should be something that is explicitly enabled as a configuration option.

Cheers

[me-ascii]

@vybs, thank you very much for the
dust.optimizers.format = function(ctx, node) { return node };
now render 20% faster and allows newlines. wow!

[seriousManual]

If I as a developer does not want whitespace between elements I should probably not add them in the first place
That comment is a little shortsighted, consider the following template:

{?data}
    {#data}
        datapoint.         anotherDataPoint.
    {/data}
{:else}
   no data
{/data}

the resulting html looks like this:

       datapoint.         anotherDataPoint.

       datapoint.         anotherDataPoint.

       datapoint.         anotherDataPoint.

so if one is structuring dust templates in an readable and intuitive way white spaces are introduced automatically.

[jvanasco]

I think another way to consider this is to restate the problem and situation.

The original dust implementation handles whitespace "optimization" in a way that is fundamentally different from how HTML handles whitespace , yet dust is primarily designed for, and used in, HTML generation.

Both from [http://akdubya.github.io/dustjs/] :

By default, Dust compresses whitespace by eliminating newlines and indentation.

and

The Dust parser is finely tuned to minimize the amount of escaping that needs to be done within static text. Any text that does not closely resemble a Dust tag is considered static and will be passed through untouched to the template's output. This makes Dust suitable for use in templating many different formats. In order to be recognized as such, Dust tags should not contain extraneous whitespace and newlines.

HTML, however, has specific rules for whitespace

Referencing the HTML4 spec [ http://www.w3.org/TR/html401/struct/text.html ]

In HTML, only the following characters are defined as white space characters:

ASCII space ( )
ASCII tab ( )
ASCII form feed ()
Zero-width space (​)

Line breaks are also white space characters. Note that although 
 and 
 are defined in [ISO10646] to unambiguously separate lines and paragraphs, respectively, these do not constitute line breaks in HTML, nor does this specification include them in the more general category of white space characters.

This specification does not indicate the behavior, rendering or otherwise, of space characters other than those explicitly identified here as white space characters. For this reason, authors should use appropriate elements and styles to achieve visual formatting effects that involve white space, rather than space characters.

For all HTML elements except PRE, sequences of white space separate "words" (we use the term "word" here to mean "sequences of non-white space characters"). When formatting text, user agents should identify these words and lay them out according to the conventions of the particular written language (script) and target medium.

This layout may involve putting space between words (called inter-word space), but conventions for inter-word space vary from script to script. For example, in Latin scripts, inter-word space is typically rendered as an ASCII space ( ), while in Thai it is a zero-width word separator (​). In Japanese and Chinese, inter-word space is not typically rendered at all.

Note that a sequence of white spaces between words in the source document may result in an entirely different rendered inter-word spacing (except in the case of the PRE element). In particular, user agents should collapse input white space sequences when producing output inter-word space. This can and should be done even in the absence of language information (from the lang attribute, the HTTP "Content-Language" header field (see [RFC2616], section 14.12), user agent settings, etc.).

The PRE element is used for preformatted text, where white space is significant.

In order to avoid problems with SGML line break rules and inconsistencies among extant implementations, authors should not rely on user agents to render white space immediately after a start tag or immediately before an end tag. Thus, authors, and in particular authoring tools, should write:

<P>We offer free <A>technical support</A> for subscribers.</P>

and not:

<P>We offer free<A> technical support </A>for subscribers.</P>

Taking all this in, we're left with a whitespace optimization routine in Dust that expects and generates "dust" compatible whitespace rules -- which are not compatible with the specific whitespace rules of HTML.

I think the most beneficial fix would be to offer 2 whitespace optimization routines by default -- the classic "dust.js" method, and an HTML specific whitespace optimizer.

The HTML specific whitespace optimizer could be developed individually by users, but -

1. This is a core and common need across many people. It should be in base.
2. There are numerous dust forks ( this is the most popular ). building something like this as a core option will lead to better adherence and adoption.

Most people who use Dust.js will want and expect HTML compatible whitespace generation, not an independently written Dust.js stle of templating with it's own rules on whitespace sensitivity.

[codeability-ab]

Once again. I can only agree with jvanasco!

What are the chances that this is going to be fixed? Any time plan?

Best regards Johan

PS: I'm in a huge project building online banking services for swedens largest bank and we're seriously considering switching to another templating product. Sad but true!

[dominykas]

(slightly off topic) @jlkonsultab can you let me know which one? And why? I gave a talk in @dublinjs last month on why I changed from HBS to Dust (with pros/cons) - so I'm interested in figuring it out further. I am still of the opinion that dust is better by miles than anything out there... With all its glitches... [I'm happy to take this to twitter/G+/IRC/]

[codeability-ab]

@dymonaz We have evaluated Handlebars, Mustache and Google Closure templates. We're just in the beginning of converting from a combined "jquery append + home made templates" solution to Dust so it won't be a big deal changing for us if we do it now.

We haven't made any decisions yet but it's leaning towards Handlebars.

The whitespace bug is the one and only reason we're discussing a switch.

Most likely we will keep Dust but the whitespace thing is r-e-a-l-l-y annoying!

/johan

[dominykas]

Whitespace is easy enough to patch... I manually do that on every update... format: function(ctx, node) { return node },

Don't HBS :) Ember has patched some things up themselves, so they have less of that - but in general - I found it a major pain. https://speakerdeck.com/dymonaz/notes-from-the-great-dust-dot-js-migration?slide=6

[jvanasco]

@dymonaz @jlkonsultab We handle the whitespace issue as part of our dev & build processes. A python script "corrects" the raw templates, saving them to a ".min.dust" extension. ( It also fixes a handful of other errors or generates warnings that the templates are somehow incorrect -- like missing a space after a bootstrap style <i class="icon></i>_ placement). The ".min.dust" files are then used in the dev environment , and by the build/deploy process.

We had that general approach in place before moving to dust - so it wasn't much trouble to address whitespace in it. All our templates are packaged like this:

• filename.dust
• filename.dust#json [ sample json for parsing ]
• filename.dust#expected [ what we expect to see ]
• filename.dust#readme [ docs ]
• filename.min.dust [ generated ]

We check each template to ensure it can parse the expected #json payload and generate the #expected file. In the future we should have variants in there ( #json-1 = #readme-1 , etc ) , but this works rather well. Within this functionality, we first generate the "minified" dust template -- and that minified file is the source the rest of the project references.

Anyways, my point is that if you're doing any sort of test suites on your code, it's really simple to build in the minification.

I just think it's absolutely absurd to have a templating library aimed at an HTML Development audience that ignores common HTML whitespace rules and adopts its own.

[jleppert]

Why don't we favor an idiomatic approach?

-{ } = strips whitespace before
{ }- = strips whitespace after
-{ }- = strips whitespace both before and after

Using this convention, all whitespace, newlines, etc. would be left in the final output template. This isn't backwards compatible at all, but at this point, it's already broken right?

[rragan]

Anything outside the braces anywhere already goes straight to the output so I don't see how this could be remotely compatible or let me output {xxx}- , e.g. a value followed by a dash.

If we are looking for conventions, we could agree that dust compilation of a file with extension of .htm or .html would do so in whitespace preserving mode. No special params or syntax at all. Alas it does not self-identify. xxx.dusth anyone for dust with html whitespace?

[jleppert]

I think file extensions are not a good interface because who's to decide what extensions constitutes the alternate behavior? What if people want to have whitespace suppressed in part of a template, and preserved in another? You need whitespace control, not a hack. Maybe move the - inside the braces.

Or come up with some other simple grammar.

[rragan]

Twig has an approach somewhat like you mention.
http://twig.sensiolabs.org/doc/templates.html#whitespace-control

[trusktr]

@jvanasco

I just think it's absolutely absurd to have a templating library aimed at an HTML Development audience that ignores common HTML whitespace rules and adopts its own.

I agree. Is there an option to turn of any sort of minification for Dust.js?

[numinos1]

Is there still no solution to the whitespace problem? This issue has persisted for over a year, and I'm starting to wonder if it's time to start looking for another templating solution.

[trusktr]

@numinos1 See here for how to turn of removal of white space: #300 (comment)

[numinos1]

Awesome! Thanks for passing that on.

On Tue, Apr 1, 2014 at 3:07 AM, Joseph Orbegoso Pea <
[email protected]> wrote:

@numinos1 https://github.com/numinos1 See here for how to turn of
removal of white space: #300 (comment)#300 (comment)

Reply to this email directly or view it on GitHubhttps://github.com//issues/238#issuecomment-39184497
.

[peterbraden]

After spending far too long debugging this, I have pretty strong opinions on a broken minifier being on by default.

Until the dust parser can guarantee that it's not going to break inline js (at the moment it strips valid newlines) then the default format function should not strip spaces.

[seriousManual]

Why not leaving the compression issue to another more specialiced component?

One could run a template that is to be compiled through a minifier like https://www.npmjs.org/package/minify I have tested it, it works nicely together with dust, is aware of inline JS, inline CSS and also is able to actually minify HTML (not only stripping whitespaces).

[sethkinast]

Dust 2.5.0 includes a config directive.

Set dust.config.whitespace = true and your templates will be passed-through without any changes.

By default it's still set to false, but it will be changed in Dust 3 to be true by default.