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

Support closed void tags with content in code blocks #8553

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

jjonescz
Copy link
Contributor

@jjonescz jjonescz commented Mar 31, 2023

Fixes #8460.
Fixes #7258.

Void tags (e.g., <col>, <link>) in code blocks could not have a content and a closing tag, so everything after them was considered to be code. But if the tag is actually a component, I think that behavior doesn't make sense (see the associated issues).

@jjonescz jjonescz added the area-compiler Umbrella for all compiler issues label Mar 31, 2023
@jjonescz jjonescz marked this pull request as ready for review March 31, 2023 15:34
@jjonescz jjonescz requested a review from a team as a code owner March 31, 2023 15:34
chsienki
chsienki previously approved these changes Mar 31, 2023
@jjonescz jjonescz marked this pull request as draft April 3, 2023 12:34
@jjonescz jjonescz dismissed chsienki’s stale review April 4, 2023 13:00

Reworked the fix

@jjonescz jjonescz marked this pull request as ready for review April 4, 2023 13:20
@333fred
Copy link
Member

333fred commented Apr 4, 2023

I don't think I have time to walk through these changes before I'm out for the rest of the week. @jjonescz, when I get back I could use a walkthrough of how we support this outside of code blocks in the parser today.

@jjonescz
Copy link
Contributor Author

jjonescz commented Apr 5, 2023

how we support this outside of code blocks in the parser today

This problem really only appears inside code blocks. The parser consumes code, then encounters <, starts consuming markup, and after <input> it doesn't know whether it should continue parsing code (if <input> is void self-closed tag) or markup (if it's a component that will be closed later). Outside code blocks, it always parses markup by default except when explicitly told not to (by @ transition)

@jaredpar jaredpar added this to the 17.8 Planning milestone Jul 12, 2023
@chsienki chsienki modified the milestones: 17.8 Planning, 17.9 Planning Oct 11, 2023
@@ -2296,5 +2344,7 @@ private class TagTracker
public SyntaxList<RazorSyntaxNode> PreviousNodes { get; }

public bool IsWellFormed { get; }

public ParserContextState? StateBeforeVoidTag { get; init; }
Copy link
Member

@333fred 333fred Mar 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is likely not tracking enough info; what if there are multiple? Please add some tests with multiple tags, some of which should be treated as void tags, some of which should not, all within a markup block inside a code block.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks. I'm not yet sure if this will be difficult to fix, need to investigate. But alternatively we could give up this PR but at least emit some better error/warning when trying to use a component named like void tag.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better backtracking support in general will be useful trying to fix some of the nested } error recovery scenarios, so I think it would be useful to invest in fixing, even if more complex.

Now, whether it's good language design to allow components to shadow void tags... that's an entirely different question 😆. I would have forbidden it and required an escape syntax of some kind, like C# does for identifiers named after keywords.

@jjonescz jjonescz marked this pull request as draft April 3, 2024 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-compiler Umbrella for all compiler issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Blazor WASM] Component named "Col" breaks razor syntax Error when using components in if blazor server side
4 participants