-
Notifications
You must be signed in to change notification settings - Fork 25.1k
Additional Blazor script coverage and fix comments #36415
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses issue #36412 by fixing HTML code comments that were causing build system errors and adds documentation for the RequiresAspNetWebAssets MSBuild property across multiple Blazor documentation files.
Key Changes:
- Fixed moniker syntax in HTML comments by removing triple colons that were causing build issues
- Added comprehensive documentation about the
RequiresAspNetWebAssetsproperty for scenarios where the Blazor script is needed but no Razor components exist - Enhanced the Blazor 10.0 release notes with information about script inclusion behavior
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| aspnetcore/release-notes/aspnetcore-10/includes/blazor.md | Added documentation about RequiresAspNetWebAssets MSBuild property in the Blazor script section |
| aspnetcore/blazor/project-structure.md | Fixed moniker syntax in HTML comments by removing triple colons to prevent build system errors |
| aspnetcore/blazor/components/integration.md | Added two NOTE sections documenting RequiresAspNetWebAssets property for Razor Pages and MVC integration scenarios |
Co-authored-by: Copilot <[email protected]>
| :::moniker range=">= aspnetcore-10.0" | ||
| moniker range=">= aspnetcore-10.0" | ||
| The `NotFound` component (`NotFound.razor`) implements a Not Found page to display when content isn't found for a request path. For more information, see xref:blazor/fundamentals/navigation#not-found-responses. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This text is not showing as intended when the version selector is set to .NET 10.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(ugh, I accidently deleted this thinking I had clicked to reply on your quote, sorry).
Original comment ...
It's commented out on purpose. It shouldn't show up in the review article (or the live article for that matter) at this time.
I think the problem is that with the triple-colon syntax inside HTML comments, the build system is flaking out and breaking the article for some versions. I think that this is the same problem that you noted when working on the not-current-release INCLUDES file. See the live 7.0 article at ...
https://learn.microsoft.com/en-us/aspnet/core/blazor/project-structure?view=aspnetcore-7.0
AFAIK, that should be rendering correctly with the versioning tags in place in the article. I think it's the commented-out triple-colon lines that are causing the problem with that broken 7.0 (and earlier) rendering.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"It's commented out on purpose."
@guardrex, maybe we are we talking about the same text? It is the following text I am referring to below. It looks like it was intended to show in 10 but does not show in 10:
"The NotFound component (NotFound.razor) implements a Not Found page to display when content isn't found for a request path. For more information, see xref:blazor/fundamentals/navigation#not-found-responses."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, per Line 66 ... this bit (Line 75 inclusive) shouldn't appear at this time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commented-out text runs from Line 66 to Line 79.
I'm just seeking to remove the triple-colons from this HTML comment block to try and resurface the MIA parts of the article 🤞🍀 ... if the server gods will bless the attempt 🙏😄.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the 7.0 version of the article, which is currently ...
https://learn.microsoft.com/en-us/aspnet/core/blazor/project-structure?view=aspnetcore-7.0
... gets its content back in the review 7.0 article (which I can't see), then these changes work ... it is the triple-colon syntax in HTML comments that breaks article rendering, which I think is what you saw on that not-latest-version INCLUDES file PR from a week or two ago.
wadepickett
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Text that should be showing for >= 10 is not showing up. See my inline comment.
|
Does that make sense, @wadepickett? If not ... no worries ... I have another way to structure this PR that will make complete sense 😄 ... I'll just hack this bit out and keep it in my 10.0 tracking issue. However, what's on the PR should be correct and fix the article's rendering problems ... in theory 😆. |
|
Thanks. You were likely to anyway, but check the published version when it goes live just there are unexpected results. |
|
@wadepickett ... Yep! That LICK'D IT! 🐮👅 ... https://learn.microsoft.com/en-us/aspnet/core/blazor/project-structure?view=aspnetcore-7.0 ... so we can't currently comment out triple-colon monikers. @cmastr ... Please consider asking DocFX engineering to go back to the old approach of stripping out all HTML comments ( |

Fixes #36412
Thanks @duckblaster! 🚀 ... This will fix the code comments and allow the content of the article to appear, including the existing content on
RequiresAspNetWebAssetsthat was added by #35987 back in August. I'm also adding remarks on that to both the Integration and What's New (release notes) articles.Tom, Wade ... I just need one review to get this one in.
cc: @cmastr ... We used to be able to place triple-colon versioning monikers in HTML code comments and have them safely hidden from the build system. Now, the build system seems to choke on them (and ends up hiding article content). I recommend going back to the prior approach because it's a time saver 💰 to have anything in HTML code comments hidden from the build system, including (especially) triple-colon monikers. Under this new logic, where the build system is processing them somehow, we need to modify all of the lines to remove the triple-colons (and add them back later when we surface the commented-out content). There can be quite a few of these lines in some cases, so it's a 💸 situation under the new paradigm.
Internal previews