Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parser: Unclosed void tags (like input/image) break generation #637

Closed
bastianwegge opened this issue Mar 23, 2024 · 8 comments
Closed

parser: Unclosed void tags (like input/image) break generation #637

bastianwegge opened this issue Mar 23, 2024 · 8 comments
Labels
enhancement New feature or request NeedsDecision Issue needs some more discussion so a decision can be made parser

Comments

@bastianwegge
Copy link
Contributor

Hello maintainers,

I just happened to come across a series of reuses of HTML Templates inside of templ and one thing jumping at me was that templ expects all tags to be closed, even if they are so called "void" like input or image or else (https://html.spec.whatwg.org/multipage/syntax.html#start-tags).

I didn't find an open issue to this so I hope it's ok to bring this up.

Is the support of unclosed tags i.e. <image src="my-image.png"> something that could make templ more HTML5 alike?

@a-h
Copy link
Owner

a-h commented Mar 24, 2024

Yeah, it's fine to bring it up.

templ is like JSX in that it expects all tags to be closed. This makes it possible to discover malformed tags at compile time.

However, templ knows whether a tag is void or not, and renders the expected HTML output as per https://templ.guide/syntax-and-usage/elements/#tags-must-be-closed

So, <img src="picture.jpg" /> in templ becomes <img src="picture.jpg"> in the rendered HTML.

Without requiring all tags to be closed, the parsing logic would have to look at the name of the element to decide whether it should seek out a closing element. It would have to throw away any </input> or similar closing elements.

Are you proposing a change? What would that look like? To force void HTML elements to be written as void? Or something else?

@bastianwegge
Copy link
Contributor Author

bastianwegge commented Mar 24, 2024

Thanks for posting the docs link!

My initial idea was to move closer to the HTML5 spec to improve general adaptation. As I'm firm with React the closing of tags feels natural to me, but this might differ based on knowledge.

I'd suggest allowing void tags to be non-closing tags, thus not to force the closing slash. Allowance would also lead to a non-breaking behavior when it comes to backwards compatibility.

Can you make an example where </input> would be useful? Because I cannot come up with one.

/e: Maybe it doesn't make sense to introduce this as it adds unnecessary complexity and the templ docs already explain how to handle this. Meaning we'd have to change the LSP and the docs 🤔

@a-h
Copy link
Owner

a-h commented Mar 24, 2024

I think there's something to this. I'm also familiar with React, and I'm also old enough to have used XHTML and thought it was a marvellous idea.

However, I have noticed that people will copy paste HTML into templ, and then be surprised that it doesn't work. Usually, because of <img> or <br> tags.

I checked out (using a program) 1800 projects that use templ, and the main reason why templ generate failed was due to that. I suspect that some people will have given up on templ at that very moment.

So maybe templ should / could go the other way and enforce unclosed <img>, <br> tags. templ fmt would be able to normalise old code without issue.

This would also have the side effect of making the HTML LSP support easier to implement, since it would mean that HTML in templ would be identical to HTML that is output.

However, it would mean moving a bit of logic into the parser.

I do wonder why React hasn't done it though. I'd want to read up on that before I jump into it.

@joerdav joerdav added enhancement New feature or request parser NeedsDecision Issue needs some more discussion so a decision can be made labels Mar 25, 2024
@joerdav joerdav changed the title lsp: Unclosed void tags (like input/image) break generation parser: Unclosed void tags (like input/image) break generation Mar 25, 2024
@a-h
Copy link
Owner

a-h commented May 8, 2024

It turns out that it is fairly straightforward to implement support for void elements. Options on behaviour are:

Option 1 - Migrate everything to self-closing, for consistency

  • <input name="test"> is re-formatted to <input name="test"/> (new behaviour, used to complain about unclosed element)
  • <input name="test"/> is unmodified, so outputs <input name="test"/>
  • <input name="test"></input> is re-formatted to <input name="test"/> (existing behaviour).

Option 2 - Switch to void

  • <input name="test"> is unmodified, so outputs <input name="test"> (new behaviour, used to complain about unclosed elements)
  • <input name="test"/> is modified to <input name="test"> (new behaviour, better reflects what's output in the underlying HTML).
  • <input name="test"></input> is re-formatted to <input name="test"> (new behaviour, used to format to <input name="test"/>).

For both options, non-void elements would still complain about lack of closing tags.

I'm thinking option 1 at the moment.

@a-h
Copy link
Owner

a-h commented May 8, 2024

@joerdav
Copy link
Collaborator

joerdav commented May 8, 2024

Agree on the preference of option 1, seems the most sensible. Am I right in saying this would only work on some element tags? Say I open an div and forget to close it would I get an error or would it reformat it?

a-h added a commit that referenced this issue May 8, 2024
@a-h
Copy link
Owner

a-h commented May 8, 2024

You'd still get an error on <div> tags. It would only apply to the specific set of HTML5 void elements. Easier to explain with a PR. 😁 #732

@bastianwegge
Copy link
Contributor Author

Very good solution! The reformatting really works and seems to be the most easy and straight-forward fix to this. 👍
I even understood most of the PR you wrote 🎉 . We can close this as soon as #732 is released.

@a-h a-h closed this as completed in ad707cb May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request NeedsDecision Issue needs some more discussion so a decision can be made parser
Projects
None yet
Development

No branches or pull requests

3 participants