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

BWA project template: "@" on @render-mode values and .Client project assembly discovery #52084

Closed
1 task done
guardrex opened this issue Nov 15, 2023 · 0 comments
Closed
1 task done
Assignees
Labels
area-blazor Includes: Blazor, Razor Components bug This issue describes a behavior which is not expected - a bug. feature-templates
Milestone

Comments

@guardrex
Copy link
Contributor

guardrex commented Nov 15, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

Creating an app from the BWA project template with global WASM interactivity produces ...

<HeadOutlet @rendermode="@InteractiveWebAssembly" />
...
<Routes @rendermode="@InteractiveWebAssembly" />

Although the at-symbols (@) on the values are fine, it departs from our guidance to not place them on values of attribute directives outside of explicit Razor expressions [@(...)].

I recommend 🔪 them off; but if you plan to keep these, it creates a delta between the framework and docs, so how would you like to proceed? Let the delta stand, or change all of the docs to this format for directive attributes? If the latter, it will need to be pushed back quite a bit (24Q1/Q2) because it's a 🐘 job to change them everywhere and I'm 🏃‍♂️ on many high priority issues for quite a while.

I opened a docs issue just to call out that this is a supported approach if the dev wants to write their markup this way. dotnet/AspNetCore.Docs#31042

Second ❓: WRT the logic for finding the .Client project assembly ...

#if (UseWebAssembly && SampleContent)
    .AddAdditionalAssemblies(typeof(Counter).Assembly);
#elif (UseWebAssembly)
    .AddAdditionalAssemblies(typeof(BlazorWeb_CSharp.Client._Imports).Assembly);
#endif

I don't see why the latter _Imports file-based approach isn't adopted even if sample content is present. After all, even the empty solution (no sample pages) has an _Imports file. As soon as the dev 🔪 out the Counter component because any production app isn't going to keep it, it will 💥. Many new devs and new-to-Blazor devs won't know the second approach for the _Imports file off the top of their head.

Expected Behavior

No response

Steps To Reproduce

No response

Exceptions (if any)

No response

.NET Version

No response

Anything else?

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-blazor Includes: Blazor, Razor Components label Nov 15, 2023
@MackinnonBuck MackinnonBuck added this to the 8.0.x milestone Nov 15, 2023
@MackinnonBuck MackinnonBuck added bug This issue describes a behavior which is not expected - a bug. feature-templates labels Nov 15, 2023
mkArtakMSFT pushed a commit that referenced this issue Nov 28, 2023
# Use consistent code style in Blazor project templates

- Removed usage of top-level statements in client project if "Do not use top-level statements" is selected
- Removed extra "@" in `@render-mode` values
- Always use `typeof(Namespace._Imports).Assembly` instead of `typeof(Counter).Assembly` so the compilation does not break when the sample Counter component is removed, and so the code is more consistent with how it is when no sample content is generated
- Added Account/AccessDenied endpoint to individual auth option to match Identity UI razor pages. This is shown when the user is authenticated but unauthorized by default.

Fixes #52079
Fixes #52084
Fixes #52167

## Customer Impact

In addition to not using top-level statements when the customer requests that we don't, this improves code style consistency within the Blazor project template and with the Blazor docs.

## Regression?

- [ ] Yes
- [x] No

## Risk

- [ ] High
- [ ] Medium
- [x] Low

These are small stylistic changes to the Blazor project templates.

## Verification

- [x] Manual (required)
- [ ] Automated

## Packaging changes reviewed?

- [ ] Yes
- [ ] No
- [x] N/A
@mkArtakMSFT mkArtakMSFT modified the milestones: 8.0.x, 8.0.1 Nov 28, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Feb 7, 2024
halter73 added a commit that referenced this issue Feb 14, 2024
# Use consistent code style in Blazor project templates

- Removed usage of top-level statements in client project if "Do not use top-level statements" is selected
- Removed extra "@" in `@render-mode` values
- Always use `typeof(Namespace._Imports).Assembly` instead of `typeof(Counter).Assembly` so the compilation does not break when the sample Counter component is removed, and so the code is more consistent with how it is when no sample content is generated
- Added Account/AccessDenied endpoint to individual auth option to match Identity UI razor pages. This is shown when the user is authenticated but unauthorized by default.

Fixes #52079
Fixes #52084
Fixes #52167

## Customer Impact

In addition to not using top-level statements when the customer requests that we don't, this improves code style consistency within the Blazor project template and with the Blazor docs.

## Regression?

- [ ] Yes
- [x] No

## Risk

- [ ] High
- [ ] Medium
- [x] Low

These are small stylistic changes to the Blazor project templates.

## Verification

- [x] Manual (required)
- [ ] Automated

## Packaging changes reviewed?

- [ ] Yes
- [ ] No
- [x] N/A
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components bug This issue describes a behavior which is not expected - a bug. feature-templates
Projects
Status: Done
Development

No branches or pull requests

4 participants