Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts the expression-building logic for property access to correctly handle value-type properties when the instance is null, aligning null-handling semantics for reference and value types. Class diagram for updated BuildPropertyAccess in LambdaExtensionsclassDiagram
class LambdaExtensions {
+ConditionalExpression BuildPropertyAccess(Expression instance, Type instanceType, string propertyName)
}
class Expression
class ConditionalExpression
class Type
LambdaExtensions ..> Expression : uses
LambdaExtensions ..> ConditionalExpression : returns
LambdaExtensions ..> Type : uses
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The conditional expression creation now has duplicated
Expression.Conditionlogic; consider extracting thetestexpression and using a single condition with only theifTrueexpression differing to improve readability. - Double-check whether you need to special-case
Nullable<T>separately from other value types, sinceExpression.Defaulton a nullable value type yieldsnull, which may or may not be what existing consumers expect when the instance is null.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The conditional expression creation now has duplicated `Expression.Condition` logic; consider extracting the `test` expression and using a single condition with only the `ifTrue` expression differing to improve readability.
- Double-check whether you need to special-case `Nullable<T>` separately from other value types, since `Expression.Default` on a nullable value type yields `null`, which may or may not be what existing consumers expect when the instance is null.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7307 +/- ##
===========================================
- Coverage 100.00% 99.99% -0.01%
===========================================
Files 745 745
Lines 32631 32637 +6
Branches 4522 4523 +1
===========================================
+ Hits 32631 32636 +5
- Partials 0 1 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in the Table component where accessing nested properties with value types would fail. The issue occurred when building expression trees for complex property paths that included value types (like integers, enums, or structs).
Key Changes:
- Modified
BuildPropertyAccessmethod to differentiate between value types and reference types when creating null-check expressions - Updated version from 10.1.4-beta03 to 10.1.4-beta04
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/BootstrapBlazor/Extensions/LambdaExtensions.cs | Fixed expression tree generation to use Expression.Default for value types instead of Expression.Constant(null), preventing runtime errors when accessing nested value type properties |
| src/BootstrapBlazor/BootstrapBlazor.csproj | Incremented version number to reflect the bug fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Link issues
fixes #7304 #7293
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Bug Fixes: