Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🧪 [Experiment] DependencyPropertyGenerator #624

Open
wants to merge 200 commits into
base: main
Choose a base branch
from
Open

🧪 [Experiment] DependencyPropertyGenerator #624

wants to merge 200 commits into from
+23,783 −4

Conversation

Sergio0694
Copy link
Member

@Sergio0694 Sergio0694 commented Dec 28, 2024 •
edited
Loading

Closes #621

This PR introduces a new DependencyPropertyGenerator component that utilizes source generators on partial properties to generate DependencyProperty quickly in your projects. This is the final API shape available with this PR:

namespace CommunityToolkit.WinUI;

[AttributeUsage(AttributeTargets.Property, AllowMultiple = false, Inherited = false)]
public sealed class GeneratedDependencyPropertyAttribute : Attribute
{
    public object? DefaultValue { get; init; }

    [DisallowNull]
    public string? DefaultValueCallback { get; init; }

    public bool IsLocalCacheEnabled { get; init; }

    [DisallowNull]
    public Type? PropertyType { get; init; }
}

public sealed class GeneratedDependencyProperty
{
    public const object UnsetValue;
}

This went through some community discussion before the current API design was finalized.

This PR includes:

  • The thin API surface with the attribute
  • A source generator for the attribute
  • A comprehensive set of analyzers
  • A code fixer to easily update all trivially supported dependency projects in a project/solution

Arlodotexe and others added 30 commits December 2, 2024 16:55
@Arlodotexe
Generated DependencyPropertyGenerator component from template
054506c
@Sergio0694
Port files from private repo
4ed6172
@Arlodotexe
Cleanup and merge ported csproj, rename files
794d0c3
@Arlodotexe
Fix default namespace and package name for DependencyPropertyGenerato…
bd3f0c5 
…r component
@Arlodotexe
Disable globalusings in DependencyPropertyGenerator
28b5c83
@Arlodotexe
Move files, fix folder name
8e7694f
@Sergio0694
Bring back embedded resources
54f0445
@Arlodotexe
Fix folder name typo
9aec601
@Arlodotexe
Fix project references and update embedded resource paths in Dependen…
f472ab0 
…cyPropertyGenerator tests
@Sergio0694
Fix wrong paths in test project
a559c58
@Sergio0694
Fix more wrong paths
bf62d4d
@Sergio0694
Fix some warnings in the test project
eb854c9
@Sergio0694
Create .gitattributes
df93079
@Sergio0694
Fix newlines to LF
03f041e
@Sergio0694
Use 'WellKnownTypeNames' in all analyzers
d503756
@Sergio0694
Remove global usings
8846a70
@Sergio0694
Finish 'InvalidPropertyDefaultValueTypeAnalyzer'
51d44bd
@Sergio0694
Add unit tests
6b2d0cb
@Arlodotexe
Consolidate GlobalUsings imports around tooling
6c1ceb7
@Arlodotexe
Fixed namespace errors when running under Uno
d9bec94
@Arlodotexe
Update tooling submodule
291f7fa
@Arlodotexe
Remove LangVersion setting from DependencyPropertyGenerator project
9637c3a
@Arlodotexe
Ran XAML styler
9f0809d
@Arlodotexe
Remove empty samples
8436bde
@Arlodotexe
Remove empty component doc to fix CI error
1babcec
@Arlodotexe
Remove unused test files from DependencyPropertyGenerator project
c2351ac
@Arlodotexe
Update .NET version to 9.0 in Dockerfile, devcontainer.json, build.ym…
471c161 
…l, and global.json
@Arlodotexe
Fixed compilation conditionals
9657827
@Arlodotexe
Temporarily limit WinUI and multitarget options in build matrix for U…
10499ac 
…no/Wasm compatibility
@Arlodotexe
Temporarily disable wasm-linux check, only build DependencyPropertyGe…
da2b2c3 
…nerator component
@Sergio0694
Fix an edge case with XAML enum types
e73eb24
@Sergio0694
Handle more codegen cases with explicit types
ccd997c
@Sergio0694
Fix more code fixer edge case scenarios
a68ed9f
@Sergio0694
Fix more edge cases in code fixer
23acb10
@Sergio0694
Add supplementary warnings on property fields
88662ef
@Sergio0694
Update analyzer tests, add more tests
189bd51
@Sergio0694
Update code fixer unit tests
2f412cc
@Sergio0694
Enable diagnostics on orphaned properties, add tests
544c0e5
@Sergio0694
Emit 'WCTDP0030' even on misnamed fields
ddce264
@Sergio0694
Add new diagnostics for default values
2ca8aa8
@Sergio0694
Improve message formats for some diagnostics
d518919
@Sergio0694
Handle 'UnsetValue' in analyzer and code fixer
ff99c77
@Sergio0694
Fix format message for 'WCTDP0028' diagnostic
901c106
@Sergio0694
Add more unit tests for 'WCTDP0030' diagnostic
515a42c
@Sergio0694
Emit diagnostics even if default value is not constant
d6743ba
@Sergio0694
Handle name colon in forwarded attribute arguments
c3ec9cf
@Sergio0694
Tweak diagnostic error codes
762665e
@Sergio0694
Update the embedded attribute type source file
f43c0e4
Lamparter
Lamparter approved these changes Jan 5, 2025
View reviewed changes
Copy link

@Lamparter Lamparter left a comment

Choose a reason for hiding this comment

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

This is looking amazing ❤️

@Arlodotexe
Copy link
Member

Arlodotexe commented Jan 8, 2025 •
edited
Loading

The completeness and stability of the code here inclines me to suggest including it in the mainline repo for 8.2, rather than housing the component in Labs. Labs is a parallel repo where we do incubation and new stuff until stable. Nearing 200 commits, this new component seems stable and well-tested.

@Sergio0694
Copy link
Member Author

It's well-tested in the sense that I have a ton of unit tests, but it's not really stable. It basically has 0 use in production. I fully expect that stuff might come up to warrant changing the API surface a bit or altering the generated code. Doing that would be a breaking change. I think it'd make sense to let this incubate a bit so that we can be sure stuff doesn't come up 🤔

@michael-hawker
Copy link
Member

Yeah, seems like we should just merge this into Labs and get feedback on this here? That's its purpose.

@Arlodotexe
Copy link
Member

Arlodotexe commented Jan 13, 2025 •
edited
Loading

Understood. If we expect that the API surface might change, then testing production via Labs before including it in 8.x stable makes sense.

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

Copilot Copilot left review comments

@Lamparter Lamparter Lamparter approved these changes

@Arlodotexe Arlodotexe Arlodotexe approved these changes

Assignees

@Sergio0694 Sergio0694

Labels
experiment 🧪 Used to track issues that are experiments (or their linked discussions)
Projects
Toolkit 8.x
Status: 👀 In review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

🧪 [Experiment] DependencyPropertyGenerator
5 participants
@Sergio0694 @Arlodotexe @michael-hawker @Lamparter @Youssef1313