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

Blazor Component called "Script" produces intellisense errors and fails to compile correctly #5267

Closed
stefanloerwald opened this issue Jul 21, 2020 · 14 comments
Milestone

Comments

@stefanloerwald
Copy link

Describe the bug

Using a component named Script.razor produces an intellisense error:

In a component, write:

<Script />
  • Intellisense detects it as the script tag (which it isnt):
Element 'Script' is a container element and must have separate closing tag.

To Reproduce

  • Create a component with file name Script.razor. Leave content as default (so that we see whether it's used or not)
  • in Index.razor, add the component: <Script />.
  • Notice the warning

This happens with both 3.1 and 5.0-preview.

@stefanloerwald stefanloerwald changed the title Blazor Component called "Script" produces intellisense errors and fails to render Blazor Component called "Script" produces intellisense errors Jul 21, 2020
@stefanloerwald
Copy link
Author

stefanloerwald commented Jul 21, 2020

Similarly, a component named Link behaves badly as well, although differently:

The editor will auto-correct it to lower-case link, which one can correct to Link, but nothing for it will get rendered.

@stefanloerwald
Copy link
Author

Actually, this is all hugely messed up:

@page "/"
<Link />
<Script />

@code {

} 

results in

warn: Microsoft.AspNetCore.Components.Server.Circuits.RemoteRenderer[100]
      Unhandled exception rendering component: InvalidCharacterError: Failed to execute 'createElement' on 'Document': The tag name provided ('') is not a valid name.
System.InvalidOperationException: InvalidCharacterError: Failed to execute 'createElement' on 'Document': The tag name provided ('') is not a valid name.
   at Microsoft.AspNetCore.Components.RenderTree.Renderer.InvokeRenderCompletedCallsAfterUpdateDisplayTask(Task updateDisplayTask, Int32[] updatedComponents)
fail: Microsoft.AspNetCore.Components.Server.Circuits.CircuitHost[111]
      Unhandled exception in circuit 'IMSm4dY1QmoaxFMDOtxDJasfz4D_bH_4-lRVEj-DIlo'.
System.AggregateException: One or more errors occurred. (InvalidCharacterError: Failed to execute 'createElement' on 'Document': The tag name provided ('') is not a valid name.)
 ---> System.InvalidOperationException: InvalidCharacterError: Failed to execute 'createElement' on 'Document': The tag name provided ('') is not a valid name.
   at Microsoft.AspNetCore.Components.RenderTree.Renderer.InvokeRenderCompletedCallsAfterUpdateDisplayTask(Task updateDisplayTask, Int32[] updatedComponents)
   --- End of inner exception stack trace ---

However,

@page "/"

<Link />
<Script />

works.

And lastly:

@page "/"

<Script />
<Link />

only displays "Script"

@stefanloerwald
Copy link
Author

Here's a repository to reproduce all the above failures: https://github.com/stefanloerwald/blazor_broken_components/

@stefanloerwald
Copy link
Author

Looking at the produced code, it seems that something is tripping up the razor compiler to produce the render tree and produces and empty element:

<Script />
@code {
}

produces

            __builder.OpenElement(0, "");
            __builder.OpenComponent<BlazorApp1.Pages.Script>(1);
            __builder.CloseComponent();
            __builder.AddMarkupContent(2, "\r\n\r\n\r\n\r\n");
            __builder.CloseElement();

As opposed to

<Script />

produces

            __builder.OpenComponent<BlazorApp1.Pages.Script>(0);
            __builder.CloseComponent();

<Script /> + <Link /> ends up even differently:

<Script />
<Link />

results in

            __builder.OpenComponent<BlazorApp1.Pages.Script>(0);
            __builder.CloseComponent();
            __builder.AddMarkupContent(1, "\r\n\r\n<Link />\r\n\r\n\r\n\r\n");

<Link /> on its own only has the issue of being auto-corrected to <link>.

@stefanloerwald
Copy link
Author

@mkArtakMSFT I don't think this is exclusively a tooling issue, as the compilation step is flawed too.

@simonziegler
Copy link

I agree with Stefan that it would be great for this to get some attention.

@ghost
Copy link

ghost commented Jul 27, 2020

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@stefanloerwald
Copy link
Author

Excuse me @mkArtakMSFT, but this isn't just something that needs enhancement, this is a pretty serious bug. Is the opinion of the blazor team really that this can wait until .net 6?

@stefanloerwald stefanloerwald changed the title Blazor Component called "Script" produces intellisense errors Blazor Component called "Script" produces intellisense errors and fails to compile correctly Jul 28, 2020
@simonziegler
Copy link

Hi, I agree with Stefan on this one.

@stefanloerwald
Copy link
Author

It seems like the razor compiler incorrectly applies the rules of HTML tags, i.e. disallowing <script /> and only allowing <script></script> falsely to a component named Script. This is probably because HTML allows any mix of lower- and uppercase letters. So blazor probably needs to special-case components that match exactly.

While bad intellisense is one thing, producing invalid render-trees is simply a bug that in my opinion shouldn't wait until some unknown future release, but should be fixed in what we have right now.

@stefanloerwald
Copy link
Author

@mkArtakMSFT, @javiercn May I ask you to acknowledge that this is in fact a bug, not simply something to be "enhanced"? Is the plan to let this bug continue to exist in netstandard2.X/netcoreapp3.1 and only fix this beyond .net 5?

@javiercn
Copy link
Member

@stefanloerwald As you point out, there seems to be a bug here.

That said, I don't think this is high on our priority list fo fix. We definitely wouldn't patch this, since there's likely a workaround, which consists of using the full type name to refer to the component.

I think this specific issue is problematic for several reasons:

  • One is that creating a component that has the same name as a task is making life hard for yourself. While we could go and do all the work to support this, I'm failing to see the added value of having a component whose name tag conflicts with an html tag, specially given that HTML tags are case insensitive and components are not.
  • This might be acceptable when you are reading/writing code from an IDE, which if properly implemented will color things correctly, but it incredibly hard to tell the difference when you are reading code somewhere that doesn't do semantic coloring.

While this is a bug, we don't think that using the same name as an html tag produces a great experience and that the work involved in supporting it doesn't make it a priority compared to other features/updates.

From my point of view, creating a component that overlaps with an HTML tag is similar to creating a type named Int32 in your own namespace. You will be able to use it, but the experience will not be good and its a problem you are creating on yourself and that has a simple solution, which is to pick a name that doesn't clash with an existing identifier.

I understand that this might not be what you expect to hear, and that you might have legitimate reasons to use components that match tag names, but we think that even if we fix the bugs here, the experience will not be good, and that's why we choose to prioritize other work over it.

I hope this at least helps explain our reasoning behind it. I'm not sure if in the future we will resolve the ambiguity between the component/tag, but we have some other ideas to allow for a variant of this that is not ambiguous. That said, we will have to balance it against other future work before we decide to commit to it.

@stefanloerwald
Copy link
Author

Hi @javiercn,

this is indeed disappointing. The whole developer experience makes the user believe that what's used is the blazor component, rather than the HTML tag (which doesn't seem to be used very often in any other way than either <script ...> or <SCRIPT ...>, if you have a look at the data in the http archive. Around 0.003% to be precise). So the vast majority of developers would probably not even expect a script tag to be written any other way than all-lower or all-upper case. Even more so in blazor, as the script tag is disallowed there anyway... So it's a rather pointless argument that there could be ambiguity.

At the very least - if you really don't want to fix this bug anytime soon - it would be a courtesy to developers to warn them if they write a component with a reserved name.

@allisonchou
Copy link
Contributor

This is a dupe of https://github.com/dotnet/aspnetcore/issues/18685, which has been fixed in the new Razor editor (VS 2022). Closing out the issue.

@allisonchou allisonchou transferred this issue from dotnet/aspnetcore Oct 9, 2021
@allisonchou allisonchou added this to the Backlog milestone Oct 9, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Nov 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants