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

New @formname directive attribute emits code in the wrong place #9323

Closed
SteveSandersonMS opened this issue Sep 26, 2023 · 4 comments · Fixed by #9324
Closed

New @formname directive attribute emits code in the wrong place #9323

SteveSandersonMS opened this issue Sep 26, 2023 · 4 comments · Fixed by #9324
Assignees
Labels
area-compiler Umbrella for all compiler issues bug Something isn't working
Milestone

Comments

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Sep 26, 2023

We discovered an issue where @formname emits the AddNamedEvent call in an incorrect place, leading to forms being broken in current RTM builds. It's likely this is due to the original spec for @formname not giving details about this.

Repro

<form @formname="myform" class="nice">
    <p>@DateTime.Now</p>
</form>

Expected compiler output (simplified):

__builder.OpenElement(5, "form");
string __formName = RuntimeHelpers.TypeCheck<string>("myform");
__builder.AddAttribute(6, "class", "nice");
__builder.AddNamedEvent("onsubmit", __formName);
__builder.OpenElement(7, "p");
__builder.AddContent(8, DateTime.Now);
__builder.CloseElement();
__builder.CloseElement();

Actual compiler output (simplified):

__builder.OpenElement(5, "form");
string __formName = RuntimeHelpers.TypeCheck<string>("myform");
__builder.AddAttribute(6, "class", "nice");
__builder.OpenElement(7, "p");
__builder.AddContent(8, DateTime.Now);
__builder.CloseElement();
__builder.AddNamedEvent("onsubmit", __formName);
__builder.CloseElement();

That is, the AddNamedEvent call must appear after the element's attributes, but before its children. Or you could think of it that named events are a special form of attribute and must be emitted at the end of the attribute list.

It's essential not to emit them after child content, because the runtime is building a flat list of rendertree frames (imagine it's writing to successive indices in a Span<RenderTreeFrame>), and once it's started appending children, it can't go back and insert further attributes/namedevents for the parent (there's no more space).

This probably also explains many or all of the test failures in dotnet/aspnetcore#50734 that are currently blocking the dotnet/aspnetcore repo from accepting SDK updates,

@danmoseley
Copy link
Member

@mkArtakMSFT this came up in discussion today -- in what circumstances will form posts not hit this. Sounds like there's not a workaround per-se but perhaps there's value in identifying when forms will work, if you're willing to modify the content. Is there anything useful we can share about that, eg., and maybe edit the RC2 blog post to add that info?

Of course 4 weeks from now this won't matter any more.

cc @danroth27 @DamianEdwards

@mkArtakMSFT
Copy link
Member

mkArtakMSFT commented Oct 11, 2023

@mkArtakMSFT this came up in discussion today -- in what circumstances will form posts not hit this. Sounds like there's not a workaround per-se but perhaps there's value in identifying when forms will work, if you're willing to modify the content. Is there anything useful we can share about that, eg., and maybe edit the RC2 blog post to add that info?

Of course 4 weeks from now this won't matter any more.

cc @danroth27 @DamianEdwards

After some further discussion with @SteveSandersonMS and @danroth27 we came up with a workaround and are updating the blog post to recommend it for time-being.

For customers who will stumble upon this issue using EditForm instead of form will avoid the issue.

@NitroDH
Copy link

NitroDH commented Oct 11, 2023

@mkArtakMSFT Possible this is causing this issue though dotnet/aspnetcore#51302 (comment) ?

It is causing this issue and I just noticed the ticket that Steve opened up about it too. Can see it's going to be fixed in GA. Thanks.

@333fred
Copy link
Member

333fred commented Oct 12, 2023

Thanks all for your patience while we got a workaround for this issue ready to go. We've published a toolset razor compiler package that can be installed to fix the build from the command line. Add this reference to the csproj of your project:

<PackageReference Include="Microsoft.Net.Compilers.Razor.Toolset" Version="7.0.0-preview.23512.5">
  <PrivateAssets>all</PrivateAssets>
  <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
</PackageReference>

Make sure to remove this package after you upgrade to the next version of .NET 8: it's only needed for RC2 due to our fixes for this issue not quite making the final build.

@dotnet dotnet locked as resolved and limited conversation to collaborators Nov 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-compiler Umbrella for all compiler issues bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants