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

Add option to prefer .NET Standard over .NET Framework as TargetFramework #641

Merged
merged 3 commits into from
Jun 1, 2024

Conversation

mayuki
Copy link
Contributor

@mayuki mayuki commented Apr 26, 2024

This PR adds the option to prioritize .NET Standard over .NET Framework as TargetFramework.

image

Background

As written in Issue #621, the currently supported Unity differs from the original .NET Framework, possessing an API surface of .NET Standard 2.1, even when the API Compatibility Level is set to .NET Framework.

Due to Unity's .NET Framework a strange and unique combination of .NET Framework 4.8 and .NET Standard 2.1, various issues arise when an assembly targeted at net46x is installed.

For example, Microsoft.Bcl.AsyncInterfaces for net462 (.NET Framework) includes System.IAsyncDisposable, which is provided as an in-built API in .NET Standard 2.1. As a result, installing it in a .NET Framework Unity project causes the following compile error:

error CS0433: The type 'IAsyncDisposable' exists in both 'Microsoft.Bcl.AsyncInterfaces, Version=8.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51' and 'mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089'

Additionally, Grpc.Net.Client has libraries for net461 that are not compatible with .NET Standard 2.x and do not work, leading to unnecessary dependencies being installed.

To address this issue, #630 made it possible to specify targetFramework in the packages.config, but this does not affect transitive dependencies. In some cases, you need to specify this for each implicitly installed package. (Example: Our project's installation of Grpc.Net.Client in package.json)

Hence, I added an option to always prioritize .NET Standard 2.x over .NET Framework as the target framework choice in the project.

Remarks

The option added by this PR is turned off by default, but in my opinion, making this behavior of prioritizing .NET Standard the default could be appropriate.

The option added by this PR is turned off by default. This means that the behaviour does not change in existing projects. But it turned on by default when NuGet.config is newly created.

I believe that libraries built for .NET Standard 2.x, especially 2.1, are more modern than those built specifically for the .NET Framework. In Unity, if API Compatibility Level changes from .NET Standard to .NET Framework, the legacy API set could be used instead of the libraries that could be used in .NET Standard 2.1, but I don't think any developer would like that.

Copy link
Collaborator

@igor84 igor84 left a comment

Choose a reason for hiding this comment

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

Nice, clean work with great explanation :)

@mayuki
Copy link
Contributor Author

mayuki commented Apr 30, 2024

Thanks for the quick review!

By the way, what do you think about enabling it by default?

I believe that having a more modern API set (.NET Standard 2.1) as the primary choice, even if .NET Framework is selected as API Compatibility Level, is not a bad idea for many developers, but it will definitely introduce breaking changes, so I would like to hear your opinions.

@igor84
Copy link
Collaborator

igor84 commented Apr 30, 2024

I never used net standard packages in net framework Unity so I can't speak from experience. We actually switched all our Unity projects to net standard before we started using nuget and never had a need to switch back.

Anyway we usually handle things like this by leaving the new option as default false in code but we change the template for new nuget.config to set it to true. That way the option becomes turned on by default only for new projects while not breaking the existing ones.

@mayuki
Copy link
Contributor Author

mayuki commented Apr 30, 2024

I initially thought that .NET Standard would be sufficient in the modern scenario, but it seems that more developers than expected are still using .NET Framework and I have received feedback about various problems.

This choice is often forced by third-party SDKs and middleware that require .NET framework, irrespective of the developer's preferences.

Now I will change it so that this option is enabled by default in new nuget.config template and disabled by default in the code.

@JoC0de
Copy link
Collaborator

JoC0de commented May 26, 2024

@mayuki @igor84 Sorry for the delay. I simplified the creation of the PrioritizedTargetFrameworksPreferNetStandard20Or21, added a unit test and changed the UI a little bit.
Would be nice if you can have a quick look at the changes and 'approve' them, than we can merge the PR.

@JoC0de JoC0de merged commit 630d3db into GlitchEnzo:master Jun 1, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants