prettier icon indicating copy to clipboard operation
prettier copied to clipboard

HTML tags should not be automatically closed in template partials

Open xthezealot opened this issue 7 years ago • 15 comments

Remember the good ol' days when we had server-side rendering with template engines, using HTML partials (especially for headers and footers)… They sometimes contained HTML tags that were closed in other files.

Today, Prettier automatically closes all tags in a file, breaking the "partiality" of those partials.

Prettier 1.15.3 Playground link

Input:

<!DOCTYPE html>
<html>
  <head></head>
  <body>

Output:

<!DOCTYPE html>
<html>
  <head></head>
  <body></body>
</html>

Expected behavior:

Let the unclosed HTML tags be unclosed (maybe with an option).

xthezealot avatar Dec 20 '18 12:12 xthezealot

But… I see that Prettier requires pure HTML ; it doesn't support any exotic tags from any server-side template engine, even when they begin with < and end with >, as in EJS.

In this situation, this "partials" issue is only a fraction of the problem.


So if templates other than Angular, Vue and JSX are out of your scope, I think we can close the issue.

Otherwise, maybe Prettier could at least ignore code between some pre-defined tags:

  • <% and %> in EJS
  • {% and %} in Nunjucks
  • {{ and }} in Mustache/Handlebars
  • Etc.

xthezealot avatar Dec 20 '18 16:12 xthezealot

--parser html is supposed to be used for complete html files. Personally, I think the "partial" formatting is out of scope since it looks like you just treat them as pure text, not HTML. Is it a common practice for server-side rendering? If so, can you share a reference?

Regarding those template languages:

  • handlebars: it's tracked in #5340.
  • pure template language (e.g., ejs, nunjucks, etc.): see https://github.com/prettier/prettier/issues/5286#issuecomment-442061386.

ikatyang avatar Dec 21 '18 14:12 ikatyang

Is it a common practice for server-side rendering? If so, can you share a reference?

It is for languages that don't have a layout/extends logic and only support partials imports.
That's the case of EJS (from https://ejs.co, Layouts section):

<%- include('header'); -%>
<h1>Title</h1>
<p>My page</p>
<%- include('footer'); -%>

And yes, this kind of templating is still relevant: https://www.npmtrends.com/ejs-vs-handlebars

xthezealot avatar Dec 21 '18 18:12 xthezealot

This flaw isn't just limited to partial HTML files, it affects all HTML documents run through Prettier. I came here to file an issue because Prettier was breaking my valid HTML documents (the whole file) because it was trying to add in closing tags that didn't need to be closed.

Here's a full valid HTML document as input into Prettier. If it doesn't look like valid HTML to you, you can run it through the (slightly outdated but good enough) Nu HTML validator, or read the HTML spec to see that it does conform to proper HTML syntax and document structure. If you load it up in a web browser it should parse this document and put the last line of text inside the <body> tag.

<!DOCTYPE html>
<html lang=en>
<head>
<title>Example of Prettier breaking valid HTML</title>
In a valid HTML document, I'm part of the &lt;body>, not the &lt;head>

And here's the broken output Prettier returns:

<!DOCTYPE html>
<html lang="en">
  <head>
    <title>Example of Prettier breaking valid HTML</title>
    In a valid HTML document, I'm part of the &lt;body>, not the &lt;head>
  </head>
</html>

The problem is that it's trying too hard to close that <head> tag, but in HTML both the opening <head> tag and closing </head> tag can be omitted in certain situations. For Prettier to add them in where they weren't originally seems like a bad design (I wasn't expecting that it would try this) but to add it in in a way that breaks the document and makes it invalid is very bad.

I only just started using Prettier this week, but bugs like this make it a no-go for me—I don't think I can use a tool that breaks valid HTML input and turns it into broken documents :/ oof

If Prettier never tried to add HTML tags into documents, this problem with </head> and an entire category of bugs will be fixed & avoided. Prettier really doesn't need to know or care about the validity of the HTML document to beautify it, but if it is going to try to rewrite my code based on HTML syntax and the content model of an HTML document I would only ever expect it to omit safely omissible code, never to add code.

Please let me know if you have trouble reproducing this bug in Prettier, or if the link to the Prettier playground doesn't work for you.

tomhodgins avatar Jan 23 '19 20:01 tomhodgins

@tomhodgins Would you mind opening a new issue? That sounds like an outright bug to me, while the OPs issue is a bit more unclear.

lydell avatar Jan 23 '19 20:01 lydell

The example code in the first post is literally the same issue → it takes a complete, valid HTML document as input and adds in tags (</body> and </html>) where they weren't present originally, never needed to be present. In that case the output is still valid HTML, but the original commenter still thought it was enough of an issue to file a bug report. Now I've got the same thing happening a month later and it's actually making all of the HTML on my website invalid.

How about Prettier just never tries to do this sort of thing in the first place. There can't be any bugs relating to it doing it wrongly if it never tries to inject HTML tags in the first place. I don't mind a formatter being opinionated, but it shouldn't make assumptions.

tomhodgins avatar Jan 23 '19 21:01 tomhodgins

The example in the OP is a valid transformation, but in your example Prettier changed the meaning of the code. Those are different things to me (although similar). Let's fix the more obvious case while thinking about the other one.

lydell avatar Jan 23 '19 21:01 lydell

For ejs, this doesn't seem to be a problem. After ejs encounters the start tag for <html> and <body> for the first time, any subsequent encounters with html and body tags (both start and end tags) are ignored.After the end rendering, </html> and </body> are automatically added.

fidelyiu avatar Oct 11 '21 02:10 fidelyiu

Just wanted to chime in to say this is a massive problem with the way Hugo handles its partials.

kolaente avatar Jan 29 '22 17:01 kolaente

Adding to @kolaente, this is stopping me from fixing #59 on prettier-plugin-go-template which is using the embed API of prettier.

Would be neat if we'd have an option to disable this! If someone from the Prettier team can confirm this, I'd be glad to open a PR.

NiklasPor avatar Mar 30 '22 18:03 NiklasPor

so, where are the solutions for this? prettier is automatically closed tags, I don't need that because I use partials header and footer for template engine ejs

mizzcode avatar Nov 08 '23 15:11 mizzcode

I'm also running into this issue while trying to make an <ol> element that is opened on one line and closed on another.

Before:

html`
  ${numberPosts ? html`<ol class="mb4">` : ""}
  ${posts}
  ${numberPosts ? html`</ol>` : ""}
`

After:

html`
  ${numberPosts ? html`<ol class="mb4"></ol>` : ""}
  ${posts}
  ${numberPosts ? html`</ol>` : ""}
`

Prettier closes the <ol> tag, leading to invalid HTML. I wish I could disable this option!

mbforbes avatar Nov 13 '23 02:11 mbforbes

is there a plan to implement the ability to instruct prettier not to inject closing tags? perhaps by introducing the idea of an html partial as a different flavor of html which may not have the closing elements by design?

this could be used to tell prettier “files in this directory are partial html assets and require a different processing lens, while other html assets outside this directory may be treated as current logic dictates”

which i’d think would make it easy for users who have mixed needs within a project scope…

as i understand the philosophy of prettier, it’s supposed to be able to do the right thing …. this is a context where that’s hard to do without the ability to distinguish html partial assets from full html documents.

i suspect that adding “treat html documents differently” logic would be fairly complex to implement with confidence that it won’t break an existing setup and that instead adding “treat html partial documents differently” would be less likely to introduce breaking behaviors to existing setups…

but i’m not certain my logic is sound :) regardless, as-is, I don’t think I can use prettier for hugo projects, which is a bit of a shame, and I’d love to be proven wrong :)

wolfspyre avatar Dec 29 '23 22:12 wolfspyre

It is painful to keep struggling w/ this auto closing issue when having separate header.html and footer.html. Seems <!-- prettier-ignore --> doesn't work for this case.

I think an easier workaround is to allow using <!-- prettier-ignore --> in header.html to prevent auto complete of </html>.

Currently this solution doesn't work. Not sure why.

zairwolf avatar Jan 06 '24 18:01 zairwolf

Also have same trouble, tmp solution: my html-partials then having non-closed tags has name from underscore sign on start, example: _header.html _footer.html

So, I just created global .prettierignore file:

_*.html 

and set path to this .prettierignore file in vsode setting.json:

"[html, scss, css, less]": {
    "editor.defaultFormatter": "esbenp.prettier-vscode"
},
"files.associations": {
    "*.html": "html"
},
"prettier.ignorePath": "/Users/{{Username}}/.prettierignore

nesklada avatar Feb 06 '24 16:02 nesklada