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

Make component binding case-sensitive #9860

Closed
4 tasks done
SteveSandersonMS opened this issue Apr 30, 2019 · 6 comments
Closed
4 tasks done

Make component binding case-sensitive #9860

SteveSandersonMS opened this issue Apr 30, 2019 · 6 comments
Assignees
Labels
area-blazor Includes: Blazor, Razor Components Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one

Comments

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Apr 30, 2019

Summary

We've going to change component binding to be case-sensitive and add warnings for cases where you made an obvious mistake. We've treated all of these issues as conventions or recommendations thus far, but having components fail to bind usually results is really vexing compiler errors from C#

  • Make compiler bind components case-sensitively
  • Make completion of components and attributes case-sensitive (not the case today when an attribute has the same name a known HTML thing)
  • Add a diagnostic for components attempting to use lower case
  • Add a diagnostic for elements that start with a capital letter but don't match a component

The problem

Suppose that you rename a component without renaming all of the usages (or suppose you change the namespace). If you were using event handlers with that component:

<MyButton OnClick="Clicked" Message="So enticing, you know you want to click!"/>
@code {
    void Clicked() {
        ...
    }
}

You now get a compile error that says something like Cannot convert method group Clicked to type System.Object.. This is because method-group-to-delegate-conversion doesn't support object, but it's fairly common with components.

You now miss the forest for the trees. You're distracted from the root cause, which is that the MyButton tag didn't resolve to a component.

Solution

We've had on the roadmap for a while to make binding case-sensitive. So this is not really a surprise.

The additional goal is to detect and warn in the case where a developer tries to reference a component, but no such component is found. Currently the behavior is to treat it as an HTML element, which is potentially very confusing.

To fix this, we want to turn the PascalCasing of component names into a stronger, built-in convention:

  1. If you try to compile a component whose name starts with a lowercase character in a-z, fail the compilation with an error (Component names cannot start with lowercase characters).

    • I don't think we can do this as a suppressible warning, because where would you put the suppression?
    • I think this rule is correct internationally. We're not stopping you from starting a component name with some non-Roman character, since that's not ambiguous.
  2. Each time something in your .razor source resolves as an element (not a component), check that its name starts with a lowercase character in a-z. If we find an element whose first char isn't in a-z case sensitively, emit a warning (Found markup element with unexpected name '{0}'. If this is intended to be a component, add a @using directive for its namespace.).

    • Again, I think this is right internationally, because all standard HTML element names do start with a character in a-z (case-insensitively), and will do probably forever.
  3. Allow the warning from (2) to be suppressed. See Blazor compiler reports misleading error #9786 (comment)

@SteveSandersonMS SteveSandersonMS added enhancement This issue represents an ask for new feature or an enhancement to an existing one area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates area-blazor Includes: Blazor, Razor Components labels Apr 30, 2019
@SteveSandersonMS SteveSandersonMS added this to the 3.0.0-preview7 milestone Apr 30, 2019
@mkArtakMSFT mkArtakMSFT removed area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels May 9, 2019
@chucker
Copy link

chucker commented May 26, 2019

To fix this, we want to turn the PascalCasing of component names into a stronger, built-in convention:

Does this also imply that a <Button> component won't collide with HTML's <button>?

@rynowak rynowak changed the title Razor compiler warning when using non-lowercase element name in component Make component binding case-sensitive May 30, 2019
@SteveSandersonMS
Copy link
Member Author

Does this also imply that a component won't collide with HTML's ?

Yes

@Mike-E-angelo
Copy link

I'm a traditional Xaml developer and can say that most Xaml developers are turned off by HTML syntax. Pascal-case brings a lot of authority and elegance to symbols, IMO. Plus it's consistent with C# and VB.NET, of course.

That said, Blazor's syntax has a lot going for it. I think the more Pascal-case you bring to the table, the more adoption -- and perhaps, less complaining 😆 -- you're going to see from Xaml developers as there won't be so much Frankenstein inconsistency between .NET and HTML symbols.

Forcing Pascal-case for .NET symbols is a major win, IMO. It will make it easier to know what is HTML and what is .NET, especially for the newbs such as myself who are onboarding and are learning the terrain.

FWIW, it would be ideal to take it a step further and allow (read: not require) the directives themselves to be Pascal-cased as well, e.g. @Bind="...".

In any case, thank you for all your efforts here. Very impressed with and excited for Blazor. 👍

@rynowak
Copy link
Member

rynowak commented Jun 25, 2019

@ajaybhargavb - one of the first considerations here is to make sure the the WTE editor is updated to align with this behavior. So, we need to do gap analysis - are there places where WTE would complete or colorize things differently from what we expect.

@rynowak
Copy link
Member

rynowak commented Jul 17, 2019

<3 @ajaybhargavb - make sure you blast out an email update when we have a build of VS 👍

@SteveSandersonMS
Copy link
Member Author

@ajaybhargavb Really glad to see this go in! This is a huge improvement 😄

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one
Projects
None yet
Development

No branches or pull requests

6 participants