AspNetCore.Docs icon indicating copy to clipboard operation
AspNetCore.Docs copied to clipboard

Improve Blazor DI article 'OwningComponetBase' example

Open nkatz01 opened this issue 3 years ago • 2 comments

I think that under heading: Utility base component classes to manage a DI scope, you mean to have Settings class implement the two interfaces underneath it:

 public class Setting
    {
        public string? SettingName { get; set; }
        public string? SettingValue { get; set; }
    }

Document Details

Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

nkatz01 avatar Jul 11 '22 14:07 nkatz01

Thanks for the note @nkatz01 ... I'll get back to you ASAP, as I'm just about to go OOF for a few hours.

image

"I'll be back." - Cyberdyne Systems T-800 Series Model 101 (Arnold Schwarzenegger)      The Terminator ©1984 Skydance Media

guardrex avatar Jul 11 '22 15:07 guardrex

Ok ... I'm baaaaaaaaaaaack! ... from the 💪 gym 🏋️. Arnold would be proud! 😄

I'm looking at the context on this. I didn't write this code, but I did review it and its updates over the past few years. Looking at it right now, I don't really like it very much. It's not utterly clear as an example. I'd rather have something like a service that can be clearly distinguished with and without OwningComponetBase that shows the difference, namely that you get component-scoped behavior with it and you get (e.g., Blazor Server, scoped) circuit-scoped behavior without it. I think that would be a better demo, and I'd like to make it cut-'n-paste, too.

Anyway, let's get back to the matter at hand. This originally came in on ...

https://github.com/dotnet/AspNetCore.Docs/pull/17064

... without the classes. The classes were added later, and I think I see where the gremlins got in here! 😈 ...

https://github.com/dotnet/AspNetCore.Docs/blob/7d2473eaf51a22d7e7948051ae2e20e07b55e418/aspnetcore/blazor/common/samples/3.x/BlazorSample_Server/Pages/dependency-injection/Preferences.razor

There was an effort ... briefly ... to have all of the documentation code examples be in apps that would compile as part of the build. That way, PRs will blow up 💥 if someone ... us, a community cat, a PU member ... changes something that breaks the code of the example. This was meant to control bad code getting into doc examples. In order to pull that off in many cases, we needed to mock up quite a few API members in Razor component examples so that the component would compile without breaking when the docs build engine tried to build the sample apps that were supplying the code. We don't do that any longer ... we don't build the examples for every PR. It was only briefly done and stopped because it resulted in way, WAY too many things breaking across the docs to do it in practice.

These classes have escaped from being hidden in the rendered example for that purpose of successfully building the component INTO THE WILD ... i.e., these classes are now surfacing in the document due to a later update ... and they shouldn't be visible! I think the later update was to try to drop some of the snippet code across doc examples to simplify management of the topic's examples in the snippet sample apps.

So, the fix is for me to just hide those classes.

Anyway ... going back to my earlier analysis ... I still think 🤔 that we need a better example of this whole scenario for this section. Therefore, I'm going to "address" this issue with a PR now to fix the problem that you called out, then I'm going to circle around later to build out a new sweet example of this in my usual cut-'n-paste demo style, where readers will be able to just snip a few bits for a test app and see it work in action.

Leave this issue open. The issue will remain open after my "addresses" PR goes in, and I'll get back to this issue at some point in the future. That future work could take a while tho. We're ramping up .NET 7 docs work now, so I can see this getting "fixed" in a PR late this year. This would make a good 🎁 holiday PR ⛄ or something for 23Q1.

Thanks again, @nkatz01 for calling it out. I'll ping u on the PR shortly.

guardrex avatar Jul 11 '22 17:07 guardrex