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

BlazorSetParametersSourceGenerator interferes with capturing CascadedEditContext #9682

Open
1 task done
ShahryarSaljoughi opened this issue Jan 16, 2025 · 14 comments · May be fixed by #9697
Open
1 task done

BlazorSetParametersSourceGenerator interferes with capturing CascadedEditContext #9682

ShahryarSaljoughi opened this issue Jan 16, 2025 · 14 comments · May be fixed by #9697

Comments

@ShahryarSaljoughi
Copy link
Contributor

ShahryarSaljoughi commented Jan 16, 2025

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

Let's say I have a component which derives from InputBase<string?>. The protected EditContext property of my component (which is declared within the InputBase<T> class) will not be initialized properly if Bit.SourceGenerator.BlazorSetParametersSourceGenerator generates the SetParametersAsync method for my component.


Current Workaround:

I can prevent the source generator from generating SetParametersAsync for my component by implementing the SetParametersAsync myself:

public override Task SetParametersAsync(ParameterView parameters)
{
    return base.SetParametersAsync(parameters);
}

Lines of Code Related to the Issue:

Here is a part of the generated code:

       [global::System.CodeDom.Compiler.GeneratedCode("Bit.SourceGenerators","8.10.0.2139")]
        [global::System.Diagnostics.DebuggerNonUserCode]
        [global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage]
        public override Task SetParametersAsync(ParameterView parameters)
        {
            var parametersDictionary = parameters.ToDictionary() as Dictionary<string, object>;
            foreach (var parameter in parametersDictionary!)
            {
                switch (parameter.Key)
                {
                    case nameof(SelectedVouchers):
                       SelectedVouchers = parameter.Value is null ? default! : (System.Collections.Generic.List<SobhOmid.Shared.Dtos.Vouchering.VoucherCodeDto>)parameter.Value;
                       parametersDictionary.Remove(parameter.Key);
                       break;
                   // ... rest of parameters
                }
            }
            return base.SetParametersAsync(ParameterView.FromDictionary(parametersDictionary as IDictionary<string, object?>));
        }

It first creates a Dictionary out of parameters and then at the end converts back the dictionary back to ParameterView object. During this forth and back conversion the cascaded parameters are lost! The image below confirms this loss:
image
As you can see in the image above, the parametersFromDictionary does not contain the cascaded parameters, though the initial parameters instance contained the cascaded edit context parameter.


Steps To Reproduce

Step 1: define the custom component, as partial class with code behind:

TextInput.razor:

@inherits InputBase<string?>

<div class="wrapper @WrapperCssClass">
    @if(!string.IsNullOrWhiteSpace(Label))
    {
        <label class="form-label lbl @(Required ? "required": "")">@Label</label>
    }
    <input @ref="Element" @bind="CurrentValueAsString" class="form-control @CssClass" placeholder="@Placeholder" disabled="@Disabled"/>
    @if (ShowValidationErrors &&  EditContext is { } && EditContext.IsModified(FieldIdentifier))
    {
        <ValidationMessage For="ValueExpression" />
    }
</div>

TextInput.razor.cs

public partial class TextInput : InputBase<string?>
{
    [Parameter] public bool ShowValidationErrors { get; set; }
    protected override bool TryParseValueFromString(string? value, [MaybeNullWhen(false)] out string result, [NotNullWhen(false)] out string? validationErrorMessage)
    {
        result = value;
        validationErrorMessage = null;
        return true;
    }
}

Step two: Use the component within an EditForm:

<TextInput WrapperCssClass="static-code-inp"
           Label="کد ثابت" Required
           @bind-Value="CodeModel.StaticCode"
           @bind-Value:after="CheckStaticCodeDuplication"
           ShowValidationErrors />

Step three: Note the bug:

  • The EditContext is not initialized (null) and validation errors are not shown.
  • The OnFieldChanged event of EditContext is not fired

Exceptions (if any)

No response

.NET Version

8.0.300

Anything else?

No response

@msynk
Copy link
Member

msynk commented Jan 17, 2025

@ShahryarSaljoughi Thanks for contacting us.
what is that InputBase component? is it the BitInputBase?
To investigate the issue, I need source code (a GitHub repo is best) that reproduces this problem.

@ShahryarSaljoughi
Copy link
Contributor Author

@msynk The InputBase is the one that is defined in Microsoft.AspNetCore.Components.Forms namespace

@ShahryarSaljoughi
Copy link
Contributor Author

@msynk The InputBase is the one that is defined in Microsoft.AspNetCore.Components.Forms namespace

I'll provide a repo reproducing the issue by Monday.

@msynk
Copy link
Member

msynk commented Jan 18, 2025

@ShahryarSaljoughi I think I understand the issue now. I will try modifying our source generator to only work on the Bit.BlazorUI namespace so it won't affect other components.

@msynk
Copy link
Member

msynk commented Jan 18, 2025

The issue is accepted and planned. Resolving it will start ASAP.

@msynk msynk moved this to Todo in bit platform Jan 18, 2025
@msynk msynk moved this to Investigating in Support tracking Jan 18, 2025
@msynk msynk moved this from Investigating to To do in Support tracking Jan 18, 2025
@msynk msynk added this to the 999 - V-Next milestone Jan 18, 2025
@msynk msynk moved this from To do to Doing in Support tracking Jan 18, 2025
@msynk msynk moved this from Todo to In Progress in bit platform Jan 18, 2025
@msynk
Copy link
Member

msynk commented Jan 18, 2025

Resolving this issue has started. It will be announced when this issue is resolved.

@ShahryarSaljoughi
Copy link
Contributor Author

@msynk Isn't the generated source code an optimization best practice? If that's the case, isn't it possible to resolve the issue such that we can still have those optimized auto generated methods for all our components?

If the source generator is to be restricted to Bit.BlazorUI namespace then what's the point of having that in the BoilerTemplate project?

@msynk
Copy link
Member

msynk commented Jan 18, 2025

It's not going to work for components that are not implemented based on our standards, such as those that inherit from other base components.

and we are not using this specific source generator anywhere else.

@ShahryarSaljoughi
Copy link
Contributor Author

ShahryarSaljoughi commented Jan 18, 2025

None of the types outside of the bit NuGet packages are likely to belong to the Bit.BlazorUI namespace, hence, I think nothing would be left for the source generator to work on.
What would happen to components deriving form AppComponentBase?. AppComponentBase is defined in Bit.Boilerplate starting project.

@msynk
Copy link
Member

msynk commented Jan 18, 2025

by anywhere else I meant anywhere other than the Bit.BlazorUI project itself.
so we are not relying on this source generator (the Bit.BlazorUI.SourceGenerator) anywhere other than the BlazorUI project.
the one you're seeing in other places is the Bit.SourceGenerators which is a different project.

@ShahryarSaljoughi
Copy link
Contributor Author

Aha thank you. I see your point clearly, however, the generator which is affecting other components is the Bit.SourceGenerators Version="8.10.0", not the one which is inclusively dedicated to Bit.BlazorUI project. I even didn't know that exists :)

@msynk
Copy link
Member

msynk commented Jan 18, 2025

yes, I'm aware of that problem too. I'm thinking of another solution for that one now.

@ShahryarSaljoughi
Copy link
Contributor Author

Could you delegate this task, to me?
Though I do not already have any solution ready to propose, but I'd like to take the challenge.

@msynk
Copy link
Member

msynk commented Jan 18, 2025

I don't think we can do much about it since the ParameterView API is very limited. so for now the workaround you've already suggested in the issue is the best we can do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Doing
Status: In Progress
Development

Successfully merging a pull request may close this issue.

2 participants