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 lifecycle events for further rendering #28958

Closed
Flachdachs opened this issue Apr 12, 2023 · 5 comments
Closed

Blazor lifecycle events for further rendering #28958

Flachdachs opened this issue Apr 12, 2023 · 5 comments
Assignees
Labels
Blazor Source - Docs.ms Docs Customer feedback via GitHub Issue

Comments

@Flachdachs
Copy link

For me it looks like the order of lifecycle events aren't described correctly. Maybe things have changed since the page was written...

However, the first image shows a gray box labeled "- First render only -". AFAIS it's correct for the first render, but when a parent component changes the value of a child component's parameter without re-creating it, there is also a call to SetParametersAsync before OnParametersSet{Async}. So, SetParametersAsync is not first-render-only. We need a new image for the re-rendering procedure, or the image needs to consider both cases. Also the text part needs to be adjusted, around the images and further down.

I also found out that OnInitialized doesn't await that SetParametersAsync completes when you start awaitable Tasks in SetParametersAsync. (It's maybe not the right place to do such things, but then why is it async?)

Additionally the flow images as such are a bit weird. How can a process step (rectangular box) have two exits? It is not a decision (diamond box). If I see it right there are actually both flows combined in one picture, the synchronous exiting downward and the asynchronous exiting to the right. Wouldn't it make sense to use different colors, separate both handlers and show the async below the sync, or at least add a text label that the downwards exit is for sync and the right one for async?


Document Details

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

@dotnet-bot dotnet-bot added Blazor Source - Docs.ms Docs Customer feedback via GitHub Issue labels Apr 12, 2023
@guardrex
Copy link
Collaborator

Hello @Flachdachs ... These images and the descriptions are an over-simplification by design. Over the years, we've had several issues opened along the lines of what you've stated and calling out other perceived problems, but the product unit has said that these images and the text are what they want to say about the generalized processing of the lifecycle methods. Steve Sanderson said that the framework processing is rather complex and would be difficult to capture into a handful of diagrams and a few paragraphs of text. The reference source and the API docs are the source of truth on it.

As the first line states (emphasis added) ...

The following simplified diagrams illustrate Razor component lifecycle event processing.

Everything here was carefully reviewed and signed off by engineering. I'm going to close this on that basis. There's no point in asking them yet again, as we went through several rounds of talks about what these diagrams show and what the text states when devs opened issues in the past asking. In the end, they are satisfied with this coverage and said that they didn't want to change it.

Thanks anyway tho for the issue. It's great to have feedback, and I encourage you to ask about coverage that's confusing or that seems to have a big 🐞 or a gotcha 😈 case not called out.

@guardrex
Copy link
Collaborator

... also there's another piece that I added after one of the earlier issues that states ...

This article simplifies some aspects of component lifecycle event processing in order to clarify complex framework logic. You may need to access the ComponentBase reference source to integrate custom event processing with Blazor's lifecycle event processing. Code comments in the reference source include additional remarks about lifecycle event processing that don't appear in this article or in the API documentation. Note that Blazor's lifecycle event processing has changed over time and is subject to change without notice each release.

I think that's the best that we can do. Like I said, the ref source is the source of truth on the processing, and devs may need to look at that if they're working on something complex or not understanding the behavior that they're seeing.

@Flachdachs
Copy link
Author

Ok, that's the decision of your developers. Simplification is useful, but it shouldn't lead to a wrong explanation. That SetParametersAsync isn't first-render-only and is instead called multiple times, is a thing that a developer should know when overriding these events.

@guardrex
Copy link
Collaborator

guardrex commented Apr 13, 2023

I'll ask Artak (PU management) offline if he wants to ping engineering staff again on this. I'm afraid to ask at this point 👦🤛😵🚑. They will get upset with me at some point. They've been pinged multiple times over the years on these images and the text on the basis of exactly this type of feedback, and they've told me that this is what they want to show and that these simplifications are basically correct as simplifications.

UPDATE: Ok ... the message is out. Stand-by ............................

@Flachdachs
Copy link
Author

Flachdachs commented Apr 13, 2023

In the meantime I read the code of ComponentBase, and to see how SetParamatersAsync is called I had to read the internal class ComponentState too. Actually both OnInitialized (only once) and OnParametersSet (each time) are called as part of SetParamtersAsync, not after it. This has consequences when overriding SetParamtersAsync.

    public override async Task SetParametersAsync(ParameterView parameters) {
        // doing stuff before
        await base.SetParametersAsync(parameters);
        // doing stuff after
    }

If you write your own code before calling base.SetParametersAsync everything is fine. But if you want the base behavior and write your code after the base call, things go crazy, because your code is executed after OnParametersSet. If you ever need to do things before OnInitialized and OnParametersSet are called you only can do your stuff before the base call. But the base call overrides your changes to parameter properties, so you only can give the base call a new version of ParameterView with your desired changes applied.

If you omit the base call at all, you also skip the execution of both OnInitialized and OnParametersSet. That's maybe the last thing that you want. So, my own conclusion is to leave SetParametersAsync alone and do stuff in OnParametersSet whenever I feel the need for it in the future. Right now I don't have a problem, I just want to understand who things are working.

And, thanks for the patience with me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blazor Source - Docs.ms Docs Customer feedback via GitHub Issue
Projects
Archived in project
Development

No branches or pull requests

3 participants