ractivejs / ractive

Next-generation DOM manipulation
http://ractive.js.org
MIT License
5.94k stars 396 forks source link

question about preserveWhitespace #3165

Open Jeff17Robbins opened 6 years ago

Jeff17Robbins commented 6 years ago

Description:

The preserveWhitespace option collapses or compresses multiple whitespace characters into a single one. While this is what HTML does under some circumstances, it can turn make the formatting of templated HTML inadvertently significant. Was this the intent of the default value false for preserveWhitespace?

Versions affected:

edge (and presumably earlier)

Platforms affected:

Windows 10, but assuming all platforms

Reproduction:

https://jsfiddle.net/jfsnwac7/

HTML

<script id=template type="text/ractive">
  <div id=container1>
    <div style="display: inline-block;">
      <span>text 1</span>
    </div>

    <div style="display: inline-block;">
      <span>text 2</span>
    </div>
  </div>
</script>

<div id=target>
</div>

<div id=container2>
  <div style="display: inline-block;">
    <span>text 3</span>
  </div>

  <div style="display: inline-block;">
    <span>text 4</span>
  </div>
</div>

JavaScript

var r = new Ractive({
  target: '#target',
  template: '#template'
});

Output

preservewhitespace

Desired Output

desired preservewhitespace

To get the desired output with straight HTML, it is of course the author's problem. Solvable in several ways, ranging from "format the HTML without any intervening whitespace (either by putting it all on one line or by using HTML comments to separate the lines)" to "use CSS on the container div (I've seen font-size:0, display:flex, and even a contrived web font which as an empty space character)".

One could argue that a template under Ractive control has the same choices available. But I would have thought, given the preserveWhitespace option, which defaults to false, that Ractive would allow me to format my template with whitespace between elements and have an option to thoroughly ignore this whitespace.

Ought the current preserveWhitespace option already do this, and simply has a bug?

Or is this issue merely a feature request?

If the latter, I would point out that other JavaScript libraries, like Polymer, seem to offer this convenience:

https://www.polymer-project.org/2.0/docs/devguide/dom-template

Remove empty text nodes Add the strip-whitespace boolean attribute to a template to remove any empty text nodes from the template's contents. This can result in a minor performance improvement.

What's an empty node? strip-whitespace removes only text nodes that occur between elements in the template and are empty (that is, they only contain whitespace characters). These nodes are created when two elements in the template are separated by whitespace (such as spaces or line breaks). It doesn't remove any whitespace from inside elements.

[In my example the container's nodelist is NodeList(3) [div, text, div]

And since that text node is empty by Polymer's definition, the Polymer option removes it. That's exactly the functionality I wish I had in Ractive.

I know. Just cuz the other kids are doing it doesn't make it right. 😄

fskreuz commented 6 years ago

Quick rundown: preserveWhitespace init option is essentially Ractive.parse()'s preserveWhitespace option. What this does is tell whether or not the parser should preserve whitespace information on the AST.

Ractive just collapses multiple sequential whitespaces with preserveWhitespace: false and I think it's by design. It's best to follow how the browser behaves to avoid surprises. Stripping the whitespace is convenient but breaks expectations. Imagine how many hours and hair are wasted by developers trying to figure out what's happening, only to find out it's documented in some dark corner of the documentation.

As far as I know, flexbox and CSS grids are the official features that address this and several other layout deficiencies in CSS. Best to use the right APIs instead of working around the issue.

But if this is a popular request, we could expand whitespace treatment. I think preserveWhitespace, which is limited to 2 options, with a new option that can do more.

{
  whitespace: "collapse", // collapse whitespace
  whitespace: "preserve", // preserve whitespace
  whitespace: "strip" // remove whitespaces where applicable
}
Jeff17Robbins commented 6 years ago

I agree with your reasoning. It is definitely a nice-to-have.

That said, I cited the Polymer example to show that other templating libraries offer this convenience option.

I'd love to see Ractive offer the same level of convenience, but defer to others as to how to prioritize it.

Best, Jeff

On Mon, Dec 18, 2017, 9:39 AM Joseph notifications@github.com wrote:

Quick rundown: preserveWhitespace https://ractive.js.org/api/#preservewhitespace init option is essentially Ractive.parse() https://ractive.js.org/api/#ractiveparse's preserveWhitespace option. What this does is tell whether or not the parser should preserve whitespace information on the AST. https://ractive.js.org/playground/#N4IgFiBcoE5SAbAhgFwKYGcUgL4BoQN4A3JGAAhiQGMUBLYtARnIF5yAlG+xgCmAA6AOwHoEkcgBMA9tQCuAWzRCUAOgBG0yQE88w0UgAOh5ZIkoYctHqHly6BYeToJAA30pRAHkMA+D6KiABJoCAjSeOQA6tIwCJIAhAEoXgD0fsk+-iKeuQCCYeTa0nIU6kgYaORkVeqh0kIA5vbS5HIYmenZoq42doYwmGgwjFFgdOgYhjRoEgBmSAiVwjgAlPpC1A0Y0ghoquGNvADkA0MjaGMTmNPUs+QLS9b2aI7OaMeRVLQMzKoOTlQaHWm22u32hxOZ0qFyuk1u90elUiYBQCgQn0o3F+TH+0iCABUALIAGV4qxBwlIFG+PDQACY2JxsXxBDlQhIZPIlCoNFpdB4jCYhGZ7JZrMI7AD3m5MhkcoEUCEwhForF4kkFSkunLurlRAUEEUSmUKlUauQ6uEmi02h0tWl5T0+uRocNRuN4TNzOKVpTQUIdnsDtIjqdBjCPdcpt6xVZItKgZjab96f9XoD0CCtoHwSGw27YZ6brGLPHyKj0cmWQy8YTSeTVrggA

  • preserveWhitespace: false - Default. Collapse sequential whitespaces to one space, trim ends.
  • preserveWhitespace: true - Preserve the whitespaces during parsing (i.e. leave \n, \t etc. alone).

Ractive just collapses multiple sequential whitespaces with preserveWhitespace: false and I think it's by design. It's best to follow how the browser behaves to avoid surprises. Stripping the whitespace is just convenience but breaks expectations. Imagine how many hours and hair are wasted by developers trying to figure out what's happening, only to find out it's documented in some dark corner of the documentation.

As far as I know, flexbox and CSS grids are the official features that address this and several other layout deficiencies in CSS. Best to use the right APIs instead of working around the issue.

But if this is a popular request, we could expand whitespace treatment. I'd think the following would make sense, a new option to expand whitespace treatment:

{ whitespace: "collapse", // collapse whitespace whitespace: "preserve", // preserve whitespace whitespace: "strip" // remove whitespaces where applicable }

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ractivejs/ractive/issues/3165#issuecomment-352443594, or mute the thread https://github.com/notifications/unsubscribe-auth/ACAWhwL5GvbFi8eY1M9DxoauEEtwvoV6ks5tBnkVgaJpZM4RFedO .

paulie4 commented 6 years ago

Polymer's docs also say that stripping out the whitespace has "a minor performance improvement," which sounds nice.

evs-chris commented 6 years ago

I have definitely had some fights with Ractive templates and whitespace before, but consistent whitespace stripping is not as easy as I had hoped the first time I encountered some spacing weirdness. The issue is that Ractive has lots of potential for corner-cases that may cause breakage in templates e.g. <div>{{#if foo}}...{{/if}} <span>should the space there be removed?</span></div> where, depending on the contents of the if block, you may expect the space before the span to be removed. Interpolators and triples are also tricky constructs to deal with for this sort of thing.

If you limit it specifically to space between literal element tags and/or text, that's probably not too hard to achieve. It would just require some lookahead in the template cleanup function, but there would still be some odd cases left over that may be surprising.

Jeff17Robbins commented 6 years ago

In my example, the problem stems from me wanting to be able to pretty-print my template, with extra newlines for clarity.

Whereas I picture your example corner case (space before that <span>) would be all on one line in the template.

So maybe there's some distinction between stripping whitespace between lines vs whitespace on a given line that would make for a simpler solution?