Default new runtime feature switches - #25108
Conversation
ghost
added
the
| _DebuggerSupport captures the explicitly specified value for DebuggerSupport. We'll use this value to determine if we need to | ||
| include pdbs as part of the publish output. | ||
| --> | ||
| <_DebuggerSupport>$(DebuggerSupport)</_DebuggerSupport> |
There was a problem hiding this comment.
Is this necessary? Why not just use $(DebuggerSupport) below in the Task? If someone is changing it in the meantime (for example in a .targets file that is imported after this - or in a Target), they explicitly want that behavior - not the cached value here.
There was a problem hiding this comment.
Also - since you are doing this above the line below - you are not getting the default setting... So by default _DebuggerSupport will be empty - which means true (since the default is to support debugging).
There was a problem hiding this comment.
I have a better solution to not publishing symbols by default. It relies on CopyOutputSymbolsToPublishDirectory which explicitly designed to control this behavior. Updated.
Porting #23932 to WebAssemblySDK. * Default new runtime feature switches These new feature switches have been added to the runtime to make applications smaller. Setting reasonable defaults to Blazor wasm projects. Fixes #25099 If there is an ask mode template to fill out, let me know and I can do it.
732f54e to
a2b535e
Compare
eerhardt
left a comment
eerhardt
left a comment
There was a problem hiding this comment.
Looks good.
My only concern is if someone defines a new Configuration, ex. Checked. With my original change we would keep DebuggerSupport to the default value (debugging is supported). With this change a new Configuration would have DebuggerSupport off, and it may confuse people why debugging support code (like DebuggerTypeProxys) are being trimmed. It would be safer to include DebuggerSupport by default on new configurations, the only drawback would be larger assemblies.
|
Thanks @eerhardt. For the Configuration bit, we haven't seen enough evidence that suggests that users configure their projects to use non-standard configurations. We can always patch this if this it we get feedback that our behavior is problematic. |
|
Hello @pranavkm! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
3 participants
Porting #23932 to WebAssemblySDK.
These new feature switches have been added to the runtime to make applications smaller. Setting reasonable defaults to Blazor wasm projects.
Fixes #25099