Skip to content

Fix optimized DP setter codegen for 'string' properties - #794

Merged
Sergio0694 merged 1 commit into
mainfrom
user/sergiopedri/fix-string-codegen
Apr 28, 2026
Merged

Fix optimized DP setter codegen for 'string' properties#794
Sergio0694 merged 1 commit into
mainfrom
user/sergiopedri/fix-string-codegen

Conversation

@Sergio0694

Copy link
Copy Markdown
Member

Follow-up to #792.

The optimized codegen for generated dependency property setters introduced in #792 calls XamlBindingHelper.SetProperty* directly, but it turns out that XamlBindingHelper.SetPropertyFromString does not work correctly when the value is null or an empty string. To work around this, the generator now special-cases string properties and emits this code instead of an unconditional helper call:

if (value is null || value.Length == 0)
{
    SetValue(TestProperty, value);
}
else
{
    XamlBindingHelper.SetPropertyFromString(this, TestProperty, value);
}

This is applied to both the local-caching and no-caching setter paths.

The existing unit tests covering string and string? dependency properties (including the parameterized rows in SingleProperty_MultipleTypes_WithNoCaching_DefaultValueIsOptimized and SingleProperty_WithCustomMetadataType_WithNoCaching, plus the explicit SingleProperty_String_WithLocalCache test and others) have been updated to reflect the new generated code, so coverage for both setter paths is preserved.

@Sergio0694
Sergio0694 requested a review from Arlodotexe April 22, 2026 04:20
XamlBindingHelper.SetPropertyFromString does not handle 'null' or empty strings correctly, so for 'string' typed dependency properties, fall back to SetValue in those cases and only call the optimized helper for non-empty strings.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Sergio0694
Sergio0694 force-pushed the user/sergiopedri/fix-string-codegen branch from 5b11acb to 05a6172 Compare April 27, 2026 18:36
@Sergio0694
Sergio0694 enabled auto-merge April 27, 2026 18:37

@Arlodotexe Arlodotexe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Source change looks good, test coverage looks good. Cloned and ran the tests locally in VS, all pass. Approved!

@Sergio0694
Sergio0694 merged commit dad3f50 into main Apr 28, 2026
23 of 24 checks passed
@Arlodotexe
Arlodotexe deleted the user/sergiopedri/fix-string-codegen branch April 28, 2026 03:39
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