Skip to content

Conversation

@jonathanpeppers
Copy link
Member

Context: #14591 (comment)

In testing a customer project, I found that using a non-standard $(Configuration) name like ReleaseProd disables XamlC.

In a30e243, we added a check for $(Configuration) to be exactly Release. Let's invert this check to be != Debug. I also added a test to verify this works.

This seems like a better default for customers, as it seems a lot worse for a ReleaseProd build to have XamlC off than a DebugProd build to accidentally have XamlC on.

Asking around, but the .NET SDK doesn't appear to do anything special for $(Optimize) and $(DebugSymbols) related to $(Configuration):

https://github.com/dotnet/sdk/blob/7d23e9d3e4aad58a5b497d8d91a50ffdf148b238/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.props#L46-L54

@jonathanpeppers jonathanpeppers added the area-xaml XAML, CSS, Triggers, Behaviors label Apr 18, 2023
Context: dotnet#14591 (comment)

In testing a customer project, I found that using a non-standard
`$(Configuration)` name like `ReleaseProd` disables XamlC.

In a30e243, we added a check for `$(Configuration)` to be exactly
`Release`. Let's invert this check to be `!= Debug`. I also added a
test to verify this works.

This seems like a better default for customers, as it seems a lot
worse for a `ReleaseProd` build to have XamlC off than a `DebugProd`
build to accidentally have XamlC on.

Asking around, but the .NET SDK doesn't appear to do anything special
for `$(Optimize)` and `$(DebugSymbols)` related to `$(Configuration)`:

https://github.com/dotnet/sdk/blob/7d23e9d3e4aad58a5b497d8d91a50ffdf148b238/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.props#L46-L54
@jonathanpeppers jonathanpeppers force-pushed the ConfigurationNamesXamlC branch from 77c978d to 9047b79 Compare April 18, 2023 19:48
@jonathanpeppers
Copy link
Member Author

Ok, the new tests are running/passing:

image

I had a little trouble running these locally.

@jonathanpeppers jonathanpeppers marked this pull request as ready for review April 18, 2023 21:41
@mattleibow
Copy link
Member

Not sure we want to do anything special, what if I have a DebugLogging or something. Maybe we should just document the features?

Unless there is some other property that is set when we are running some optimized build. How does a console app handle optimization in a ReleaseProd or a DebugWithLogging?

Once we start adding exceptions we no have to add all exceptions. Maybe not, but where do we stop?

@jonathanpeppers
Copy link
Member Author

I asked the .NET SDK team, and they have no solution for this. Take for example the link above:

https://github.com/dotnet/sdk/blob/7d23e9d3e4aad58a5b497d8d91a50ffdf148b238/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.props#L46-L54

Customers have to know, if they want to create ReleaseProd at minimum set Optimize=true. And that doesn't even count the iOS/Android settings you'd need.

For this case, it's way worse for XamlC to be off -- so this seems like the fix for now until there is a general .NET SDK / MSBuild feature or design.

@jonathanpeppers
Copy link
Member Author

The customer's app I found this in, was spending ~2.2s parsing XAML at runtime. 👀

image

@mattleibow mattleibow merged commit 7234bbe into dotnet:main Apr 19, 2023
@jonathanpeppers jonathanpeppers deleted the ConfigurationNamesXamlC branch April 19, 2023 21:44
@github-actions github-actions bot locked and limited conversation to collaborators Dec 11, 2023
@samhouts samhouts added the fixed-in-8.0.0-preview.4.8333 Look for this fix in 8.0.0-preview.4.8333! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-xaml XAML, CSS, Triggers, Behaviors fixed-in-8.0.0-preview.4.8333 Look for this fix in 8.0.0-preview.4.8333!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants