Fsharp: PR #2093 broke Json.NET

[majocha]

commit 13b0ba050d450ca09a3e461d6355e490a273fca9 introduced a change that breaks Json.NET deserialization of lists.

Repro steps

edit to add repro directly using Newtonsoft.Json

#r "Newtonsoft.Json"

open Newtonsoft.Json

[1; 2; 3] 
|> JsonConvert.SerializeObject 
|> JsonConvert.DeserializeObject<int list>

Expected behavior

val u : int list = [1; 2; 3]

Actual behavior

"Newtonsoft.Json.JsonSerializationException: Cannot create and populate list type Microsoft.FSharp.Collections.FSharpList`1[System.Int32]. Path '', line 1, position 1."

Workaround

Use Json.NET 9.0.2-beta or later which fixes this

Related information 

master branch
net40

Can't say anything about severity. Newtonsoft.Json is quite a popular library, for sure.

[smoothdeveloper]

@majocha it might be worth to track a bug on https://github.com/Microsoft/fsharplu I doubt we can say it is a compiler bug if some library breaks because a type exposes a new interface.

[forki]

does it also break direct serialization in json.net? TBH 13b0ba0 seems absolutely harmless

[majocha]

@forki, unfortunately it does.
Json.net assumes F# list doesn't implement IReadOnlyCollection
link

[forki]

ok. then we should log that with JSON.NET and ask @fgreinacher and @JamesNK

[majocha]

I'm not sure if the moment is right. If this change makes it into RC3 there will be simpler way to repro and test for it.

[forki]

afaik this is merged in RTM branch

[vasily-kirichenko]

I doubt there will be RC3 at all.

[JamesNK]

How come FSharpList<T> doesn't have a constructor that takes an IEnumerable<T>? If it had one then it would automatically work with Json.NET

[cartermp]

@JamesNK F# lists are defined as a DU, which you can't have constructors for. They're defined this way (instead of as a class) so you can pattern match on [] and head :: tail cases.

[dsyme]

@JamesNK Yeah, I'm sorry about this. FSharpList is just a bit odd-looking from the C# perspective and doesn't have the usual shape for a collection. As @cartermp says this is because it happens to be defined as an F# discriminated union.

@JamesNK Can you fix this in Json.NET?

@cartermp @KevinRansom It's a real shame that adding IReadOnlyCollection has busted something so fundamental - it's a lesson to me that it's not necessarily worth changing anything in FSharp.Core, and not so late in the development cycle

[fgreinacher]

https://github.com/JamesNK/Newtonsoft.Json/pull/1181 has the fix for Json.NET.

[fgreinacher]

Important question: When will F# 4.1 be released? I think @JamesNK wants to release the next Json.NET version after VS 2017 is out.

[cartermp]

@fgreinacher F# 4.1 will ship alongside VS 2017 (although you can use it in production today due to the Go-live license in vs2017 RC, and it's available in Ionide for VSCode).

[cartermp]

@dsyme Agreed regarding the development cycle. We should honor a cutoff date.

[liboz]

Sorry about causing this. I didn't expect adding that interface to cause any issues really. Is there anything that can be done in the future to stop issues like this from happening other than having a cutoff date?

[KevinRansom]

@liboz

This is a good change that we would have taken for FSharp.Core it improves F# interop with C#. It demonstrates, however, why we are resistant to last minute changes, the dotnet eco-system is huge and destabilizing that is always risky.

The issue with json.net and fsharplist has been found and fixed, so the preview process worked well ... if somewhat uncomfortably for all involved.

Kevin

[JamesNK]

Fix is merged in Json.NET

I'll probably do a new pre-release version on NuGet sometime soon so this is available for people who use the latest version of FSharp.Core. The next non-prerelease version of Json.NET will come out after VS2017.

[dsyme]

Closing as this has been dealt with

[dsyme]

Fix is in Json.NET 9.0.2-beta and later

https://www.nuget.org/packages/Newtonsoft.Json/9.0.2-beta2