[JS] casting a .NET Dictionary to ICollection JS should not use .push

[goswinr]

Casting a .NET Dictionary to an ICollection emits .push for .Add, but a JS Map has no .push member.

open System.Collections.Generic

let dic = Dictionary<int,string>()  

let coll = dic :> ICollection<KeyValuePair<_,_>>    

let kv = KeyValuePair(1,"a")

coll.Add(kv) // emits .push on a JS Map, but Map has no .push member

REPL

fable 4.21

[MangelMaxime]

It seems like this is not has easy to fix as I though.

JS Map doesn't seem to have a Add equivalent, it only have a set prototype for adding / updating an element.

I think the issue comes from the fact that Dictionary<int, string>() is represented using JS.Map instead of a class / wrap we control.

Because of that, when calling:

open System.Collections.Generic

let dic = Dictionary<int,string>()  

dic.Add(2, "b")

Then Fable is able to replace the Add call with addToDict

import { addToDict } from "fable-library-js/MapUtil.js";

export const dic = new Map([]);

addToDict(dic, 2, "b");

But when doing the cast to ICollection Fable can't do the replacement, because the concrete type of ICollection can be anything. I think the path to fix this issue is to rework how Dictionary<'Key, 'Value> works and instead of using JS.Map directly, we should extends it:

 class ExtendedMap extends Map {
    constructor() {
        super();
    }

   add(key, value) { // This will replace the call to `addToDict`
    // ...
   }
 }

In theory, it means that the new Map class will keep fulfilling the following statement from Fable

If people compare the constructor there will be issues, but the existing API should keep working.

[goswinr]

@MangelMaxime I am confused about how Interfaces are used in Fable. I have a type that implements IDictionary<K,V> see REPL it seems to correctly generate the ICollection interface: But then just calls pushwhen used: js: Also, the IDictionary.Add interface uses addToDict, but the toConsole side effect in the implementation is missing: becomes:

[MangelMaxime]

My theory, is when doing:

let iD = (b :> IDictionary<string, int>)
iD.Add("B",2)

Fable only see the type of iD as IDictionary and because of that the code hit Fable replacement code. And doesn't use your custom Type implementation.

This can be visible when doing:

type MyDict<'K,'V when 'K:equality > () =
    // ..
    interface IDictionary<'K,'V> with

        member _.Add(k, v) = 
            printfn "add IDictionary"
            failwith "dwdwdw" // I am on purpose not using `dic.Add(k, v)`
    // ..

let iD = (b :> IDictionary<string, int>)
iD.Add("B",2)

generates

export class MyDict$2 {
    // ..
    "System.Collections.Generic.IDictionary`2.Add5BDDA1"(k, v) {
        toConsole(printf("add IDictionary"));
        throw new Error("dwdwdw");
    }
    // ..
}
export const iD = b;

addToDict(iD, "B", 2);

I don't think there is a way to implements to implements your own MyDict like you are trying to do. Where you trying to do that for working around the issue reported here ?

[goswinr]

Yes, I was trying to work around the original issue. Do you know why correct code is generated for the interface but not used?

[MangelMaxime]

This is because IDictionary hit Fable replacement.

Fable replacement is the mechanism that's allow Fable to support native API, you cannot say I don't want replacement to happen.

To work around the issue, you can use dynamic typing temporary to create a helper function.

[MangelMaxime]

I am looking at how to fix ICollection and I don't know how to do it.

The problem is that ICollection can be a lot of different types in reality with different APIs supported.

let dic = Dictionary<int,string>()
let dicCollection = dic :> ICollection<_>    
let kv = KeyValuePair(1,"a")
printfn "Dic collection count: %d" dicCollection.Count

let intArray : int array = [|1;2;3|]
let intCollection = intArray :> ICollection<int>
intCollection.Add(4)
printfn "Int collection count: %d" intCollection.Count

let stringArray : string array = [|"a";"b";"c"|]
let stringCollection = stringArray :> ICollection<string>
stringCollection.Add("d")
printfn "String collection count: %d" stringCollection.Count

With the example above, we already have 3 different types at runtime (with different APIs)

• JS.Map for Dictionary (don't have add only set)
• Int32Array for int array (cannot be mutated directly)
• Standard JS array (aka []) for string array (works fine because there is push)

@ncave Do you have any ideas? I can implement it with some guidance, I just don't know how we can ensure that ICollection works for any possible type.

[ncave]

@MangelMaxime In theory we need different wrappers for each internal type that is supposed to implement ICollection, and return those on casting.

[MangelMaxime]

That was my supposition.

So the idea is for:

open System.Collections.Generic

let stringArray : string array = [|"a";"b";"c"|]
let stringCollection = stringArray :> ICollection<string>
stringCollection.Add("d")

export const stringArray = ["a", "b", "c"];
export const stringCollection = stringArray;
void (stringCollection.push("d"));

to becomes something like that?

export const stringArray = ["a", "b", "c"];
export const stringCollection = castArrayToIColletion(stringArray);
void (stringCollection.push("d"));

I suppose this also means we will need to support the other direction too going from ICollection to string array?

[ncave]

@MangelMaxime We could, technically the wrapper can just return the wrapped collection. But I'm sure I'm glossing over some details.

[goswinr]

Thanks for looking into this. For the .Remove member of ICollection it is even more tricky. It doesn't throw an error, but actually never removes the KeyValue Pair. see REPL

[ncave]

To clarify, although #3949 fixes the current issue using runtime checks, arguably a better way forward would be to eventually provide proper F# wrappers for all native JS collections, so that all their other interfaces can be easily implemented.

Something like this which does it for JS Map/Set, but we probably want a JS Array wrapper for ResizeArray too.

[goswinr]

Thank you for fixing this @ncave ! Is there an overview of which Interfaces of .NET Collections are supported in Fable ? Maybe documenting and failing at compile time it is good enough? For Resizearray there are these three issues (or more? ): https://github.com/fable-compiler/Fable/issues/3812 https://github.com/fable-compiler/Fable/issues/3718 https://github.com/fable-compiler/Fable/issues/2506

[ncave]

@goswinr I'm not sure, each new version of .NET adds quite a few new interfaces. For interfaces that match replacement types one to one, it's easy to add support. For interfaces implemented on multiple native types (without wrappers), we need runtime checks.

For now I've also added the mappings and tests for IReadOnlyCollection (see #3953).