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

Remove OSPlatformAttribute attributes from wasm linked output #41799

Merged
merged 1 commit into from
Sep 3, 2020

Conversation

marek-safar
Copy link
Contributor

Fixes #40163

@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@marek-safar marek-safar requested a review from eerhardt September 3, 2020 14:43
Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

Were you able to measure if this has any discernible impact on size in a Blazor wasm application? My assumption would be that it doesn't, since we have only annotated 2 types of APIs with these attributes so far:

  1. Windows only - which should never get called in a Blazor WASM app.
  2. UnsupportedOSPlatform("browser") - which also should never get called in a WASM app.

I was thinking this would more affect Xamarin since there we have APIs that are iOS or Android specific, and the application would actually call those APIs.

@marek-safar
Copy link
Contributor Author

marek-safar commented Sep 3, 2020

@eerhardt yeah, for the ideal app UnsupportedOSPlatformAttribute should have no effect on the final size as APIs which throw PNSE should not be reachable (and linked away) for such configuration but I don't think we can guarantee that.

The case for SupportedOSPlatformAttribute is simpler and we should just remove it all the time.

@marek-safar
Copy link
Contributor Author

/backport to release/5.0-rc2

@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2020

Started backporting to release/5.0-rc2: https://github.com/dotnet/runtime/actions/runs/238084971

@jkotas jkotas merged commit 06d5a5e into dotnet:master Sep 3, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trim OSPlatformAttributes from WASM apps
5 participants