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

[X] don't expand types to Extension for x:Static #17276

Merged
merged 1 commit into from
Feb 4, 2025
Merged

Conversation

StephaneDelcroix
Copy link
Contributor

@StephaneDelcroix StephaneDelcroix commented Sep 8, 2023

Description of Change

if a type name exists with an Extension suffix, xaml rules require us to use that. But not for x:Static

Issues Fixed

@StephaneDelcroix StephaneDelcroix requested a review from a team as a code owner September 8, 2023 14:37
@samhouts samhouts added this to the Under Consideration milestone Sep 8, 2023
@Eilon Eilon added the area-xaml XAML, CSS, Triggers, Behaviors label Sep 8, 2023
@samhouts samhouts added the stale Indicates a stale issue/pr and will be closed soon label Sep 19, 2023
@samhouts
Copy link
Member

Do we know if this is a path that is commonly used? Is this going to break a bunch of people?

@StephaneDelcroix
Copy link
Contributor Author

Do we know if this is a path that is commonly used? Is this going to break a bunch of people?

it's only affecting namespace with both MyClass and MyClassExtension defined. I believe this is safe and the modified behaviour is the expected one

@StephaneDelcroix
Copy link
Contributor Author

this is a dupe pr. the dupe is #15903. pick a single one

@StephaneDelcroix StephaneDelcroix changed the title [X] don't expant types to Extension for x:Static [X] don't expand types to Extension for x:Static Sep 21, 2023
@hartez hartez removed their request for review January 10, 2024 20:23
@mattleibow
Copy link
Member

@StephaneDelcroix this has been many coffee cycles since this was created... Do we still want/need this?

@StephaneDelcroix StephaneDelcroix changed the base branch from main to net9.0 July 29, 2024 09:41
@StephaneDelcroix StephaneDelcroix changed the base branch from net9.0 to main July 29, 2024 09:42
@StephaneDelcroix StephaneDelcroix requested a review from a team as a code owner July 29, 2024 09:53
@StephaneDelcroix StephaneDelcroix changed the base branch from main to net9.0 July 29, 2024 09:53
@PureWeen PureWeen removed this from the Under Consideration milestone Aug 2, 2024
@StephaneDelcroix
Copy link
Contributor Author

@StephaneDelcroix this has been many coffee cycles since this was created... Do we still want/need this?

y, but in net9

@StephaneDelcroix StephaneDelcroix removed the stale Indicates a stale issue/pr and will be closed soon label Sep 12, 2024
@rmarinho rmarinho changed the base branch from net9.0 to main October 19, 2024 23:53
Copy link
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

This PR has ABI breaks as you noted, however, even if we switch to a new TFM (net10) old libraries that use this type and are built with an older maui will still break. People would have to recompile.

One workaround we can do is use default interface members.

Unless... and you would be on the hook for this (not sure what the hook would be and/or if it is enforceable) you say that this type is not meant to be used by anyone outside of maui and doing so is them opting into breaks.

And then this needs that oh-so-common rebase.

@StephaneDelcroix
Copy link
Contributor Author

I reviewed this again. the only risk of breaking change would be if a 3rd party provide a markup extension that invoke serviceproviderGetService(typeof(IXamlTypeResolver)) which is unlikely

@StephaneDelcroix
Copy link
Contributor Author

no longer a breaking change

@PureWeen PureWeen requested a review from mattleibow January 29, 2025 23:20
@StephaneDelcroix StephaneDelcroix removed the request for review from a team January 30, 2025 09:06
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

mattleibow
mattleibow previously approved these changes Jan 31, 2025
@StephaneDelcroix
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

PureWeen commented Feb 3, 2025

/rebase

if a type name exists with an Extension suffix, xaml rules requires us
to use that, but for x:Static we expect the user to know better
@mattleibow mattleibow merged commit 48dd62c into main Feb 4, 2025
124 checks passed
@mattleibow mattleibow deleted the fix_11833 branch February 4, 2025 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-xaml XAML, CSS, Triggers, Behaviors
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

x:Static in conjunction with XamlCompilation results in XFC0101 compile time error.
5 participants