googlearchive / TemplateBinding

TemplateBinding Prolyfill
290 stars 61 forks source link

Whitespace in templates #87

Open rafaelw opened 11 years ago

rafaelw commented 11 years ago

From jmesserly:

Hello Toolkitchen Cooks,

I was wondering about your experience so far with <template> element and whitespace. Jacob (CC'd) has tried to build an app using our web component/MDV shim, and he was rather frustrated that the shim was (correctly) preserving the whitespace in <template>. For example:

<element name="...">
  <template>
    <p>some shadow DOM content in here...</p>
  </template>
</element>

The HTMLTemplateElement.content will have a Text node with the newline+identation, both at the beginning and at the end.

Jacob went ahead and filed this bug: https://www.w3.org/Bugs/Public/show_bug.cgi?id=21017

It sounds like it's out of scope for the template spec. But could it be handled elsewhere in the system?

What are you folks planning to do about this? Do web developers ever want to preserve whitespace inside templates?

I'm torn: on one hand I want to reduce developer pain and make it easy to use templates. On the other hand, I'm worried that if our shim/polyfill removes all whitespace nodes in all templates automatically, we'll be diverging from the web standards.

Advice appreciated. :)

Cheers,

rafaelw commented 11 years ago

So would the idea be to remove all leading and trailing whiltespace nodes within a template?

jmesserly commented 11 years ago

@jacob314 and @sigmundch might be able to add more details... I think the heuristic was a bit more complicated, attempting to find "indentation space".

the comment on the implementation is something like this:

/**
 * Trims or compacts the leading/trailing white spaces of [text]. If the leading
 * spaces contain no line breaks, then all spaces are merged into a single
 * space. Similarly, for trailing spaces. These are examples of what this
 * function would return on a given input:
 *
 *       trimOrCompact('  x  ')          => ' x '
 *       trimOrCompact('\n\n  x  \n')    => 'x'
 *       trimOrCompact('\n\n  x       ') => 'x '
 *       trimOrCompact('\n\n  ')         => ''
 *       trimOrCompact('      ')         => ' '
 *       trimOrCompact(' \nx ')          => ' x '
 *       trimOrCompact('  x\n ')         => ' x'
 */

(source https://github.com/dart-lang/web-ui/blob/3008f4551d473cb8318e62339f90b375b5bd7d15/lib/src/utils.dart#L223)

sigmundch commented 11 years ago

Exactly. The heuristics were important so we can preserve intentional spaces a developer had written on a single line. For example we preserve the space between these two elements:

 <div></div> <span></span>

But we would remove it here:

 <div></div>
 <span></span>

We did something similar for content bindings. So this:

 <div></div>       {{x}}     {{y}}     <span></span>

Is converted to:

 <div></div> {{x}} {{y}} <span></span>

But this:

 <div></div>
{{x}}
{{y}}
 <span></span>

becomes

 <div></div>{{x}}{{y}}<span></span>
arv commented 11 years ago

I don't understand how you can do this? What if I have set the style to white-space: pre?

sigmundch commented 11 years ago

this was not done automatically: you had to explicitly indicate this was your intention for a template node. We opted for doing this with an attribute indentation="remove" to turn this on, or indentation="preserve" to turn it back off in a nested template.

jmesserly commented 11 years ago

yeah, we weren't removing whitespace by default, because that would violate HTML5 and <template> specs. It was an opt-in feature. Apparently it is supported by a lot of other template engines, though.

jyasskin commented 10 years ago

Here's a complete file that shows the problem:

<!DOCTYPE html>
<html>
<head>
  <script src="polymer.min.js"></script>
  <style>pre {background-color:red;}</style>
  <polymer-element name="my-code" noscript>
    <template>
      <style>:host { font-family: monospace; }</style>
      <content/>
    </template>
  </polymer-element>
</head>
<body>
  <pre><my-code>Some code!</my-code></pre>
</body>
</html>

I'd imagine this as part of the bindings layer, not <template> itself, and I'd have it cause inter-element whitespace (as opposed to a guess at indentation whitespace) to be dropped when the template is expanded. The effect would be similar to the /x regex flag (http://www.regular-expressions.info/freespacing.html). If the user wants significant whitespace inside a whitespace-stripping template, they should wrap it in another <template whitespace="preserve">.

jmesserly commented 10 years ago

I looked at performance implications of this using benchmark/codereview-diff.html, and it doesn't seem to make a huge difference anymore. So lowering priority.

@jyasskin -- here's the workaround http://jsbin.com/zotageha/1/edit ... it's a bit goofy but it can be done in an okay way. Same trick works for very performance sensitive templates: https://github.com/Polymer/TemplateBinding/blob/master/benchmark/codereview-diff.html#L173

jmesserly commented 10 years ago

in case the jsbin doesn't work for some reason, the code change was:

  <polymer-element name="my-code" noscript>
    <template
      ><style>:host { font-family: monospace; }</style
      ><content></content    
    ></template>
  </polymer-element>
ajklein commented 10 years ago

It's definitely possible to come up with cases where this does matter (see http://crbug.com/399816), but it may be reasonable to treat these as browser bugs (Firefox seems to handle this OK, for example).

jmesserly commented 10 years ago

adjusted priority -- discussing this with various folks, it sounds like the semantic issues worry people too (i.e. they don't want those extra nodes there, or having to worry that their programmer-friendly formatting != desired runtime formatting). So we should try and make a decision soon-ish.