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

Avoid Select when no boxing required #16886

Merged
merged 3 commits into from
Oct 3, 2024

Conversation

YohDeadfall
Copy link
Contributor

What is the current behavior?

Always instantiate an instance of Select observer.

What is the updated/expected behavior with this PR?

Don't create any Select if the binding value type is a reference type.

Checklist

Breaking changes

Shouldn't but if the observable implements IObservable<object> then this interface will be used instead of covariance.

Alternative solutions

If we don't care about observables implementing IObservable<object> differently than IObservable<T> then the code can be rewritten to simply use as cast and use the result. It can be helpful for smart observables handling boxing internally, but that I assume is pretty rare.

@avaloniaui-bot
Copy link

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

@cla-avalonia
Copy link
Collaborator

cla-avalonia commented Sep 2, 2024

  • All contributors have signed the CLA.

@avaloniaui-bot
Copy link

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

Copy link
Member

@MrJul MrJul left a comment

Choose a reason for hiding this comment

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

A simple but nice optimization.
LGTM, thank you!

@avaloniaui-bot
Copy link

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

@MrJul MrJul added this pull request to the merge queue Oct 3, 2024
Merged via the queue into AvaloniaUI:master with commit 492cbe5 Oct 3, 2024
11 checks passed
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