Remove explicit backing field generated Relay Commands when possible#1162
Remove explicit backing field generated Relay Commands when possible#1162Avid29 wants to merge 12 commits intoCommunityToolkit:mainfrom
Conversation
|
Sorry. This PR is horribly named, but GitHub isn't letting me edit for some reason |
|
Some tests are failing. It appears to be because they test that both the field and property is generated, except now a field is no longer always generated. For example: |
…RelayCommand tests NOTE: This conditional compilation is based on roslyn version, not C# version or target platform. This could hypothetically lead to reduced test coverage in the future.
Avid29
changed the title
…RelayCommand tests NOTE: This conditional compilation is based on roslyn version, not C# version or target platform. This could hypothetically lead to reduced test coverage in the future.
Avid29
force-pushed
the
relay-command-sg_field-keywords
branch
from
314633d to
4a2c060
Compare
| AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(typeof(RelayCommandGenerator).Assembly.GetName().Version.ToString())))))) | ||
| .WithOpenBracketToken(Token(TriviaList(Comment($"/// <summary>The backing field for <see cref=\"{commandInfo.PropertyName}\"/>.</summary>")), SyntaxKind.OpenBracketToken, TriviaList()))) | ||
| .AddAttributeLists(forwardedFieldAttributes); | ||
| ImmutableArrayBuilder<MemberDeclarationSyntax> declarations = ImmutableArrayBuilder<MemberDeclarationSyntax>.Rent(); |
There was a problem hiding this comment.
Based on other usage of ImmutableArrayBuilder.Rent, it's meant to be disposed.
https://github.com/search?q=repo%3ACommunityToolkit%2Fdotnet%20ImmutableArrayBuilder&type=code
There was a problem hiding this comment.
Nice catch. I thought that the ToImmutable call at the end of the function disposed the builder, but upon further investigation, I was wrong. I'll add a using declaration, so dispose is called at the end of the scope.
|
I'd love for this to go in. I had looked into what it meant to implement this a few months ago and it was pretty much what you ended up doing. Thanks for actually doing it! I was too wondering what to do about the breaking change to anyone using the field directly. It seems only avoidable if we require folks to opt-in (via an extra parameter to the attribute?) but that would make the optimal scenario more verbose and worse in my opinion. |
|
@huguesv I've added an optional named parameter to the |
2 participants
Closes #1073
When the C# language version supports it, this change replaces the backing field for generated
RelayCommands with an implicit field using the newfieldkeyword.PR Checklist
Other information
Technically, this is a breaking change since someone may have been using the backing fields for whatever reason