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

Made AvaloniaPropertyMetadata immutable after property initialization #15384

Merged
merged 2 commits into from
Apr 17, 2024

Conversation

MrJul
Copy link
Member

@MrJul MrJul commented Apr 15, 2024

What does the pull request do?

This PR makes AvaloniaPropertyMetadata immutable after the AvaloniaProperty using it is initialized.

Rationale:

  • It's dangerous to change the metadata after the property has been initialized (no objects currently using the default value would be aware of the change).
  • It's dangerous to pass the same metadata to two different properties (the second one might get the overridden defaults from the first without realizing it).
  • Having mutable metadata prevents some optimizations, see Improve AvaloniaObject.GetValue performance #15342 (review)

What is the current behavior?

The default property metadata can be modified after a property is initialized, by using the Merge method.

What is the updated/expected behavior with this PR?

An InvalidOperationException is thrown when the metadata is modified.

How was the solution implemented (if it's not obvious)?

A Freeze() method has been added to AvaloniaPropertyMetadata.
It would probably be better to have metadata effectively always immutable and have Merge return a new instance if needed instead of changing the current one, but that ship has sailed for v11 since Merge is public.

Breaking changes

Yes, this is a behavioral breaking change.
In practice, any code that would hit the exception was probably introducing a bug.

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0047313-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0047327-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@maxkatz6 maxkatz6 requested a review from grokys April 17, 2024 02:41
@maxkatz6 maxkatz6 merged commit 1d18586 into AvaloniaUI:master Apr 17, 2024
10 checks passed
@maxkatz6 maxkatz6 added the backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch label Apr 17, 2024
@MrJul MrJul deleted the feature/readonly-property-metadata branch April 17, 2024 13:26
maxkatz6 pushed a commit that referenced this pull request Apr 20, 2024
…#15384)

* Made AvaloniaPropertyMetadata immutable after property initialization

* Removed redundant throw
@maxkatz6 maxkatz6 added backported-11.1.x and removed backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch labels Apr 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants