Angular parser should not treat table data cells like block elements.

[jdpearce]

Prettier 1.15.2
Playground link

--parser angular

Input:

<table>
  <tr>
    <td class="somethingReallyReallyLong" data-test="somethingElse">{{ someValue }}</td>
  </tr>
</table>

Output:

<table>
  <tr>
    <td class="somethingReallyReallyLong" data-test="somethingElse">
      {{ someValue }}
    </td>
  </tr>
</table>

Expected behavior:
Something more like this :

<table>
  <tr>
    <td class="somethingReallyReallyLong" data-test="somethingElse">{{ 
      someValue
    }}</td>
  </tr>
</table>

The td tag should be treated the same way as span tags because white space is sensitive within a td.

[vjeux]

cc @ikatyang

[ikatyang]

I tried it on Chrome and it seems the whitespaces are insensitive there, could you share more info? or did you change their display?

<table>
  <tr>
    <td class="somethingReallyReallyLong" data-test="somethingElse">111</td>
    <td class="somethingReallyReallyLong" data-test="somethingElse">222</td>
  </tr>
</table>
<table>
  <tr>
    <td class="somethingReallyReallyLong" data-test="somethingElse">
      333
    </td>
    <td class="somethingReallyReallyLong" data-test="somethingElse">
      444
    </td>
  </tr>
</table>

[jdpearce]

See this Stackblitz sample. You're absolutely right, the whitespace isn't normally sensitive when rendered by the browser, but it is within the angular template parser - this results in extra spaces around the content.

I've had to add some CSS to make it apparent (and I suspect this may be what made it apparent to my colleague who found the issue), but when our tests check for the contents of the td and prettier formatting has resulted in the template parser adding extra spaces, this is also a problem.

Perhaps this is something that should be brought up with the Angular team instead? Still, if this is supposed to be working for Angular templates, perhaps it shouldn't break the way it currently does?

[michaeljota]

@jdpearce I just saw your stackblitz. If I remove white-space: pre; from the CSS the text is showed as expected. Can this be the issue?

[michaeljota]

Stackblitz

It's interesting at least the fact that it actually Angular display it differently as the Browser would, but I assume is an Angular issue (or maybe a feature).

Am I missing something?

Update: It's not a bug, it's a feature ©

[jdpearce]

Adding that bit of CSS was deliberate just to show that there's extra white space there. This is a problem when you're doing something like comparing document.querySelector('#cellOne').textContent to the value you expect it to be.

Arguably, this may be the wrong thing to do, but examples of these kind of tests are all over the angular docs.

I suspect something like document.querySelector('#cellOne').innerText may be a better idea, but I guess no one ever expected the templates to be reformatted this way?

[vjeux]

Thanks for following up. So the issue in this case is that node.textContent is whitespace sensitive and therefore when the formatting changes, the value changes as well.

This is not a specific issue with “td” and a different issue than the css whitespace property.

For css whitespace, we have mitigations in place where we respect whitespace for tags and have a comment to disable it on a per-callsite if needed:

The issue with this particular issue is that it’s unclear what nodes are going to be affected. We added a --html-whitespace-sensitivity strict which is a bit nuclear option to avoid doing any whitespace changes but ends up making the code look worse (in my opinion).

You can read more about the tradeoffs on the blog post: https://prettier.io/blog/2018/11/07/1.15.0.html

This is a very difficult trade-off to make. We want to make prettier a drop-in that will not break anything. Unfortunately this isn’t as easy in html (and why we just released it).

My instinct is that it’s unlikely going to break much in production but has the opportunity to break tests. Broken tests are annoying but if you convert your codebase at once, you can fix your tests at once and then you don’t have to think about it.

[thorn0]

Seems to be a duplicate of #5463: inserting white spaces around an interpolation breaks tests.

[thorn0]

On the side note, if some practice is all over the angular docs, it means nothing. A great example and a blog post about it.

[jdpearce]

@thorn0 Absolutely agreed on your side note. My pointing that out was what not to suggest that it's good practice, simply that if it's perceived to be "how things are done" then a lot of people will end up coming across this issue.

[thorn0]

Duplicate of #5463