Skip to content

Conversation

@tomerqodo
Copy link

Benchmark PR from qodo-benchmark#84

@greptile-apps
Copy link

greptile-apps bot commented Jan 21, 2026

Greptile Summary

This PR adds a new DisplayName<TValue> component for Blazor that displays field display names by reading from [Display] or [DisplayName] attributes, with fallback to the property name.

  • Added DisplayName<TValue> component that follows the same pattern as ValidationMessage<TValue> using expression-based field binding
  • Created ExpressionMemberAccessor helper class with caching support for extracting member info and display names from expressions, including hot reload support
  • Implemented proper attribute precedence: DisplayAttribute takes priority over DisplayNameAttribute, matching MVC conventions
  • Added comprehensive unit tests and E2E tests covering all scenarios including localization with ResourceType
  • Updated Blazor project templates to use the new component instead of hardcoded label text, improving localization support
  • Issue: _displayNameCache field in ExpressionMemberAccessor is declared but never used, and not cleared in hot reload handler

Confidence Score: 4/5

  • This PR is generally safe to merge with minor fixes needed
  • Score reflects well-implemented feature with comprehensive tests, but has unused cache field that should be removed or utilized for optimal performance
  • Pay attention to src/Components/Web/src/Forms/ExpressionMemberAccessor.cs which has unused cache infrastructure

Important Files Changed

Filename Overview
src/Components/Web/src/Forms/ExpressionMemberAccessor.cs New internal helper class for extracting display names from expressions with caching support. Has unused cache field and missing cache clear in hot reload handler.
src/Components/Web/src/Forms/DisplayName.cs New component that displays field display names from attributes. Implementation follows ValidationMessage pattern correctly.
src/ProjectTemplates/Web.ProjectTemplates/content/BlazorWeb-CSharp/BlazorWebCSharp.1/Components/Account/Pages/Login.razor Updated template to use DisplayName component instead of hardcoded labels, improving localization support. Indentation inconsistency on password field.

Sequence Diagram

sequenceDiagram
    participant User
    participant DisplayName
    participant ExpressionMemberAccessor
    participant MemberInfoCache
    participant MemberInfo
    participant Attributes
    
    User->>DisplayName: Render with For expression
    DisplayName->>DisplayName: SetParametersAsync()
    
    alt Expression changed
        DisplayName->>ExpressionMemberAccessor: GetDisplayName(expression)
        ExpressionMemberAccessor->>MemberInfoCache: Check cache for expression
        
        alt Cache miss
            ExpressionMemberAccessor->>ExpressionMemberAccessor: Parse expression tree
            ExpressionMemberAccessor->>MemberInfoCache: Store MemberInfo
        end
        
        ExpressionMemberAccessor->>MemberInfo: GetCustomAttribute<DisplayAttribute>()
        
        alt DisplayAttribute exists
            MemberInfo-->>ExpressionMemberAccessor: DisplayAttribute.GetName()
            ExpressionMemberAccessor-->>DisplayName: Return display name
        else Check DisplayNameAttribute
            ExpressionMemberAccessor->>MemberInfo: GetCustomAttribute<DisplayNameAttribute>()
            alt DisplayNameAttribute exists
                MemberInfo-->>ExpressionMemberAccessor: DisplayNameAttribute.DisplayName
                ExpressionMemberAccessor-->>DisplayName: Return display name
            else Fallback
                ExpressionMemberAccessor-->>DisplayName: Return member.Name
            end
        end
        
        DisplayName->>DisplayName: Update _displayName
        DisplayName->>DisplayName: BuildRenderTree()
    end
    
    DisplayName-->>User: Render display name text
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

16 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

internal static class ExpressionMemberAccessor
{
private static readonly ConcurrentDictionary<Expression, MemberInfo> _memberInfoCache = new();
private static readonly ConcurrentDictionary<MemberInfo, string> _displayNameCache = new();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: _displayNameCache is declared but never used. Either implement caching in GetDisplayName(MemberInfo) or remove this field.

Suggested change
private static readonly ConcurrentDictionary<MemberInfo, string> _displayNameCache = new();
private static readonly ConcurrentDictionary<Expression, MemberInfo> _memberInfoCache = new();
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/Components/Web/src/Forms/ExpressionMemberAccessor.cs
Line: 16:16

Comment:
**logic:** `_displayNameCache` is declared but never used. Either implement caching in `GetDisplayName(MemberInfo)` or remove this field.

```suggestion
    private static readonly ConcurrentDictionary<Expression, MemberInfo> _memberInfoCache = new();
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +84 to +85
{
_memberInfoCache.Clear();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: ClearCache() only clears _memberInfoCache but not _displayNameCache. If _displayNameCache is meant to be used, it should also be cleared here for hot reload support.

Suggested change
{
_memberInfoCache.Clear();
private static void ClearCache()
{
_memberInfoCache.Clear();
_displayNameCache.Clear();
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/Components/Web/src/Forms/ExpressionMemberAccessor.cs
Line: 84:85

Comment:
**logic:** `ClearCache()` only clears `_memberInfoCache` but not `_displayNameCache`. If `_displayNameCache` is meant to be used, it should also be cleared here for hot reload support.

```suggestion
    private static void ClearCache()
    {
        _memberInfoCache.Clear();
        _displayNameCache.Clear();
    }
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines 33 to +38
<div class="form-floating mb-3">
<InputText type="password" @bind-Value="Input.Password" id="Input.Password" class="form-control" autocomplete="current-password" aria-required="true" placeholder="password" />
<label for="Input.Password" class="form-label">Password</label>
<ValidationMessage For="() => Input.Password" class="text-danger" />
<InputText type="password" @bind-Value="Input.Password" id="Input.Password" class="form-control" autocomplete="current-password" aria-required="true" placeholder="password" />
<label for="Input.Password" class="form-label">
<DisplayName For="() => Input.Password" />
</label>
<ValidationMessage For="() => Input.Password" class="text-danger" />
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Inconsistent indentation - this block should align with the one above at line 26-31.

Suggested change
<div class="form-floating mb-3">
<InputText type="password" @bind-Value="Input.Password" id="Input.Password" class="form-control" autocomplete="current-password" aria-required="true" placeholder="password" />
<label for="Input.Password" class="form-label">Password</label>
<ValidationMessage For="() => Input.Password" class="text-danger" />
<InputText type="password" @bind-Value="Input.Password" id="Input.Password" class="form-control" autocomplete="current-password" aria-required="true" placeholder="password" />
<label for="Input.Password" class="form-label">
<DisplayName For="() => Input.Password" />
</label>
<ValidationMessage For="() => Input.Password" class="text-danger" />
<div class="form-floating mb-3">
<InputText type="password" @bind-Value="Input.Password" id="Input.Password" class="form-control" autocomplete="current-password" aria-required="true" placeholder="password" />
<label for="Input.Password" class="form-label">
<DisplayName For="() => Input.Password" />
</label>
<ValidationMessage For="() => Input.Password" class="text-danger" />
</div>

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ProjectTemplates/Web.ProjectTemplates/content/BlazorWeb-CSharp/BlazorWebCSharp.1/Components/Account/Pages/Login.razor
Line: 33:38

Comment:
**style:** Inconsistent indentation - this block should align with the one above at line 26-31.

```suggestion
                <div class="form-floating mb-3">
                    <InputText type="password" @bind-Value="Input.Password" id="Input.Password" class="form-control" autocomplete="current-password" aria-required="true" placeholder="password" />
                    <label for="Input.Password" class="form-label">
                        <DisplayName For="() => Input.Password" />
                    </label>
                    <ValidationMessage For="() => Input.Password" class="text-danger" />
                </div>
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants