Angular: `Component.preserveWhitespace = false` is too aggressive and not context aware

[cedx]

Same issue as the TypeScript flavor of Angular: https://github.com/angular/angular/issues/21049
(read the feed for the full explanation of the problem).

I've tried to apply preserveWhitespace: false to my components, and ALL OF THEM have rendering glitches (i.e. words linked together).

A markup like: Foo <strong>Bar</strong> Baz (display: "Foo Bar Baz")...
...is processed to: Foo<strong>Bar<strong>Baz (display: "FooBarBaz").

If I want to have a proper display of my texts, I need to add an insane amount of &ngsp;/ .

I think that preserveWhitespace: false should ignore the (first) preceding/leading whitespace before/after tags within an inline formatting context.

[matanlurey]

That's surprising that _all_ of them have glitches. Could you just use : true on some?

/cc @alorenzen

[alorenzen]

Because of the need for using an internationalization library like package:intl, most of our clients aren't actually doing a lot of inline text formatting in their templates. So we haven't found this to be an issue in practice for most teams.

I personally don't like the idea of treating some tags differently than others. Then, we're getting back towards being a full HTML compliant parser, which is certainly not a goal.

[cedx]

That's surprising that all of them have glitches.

@matanlurey Unfortunately, the project I'm working on has inline tags in almost every component template (it's more a showcase site than a web application). I have control over the Dart code but not the HTML design (there's a lot of <strong>, <em> and other inline tags).

most of our clients aren't actually doing a lot of inline text formatting in their templates

@alorenzen I had the opposite experience. For example, in my previous job, the translation was done by external translators using the Smartling system, which has a smart editor that is "HTML aware" (it properly extracts and shows text from the HTML tags, and inserts the translations back into those tags).
So, it was very common to embed HTML in source messages (like <strong> tags, but also <a> links: this allows you to translate an entire content, instead of dividing that content into X sections to translate).

If you look at the original AngularTS issue, you'll see that preserveWhitespace = false is also problematic in some translation contexts.

Then, we're getting back towards being a full HTML compliant parser, which is certainly not a goal.

I like the @Goodwine 's proposal of introducing another property (let's say spaceCollapsing, or change the existing preserveWhitespace property) that let customize the whitespace processing:

• preserve == preserveWhitespace = true
• remove == preserveWhitespace = false
• collapse : whitespace is reduced to a single space (thus preserving inline formatting contexts)

This change should be lightweight and easy to implement.

[furrary]

A markup like: Foo \Bar\ Baz (display: "Foo Bar Baz")...
...is processed to: Foo\Bar\Baz (display: "FooBarBaz").

I find this really counter intuitive. IMO, if AngularDart aims to beproductivethis is not the way to go. We still use .html, so we need to respect some of its fundamental.

Because of the need for using an internationalization library like package:intl, most of our clients aren't actually doing a lot of inline text formatting in their templates. So we haven't found this to be an issue in practice for most teams.

It may not be your intention, but these sentences make me feel like the Dart team only care about large companies who write websites in global scale and don't really care about other people out there.

[matanlurey]

@furrary:

but these sentences make me feel like the Dart team only care about large companies who write websites in global scale and don't really care about other people out there.

To be clear, you can _always_ use preserveWhitespace: true.

We need to be careful here, because we really don't want this to end up being _too_ configurable (i.e. too many states or an enum of options), and at the same time, don't want it to be confusing (it clearly is today, I agree with you here). I'll speak to the rest of the team this week and try to get a consensus on how to move forward.

[ninachen]

We have a fair amount of inline text formatting in our templates and do run into this. As a counterpoint, though, the presence of whitespace in HTML can be an active hindrance.

Here's an example with undesirable whitespace: https://codepen.io/anon/pen/vdKWYa

Because we wish to keep the different DOM elements on separate lines for readability, the newline gets rendered as actual whitespace, which is undesirable in this case. So rather than using one of many possible hacky solutions[0] to get rid of the whitespace, it's great that preserveWhitespace: false absolves this problem while making the HTML file smaller to boot.

Personally, I'd much rather have to stick some   in the template than have to jump through hoops to get the template _not_ to render the whitespace when it's unwanted. Or if the template has tons of whitespace that it wants to preserve, it can turn the option back on, as @matanlurey suggested.

[0] https://davidwalsh.name/remove-whitespace-inline-block

[Goodwine]

I disagree, the preserveWhitespace:false feature is buggy IMO.

The reason for its existence is letting us write readable HTML but have the compiler take care of things, basically removing all extra spaces.
The problem is that it's removing more than it should, and it's breaking expectations. It must render exactly the same thing regardless of the preserveWhitespace setting (again, IMO).

If for whatever reason this buggy behavior is seen as a feature to others. Then I believe that the behavior for preserveWhitespace:true should change to normalize all template whitespaces so we can get as much binary size reduction as possible but without being buggy.

[matanlurey]

Just so nobody is frustrated, we have consensus we should do _something_.

The primary maintainer of the AST code (@alorenzen) is out for a couple weeks, and I'd rather not make any decisions until he returns. Continue to please add your comments here on this bug, and we'll hopefully circle back soon.

[matanlurey]

We've decided @alorenzen will look at this when he gets back ;)

Andrew, see internal team notes, @ferhatb agrees we can loosen - let's chat when back.

[matanlurey]

General concepts from the last review:

• We'll do this.
• We'll do it as a visitor in angular_ast.
• We'll have a rough list of requirements.

[matanlurey]

Some ideas: https://kangax.github.io/html-minifier/

[matanlurey]

I'm going to own this.

[Goodwine]

I noticed on your previous email you had an example where

<span>
  some text
</span>

transformed to

<span>some text</span>

And you mentioned that all new lines should be removed.
Maybe you also already thought of this but it's worth pointing out an edge case which is having multiple tags together and relying on how browsers handle whitespaces, like:

<span>
  some text
</span>
<span>
  other text
</span>

should become

<span>some text</span> <span>some text</span>

or "more correctly"

<span> some text </span> <span> some text </span>

[matanlurey]

@Goodwine:

Maybe you also already thought of this but it's worth pointing out an edge case which is having multiple tags together and relying on how browsers handle whitespaces, like.

I'd prefer to implement mostly what https://kangax.github.io/html-minifier/ does.

I don't think keeping the newlines as spaces is what most people expect.

[Goodwine]

@matanlurey
There are many settings in that minifier, the default settings collapses \n into a single space.

[matanlurey]

This seems reasonable:

WDUT @Goodwine

[Goodwine]

I think that this won't solve the problem I originally reported, because of i18n.

<span i18n>
  some text
</span>
<span i18n>
  some text too
</span>

will be translated to (we can't guarantee that original spaces will be preserved because they are not meaningful for translators):

<span i18n>alqui scriptor</span>
<span i18n>alqui etiam scriptor</span>

the minifier will then transform this to:

<span i18n>alqui scriptor</span><span i18n>alqui etiam scriptor</span>

rendering alqui scriptoralqui etiam scriptor instead of alqui scriptor alqui etiam scriptor.

[Goodwine]

Hm, actually, I just checked again and the minifier actually does work :D, would your implementation do the same?

[matanlurey]

I think that is the goal. Not trying to re-invent the wheel here :)

[ninachen]

re: collapsing newlines into a single space

I recognize that this is the existing behavior of HTML, but I'll echo @matanlurey in that it's not always expected -- or desired -- that newlines are collapsed into a single space. See my comment above.

This seems to be a case of where it's not possible to meet all the use cases. :)

Additionally, I'm confused by the example shown in the last minify example, as the newline between the spans gets removed in the first pair but is converted to a single space in the second pair. Is that expected behavior?

[cedx]

I had the opportunity to use HTML Minifier on several projects: if all options are implemented, the output should be fine whatever the requirements. See the Conservative collapse option for example.

[matanlurey]

@ninachen:

This seems to be a case of where it's not possible to meet all the use cases. :)

I'm going to be a bit conservative, so at least it won't be (as) confusing.

@cedx:

I had the opportunity to use HTML Minifier on several projects: if all options are implemented, the output should be fine whatever the requirements.

I don't think I will implement all of them (many don't make sense in AngularDart), but if you generally like the output then our take should be almost as good :)

[rainboxx]

Coming here from the Angular issues linked. The solution showed by @Goodwine looks very strange to me. Imagine you have an a tag like this:

<a href="long link">
  here comes link text
</a>

You'll end up having:

<a href="long link">here comes link text </a>

And suddenly you have an extra space within your link.

The following:

<p>
  this is a paragraph with
  <a href="long link">
    a link inside
  </a>
  and some text
</p>

should probably be minified to:

<p>this is a paragraph with <a href="long link">a link inside</a> and some text</p>

But:

<div class="foo">
  <div>some things here</div>
  <div>some other things there</div>
</div>

should probably be minified to:

<div class="foo"><div>some things here</div><div>some other things there</div></div>

You actually don't want to have spaces between tags, since that might result in weirdness when positioning things. React.DOM is producing this:

From:

<div className="foo">
  <div>some things here</div>
  <div>some other things there</div>
</div>

To:

<div class="foo" data-reactid=".0"><div data-reactid=".0.0">some things here</div><div data-reactid=".0.1">some other things there</div></div>

From:

<p>
  this is a paragraph with
  <a href="long link">
    a link inside
  </a>
  and some text
</p>

To:

<p data-reactid=".0"><span data-reactid=".0.0">this is a paragraph with</span><a href="long link" data-reactid=".0.1">a link inside</a><span data-reactid=".0.2">and some text</span></p>

(which also is not desired.)

From:

<p>this is a paragraph with <a href="long link">a link inside</a> and some text</p>

To:

<p data-reactid=".0"><span data-reactid=".0.0">this is a paragraph with </span><a href="long link" data-reactid=".0.1">a link inside</a><span data-reactid=".0.2"> and some text</span></p>

[Goodwine]

You are not getting my point,

for anchor tags:

<a>
  whatever
</a>

it already behaves like this in every browser

<a>whatever </a>

Please look at https://stackblitz.com/edit/angular-uondfj
My point is that preserveWhitespace: false should not change how a browser renders a component regardless of what minification strategy it uses.

[matanlurey]

This has landed in the internal branch, and will be syncing here shortly.