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.

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

[Question] GetValue for abstract AvaloniaProperty<T> #3508

Closed
jp2masa opened this issue Feb 4, 2020 · 2 comments
Closed

[Question] GetValue for abstract AvaloniaProperty<T> #3508

jp2masa opened this issue Feb 4, 2020 · 2 comments

Comments

@jp2masa
Copy link
Contributor

jp2masa commented Feb 4, 2020

I noticed some projects using Avalonia expose property definitions as AvaloniaProperty<T> instead of the concrete type (StyledProperty, DirectProperty,...), so there are some calls to GetValue for those properties which no longer work after #3255 and #3287 (the resolved overload is the one taking AvaloniaProperty, which returns untyped value, i.e. object).

So my question is whether it's incorrect to expose properties as AvaloniaProperty<T>, and if not, if it would make sense to provide a GetValue<T>(AvaloniaProperty<T> property) method in AvaloniaObject in order not to break those usages.

@grokys
Copy link
Member

grokys commented Feb 4, 2020

Yes, you're right - as mentioned in the Breaking Changes section of #3255:

If your readonly property fields are declared as of type AvaloniaProperty then you should change them to StyledProperty<> or DirectProperty<,> fields

The reason for this is: because DirectProperty and StyledProperty use different backing stores, they have completely different code paths. Now we have the XAML compiler, setting and binding properties can be strongly typed. so we can avoid the cast to detect which whether the property is styled or direct at runtime. This is preferable because even though a cast is fast, it does have some performance overhead and we set a lot of properties on application startup.

There is still an extension method overload for GetValue which takes an AvaloniaProperty<T>, so things should continue to work (however you may need to use this.GetValue() instead of GetValue() in order for the extension method to work).

If this is causing a lot of problems we can rethink this decision, but note that there will be other breaking changes related to performance coming for 0.10 so it was my judgement that it's best to break things now.

@jp2masa
Copy link
Contributor Author

jp2masa commented Feb 4, 2020

That's completely fine, I simply wasn't aware of the breaking change (missed it somehow) nor the extension method.

Thanks for explaining, keep up the good work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants