Blazor supports DisplayName for models#4
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Javier Calvarro Nelson <jacalvar@microsoft.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d720c560e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| private static readonly ConcurrentDictionary<Expression, MemberInfo> _memberInfoCache = new(); | ||
| private static readonly ConcurrentDictionary<MemberInfo, string> _displayNameCache = new(); |
There was a problem hiding this comment.
Keying _memberInfoCache by Expression will grow unbounded in normal Blazor usage: For="() => model.Property" creates a new expression instance on each render, so every render adds a new entry and the cache is never evicted (except hot reload). In long‑lived pages with frequent re-renders this leaks memory and increases lookup costs. Consider caching by MemberInfo/FieldIdentifier, or use a value-based comparer or ConditionalWeakTable so repeated renders reuse entries.
Useful? React with 👍 / 👎.
| namespace Microsoft.AspNetCore.Components.Forms | ||
| { |
There was a problem hiding this comment.
Repo compliance rule #2 in /workspace/aspnetcore-combined-codex/AGENTS.md requires file-scoped namespaces. This new file uses a block-scoped namespace (namespace ... { }), which violates that rule; it should be switched to namespace ...;.
Useful? React with 👍 / 👎.
| [Fact] | ||
| public async Task SupportsLocalizationWithResourceType() | ||
| { | ||
| var model = new TestModel(); | ||
| var rootComponent = new TestHostComponent |
There was a problem hiding this comment.
Repo rule #6 in /workspace/aspnetcore-combined-codex/AGENTS.md requires explicit // Arrange, // Act, and // Assert comments in each test method. This test omits those markers, so it doesn’t meet the required AAA structure.
Useful? React with 👍 / 👎.
| private static async Task<string> RenderAndGetOutput(TestHostComponent rootComponent) | ||
| { |
There was a problem hiding this comment.
Repo rule #8 in /workspace/aspnetcore-combined-codex/AGENTS.md requires Task-returning methods to end with Async. RenderAndGetOutput is async but lacks the suffix, so it violates the naming convention.
Useful? React with 👍 / 👎.
2 participants
Benchmark PR from agentic-review-benchmarks#4