-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Explicitly implement SetParametersAsync #12254
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -78,15 +78,30 @@ protected virtual Task OnInitializedAsync() | |
|
||
/// <summary> | ||
/// Method invoked when the component has received parameters from its parent in | ||
/// the render tree, and the incoming values have been assigned to properties. | ||
/// the render tree, before the incoming values have been assigned to properties. | ||
/// </summary> | ||
protected virtual void OnParametersSetting() | ||
{ | ||
} | ||
|
||
/// <summary> | ||
/// Method invoked when the component has received parameters from its parent in | ||
/// the render tree, before the incoming values have been assigned to properties. | ||
/// </summary> | ||
/// <returns>A <see cref="Task"/> representing any asynchronous operation.</returns> | ||
protected virtual Task OnParametersSettingAsync() => Task.CompletedTask; | ||
|
||
/// <summary> | ||
/// Method invoked when the component has received parameters from its parent in | ||
/// the render tree, after the incoming values have been assigned to properties. | ||
/// </summary> | ||
protected virtual void OnParametersSet() | ||
{ | ||
} | ||
|
||
/// <summary> | ||
/// Method invoked when the component has received parameters from its parent in | ||
/// the render tree, and the incoming values have been assigned to properties. | ||
/// the render tree, after the incoming values have been assigned to properties. | ||
/// </summary> | ||
/// <returns>A <see cref="Task"/> representing any asynchronous operation.</returns> | ||
protected virtual Task OnParametersSetAsync() | ||
|
@@ -175,22 +190,22 @@ void IComponent.Configure(RenderHandle renderHandle) | |
/// Method invoked to apply initial or updated parameters to the component. | ||
/// </summary> | ||
/// <param name="parameters">The parameters to apply.</param> | ||
public virtual Task SetParametersAsync(ParameterCollection parameters) | ||
Task IComponent.SetParametersAsync(ParameterCollection parameters) | ||
{ | ||
parameters.SetParameterProperties(this); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was another issue I discovered here (or maybe it's intentional and I just missed it). The way this is currently implemented, Init runs after parameters are set. This means that you can read parameter values in Init and it works. That seems really surprising and I wonder how often it's misused. I assumed that it's valuable to separate these things so that we can keep the exact timing of init being called an implementation detail. |
||
if (!_initialized) | ||
{ | ||
_initialized = true; | ||
|
||
return RunInitAndSetParametersAsync(); | ||
return RunInitAndSetParametersAsync(parameters); | ||
} | ||
else | ||
{ | ||
return CallOnParametersSetAsync(); | ||
|
||
return SetParametersCoreAsync(parameters); | ||
} | ||
} | ||
|
||
private async Task RunInitAndSetParametersAsync() | ||
private async Task RunInitAndSetParametersAsync(ParameterCollection parameters) | ||
{ | ||
OnInitialized(); | ||
var task = OnInitializedAsync(); | ||
|
@@ -223,9 +238,35 @@ private async Task RunInitAndSetParametersAsync() | |
// Don't call StateHasChanged here. CallOnParametersSetAsync should handle that for us. | ||
} | ||
|
||
await SetParametersCoreAsync(parameters); | ||
} | ||
|
||
private async Task SetParametersCoreAsync(ParameterCollection parameters) | ||
{ | ||
await CallOnParametersSettingAsync(); | ||
parameters.SetParameterProperties(this); | ||
await CallOnParametersSetAsync(); | ||
} | ||
|
||
private Task CallOnParametersSettingAsync() | ||
{ | ||
OnParametersSetting(); | ||
var task = OnParametersSettingAsync(); | ||
// If no async work is to be performed, i.e. the task has already ran to completion | ||
// or was canceled by the time we got to inspect it, avoid going async and re-invoking | ||
// StateHasChanged at the culmination of the async work. | ||
var shouldAwaitTask = task.Status != TaskStatus.RanToCompletion && | ||
task.Status != TaskStatus.Canceled; | ||
|
||
// We always call StateHasChanged here as we want to trigger a rerender after OnParametersSet and | ||
// the synchronous part of OnParametersSetAsync has run. | ||
StateHasChanged(); | ||
|
||
return shouldAwaitTask ? | ||
CallStateHasChangedOnAsyncCompletion(task) : | ||
Task.CompletedTask; | ||
} | ||
|
||
private Task CallOnParametersSetAsync() | ||
{ | ||
OnParametersSet(); | ||
|
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.
Our idea of making this explicit with overrides is that you can't use the
base
keyword within in an explicit interface implementation. So it's not really possible to interact with by overriding or it would require a lot more design.It seemed more natural to give you a before/after lifecycle model. The only use case I can really think of for before is to set default values, but it's otherwise SUPER painful to do.