Add section on making FSharp.Core references explicit

[dsyme]

cc @cartermp and @ctaggart

This adds advice to use explicit FSharp.Core references

[dsyme]

There is also a downside to mention here. If made explicit, this will potentially result in confusing compile errors in the future, should the underlying compiler be updated and a feature be used that depends on a newer FSharp.Core version.

A concrete example of this in another language was C# 7.0 tuple support, where the majority of users who tried the feature were given an error saying that they didn't have System.ValueTuple. This caused massive confusion, since people assumed they could just upgrade their tools (which came with a new compiler) and start using the new feature. We communicate things the same way for F# (use F# 4.5 and get new byref stuff!), so it stands to reason that people will be confused by this if they stick with an explicit reference and aren't as savvy about how language features surface as we are.

This is at the root of our disagreement on the VF# repo, and I think it's important to mention as the tradeoff being made here.

OK, I'll mention it.

FWIW I think this scenario is optimizing for the wrong thing (language junkies who enjoy fiddling with new corner-case language features) rather than crucial software engineering soundness (for users of the existing language and toolchain).

Further, we could address this issue by simply adding decent compiler messages (e..g. "feature XYZ is only available if referencing FSharp.Core 134.2.4.2").

We should optimize for correctness and soundness, not for our ability to implicitly and silently onboard people to new language features.

[dsyme]

This is particularly important when making nuget packages where you don't want to unexpectedly force a package upgrade on downstream consumers.

I don't believe that this is correct given our recent bug fixes and how NuGet works. If you specify a higher version for your library, but the project references a lower version, it will incur a package downgrade instead of an upgrade. Though this may be good to verify before publishing.

I don't really understand the comment, but more importantly I don't understand what's incorrect about the guidance :)

No one ever, ever wants their library nuget package to suddenly imply a dependency on some random higher version of FSharp.Core just because you installed some new .NET SDK on the machine you're doing the build on.

I've published hundreds of F# library packages and every time I've wanted to control the FSharp.Core dependency explicitly. Having the generated dependencies for your nuget package depend on the particular .NET SDK you're using is just like playing with hand-grenades because you think they make good cricket balls. It's wrong and in this community guidance we should simply say "just make your FSHarp.Core dependency explicit".

[cartermp]

@dsyme

This is particularly important when making nuget packages where you don't want to unexpectedly force a package upgrade on downstream consumers.

This is not how NuGet works. The combination of:

• Lowest applicable version
• Nearest wins

Means that users will see a package downgrade warning, not an unexpected upgrade. Example:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>netcoreapp2.1</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <Compile Include="Program.fs" />
  </ItemGroup>

  <ItemGroup />

  <ItemGroup>
    <PackageReference Update="FSharp.Core" Version="4.3.4" />
    <PackageReference Include="Akkling" Version="0.9.2"/>
  </ItemGroup>

</Project>

dotnet restore
  Restoring packages for /Users/phillip/scratch/test/test.fsproj...                                                                                                               
  Installing Hyperion 0.9.8.                                                                                                                                                      
  Installing Akka.Serialization.Hyperion 1.3.8-beta66.                                                                                                                            
  Installing System.ValueTuple 4.3.1.                                                                                                                                             
  Installing Akka 1.3.8.                                                                                                                                                          
  Installing Akkling 0.9.2.                                                                                                                                                       
/Users/phillip/scratch/test/test.fsproj : warning NU1605: Detected package downgrade: FSharp.Core from 4.5.0 to 4.3.4. Reference the package directly from the project to select a
 different version.                                                                                                                                                               
/Users/phillip/scratch/test/test.fsproj : warning NU1605:  test -> Akkling 0.9.2 -> FSharp.Core (>= 4.5.0)                                                                        
/Users/phillip/scratch/test/test.fsproj : warning NU1605:  test -> FSharp.Core (>= 4.3.4)                                                                                         
  Restore completed in 3.24 sec for /Users/phillip/scratch/test/test.fsproj.   

And the project will run:

dotnet run
/Users/phillip/scratch/test/test.fsproj : warning NU1605: Detected package downgrade: FSharp.Core from 4.5.0 to 4.3.4. Reference the package directly from the project to select a
 different version.                                                                                                                                                               
/Users/phillip/scratch/test/test.fsproj : warning NU1605:  test -> Akkling 0.9.2 -> FSharp.Core (>= 4.5.0)                                                                        
/Users/phillip/scratch/test/test.fsproj : warning NU1605:  test -> FSharp.Core (>= 4.3.4)                                                                                         
/Users/phillip/scratch/test/test.fsproj : warning NU1605: Detected package downgrade: FSharp.Core from 4.5.0 to 4.3.4. Reference the package directly from the project to select a
 different version.                                                                                                                                                               
/Users/phillip/scratch/test/test.fsproj : warning NU1605:  test -> Akkling 0.9.2 -> FSharp.Core (>= 4.5.0)                                                                        
/Users/phillip/scratch/test/test.fsproj : warning NU1605:  test -> FSharp.Core (>= 4.3.4)                                                                                         
Hello World from F#!                                                                                                                                                              

Similarly, if I'm on an older SDK that doesn't have FSharp.Core 4.5.x and don't reference FSharp.Core explicitly in my project, I will see a downgrade warning.

[dsyme]

Right, but nuget is simply broken here. Each of those warnings represent likely catastrophic failure at runtime. They should not be warnings, and of course paket would report a hard failure on paket update in these scenarios. So I don't take the nuget behaviour here as a credible baseline, it's just unsoundness by design.

[dsyme]

In this case, the C# project references 4.3.4 and that is respected. Note that in the C# targets, NU1605 is treated as an error rather than a warning; we do not do this in F#.

Why don't we treat this as an error for F#? That would certainly help.

[cartermp]

Opinions on NuGet behavior here is orthogonal - the current guidance as-written now states that you can auto-upgrade consumers of your library by taking a dependency on a higher FSharp.Core version, when that is not what happens.

I'm personally fine with us treating it as an error - let's file an issue on VF#.