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

[release/8.0-staging][wasm] remove problematic default settings #112216

Open
wants to merge 1 commit into
base: release/8.0-staging
Choose a base branch
from

Conversation

lewing
Copy link
Member

@lewing lewing commented Feb 5, 2025

Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@dotnet-issue-labeler dotnet-issue-labeler bot added needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners new-api-needs-documentation labels Feb 5, 2025
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@lewing
Copy link
Member Author

lewing commented Feb 6, 2025

This probably still doesn't fully resolve the linked issue but there are others that are related.

@vitek-karas
Copy link
Member

The majority root cause of the linked issues is that trimmer removes parameter names from methods (.ctors) - this is problematic for serialization of KeyValuePair for example as the logic there relies on the parameter names.
This change modifies trimming of attributes and nullability info (attributes which encode the ? in C#) - I don't see how that will fix the original problem. As seen in the comments the workaround was to tell trimmer to not remove parameter names. If we can't fix the reflection any other way, I would suggest we add that to the default settings for trimmer for Blazor instead.

@sbomer for trimmer official guidance, I don't work on it directly anymore.

@sbomer
Copy link
Member

sbomer commented Feb 6, 2025

That issue also mentioned #81979 which was fixed in .NET 9. Does the issue repro when targeting net9.0? If this fixes it we could consider backporting #102850 to .NET 8.

@akoeplinger
Copy link
Member

That issue also mentioned #81979 which was fixed in .NET 9. Does the issue repro when targeting net9.0? If this fixes it we could consider backporting #102850 to .NET 8.

We removed setting the switches (same change as this PR) in #102934 for 9.0 so it's hard to say.

@lewing lewing added this to the 8.0.x milestone Feb 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Workloads Workloads like wasm-tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants