fix(all): pass raw value for optional arguments of native bindings (#4688)

Author: MangelMaxime

src/Fable.Transforms/FSharp2Fable.Util.fs

@@ -2908,6 +2908,21 @@ module Util =
         else
             ValueNone
 
+    /// Member bound to native code (binding) rather than implemented in F#.
+    let private callsExternalBinding (memb: FSharpMemberOrFunctionOrValue) =
+        isEmittedOrImportedMember memb
+        || match memb.DeclaringEntity with
+           | Some ent ->
+               isGlobalOrImportedFSharpEntity ent
+               || isErasedOrStringEnumFSharpEntity ent
+               // Fable's own bindings carry no attribute, match by namespace.
+               || (
+                   match ent.TryFullName with
+                   | Some name -> name.StartsWithAny("Fable.Core.JS.", "Fable.Core.Py.")
+                   | None -> false
+               )
+           | None -> false
+
     /// Removes optional arguments set to None in tail position
     let transformOptionalArguments
         (com: IFableCompiler)
@@ -2923,11 +2938,16 @@ module Util =
         then
             args
         else
+            // Native bindings expect the raw value, not F#'s `Some` wrapper for `?` args.
+            let unwrapProvidedOptional = callsExternalBinding memb
+
             (memb.CurriedParameterGroups[0], args, (true, []))
             |||> Seq.foldBack2 (fun par arg (keepChecking, acc) ->
-                if keepChecking && par.IsOptionalArg then
+                if par.IsOptionalArg then
                     match arg with
-                    | Fable.Value(Fable.NewOption(None, _, _), _) -> true, acc
+                    | Fable.Value(Fable.NewOption(None, _, _), _) when keepChecking -> true, acc
+                    | Fable.Value(Fable.NewOption(Some value, _, _), _) when unwrapProvidedOptional ->
+                        false, value :: acc
                     | _ -> false, arg :: acc
                 else
                     false, arg :: acc

tests/Integration/Integration/data/optionalArgs/OptionalArgs.fs

@@ -0,0 +1,39 @@
+module OptionalArgs
+
+open Fable.Core
+open Fable.Core.JsInterop
+
+// An optional argument (`?x: obj`) passed to a native binding must reach the native
+// function as the raw value. `obj`-typed optionals are not flattened, so without
+// unwrapping the F# `Some` wrapper (`some(...)`) would leak into the generated code.
+
+// JS binding (Fable.Core.JS)
+let jsBinding (v: obj) (sp: obj) : string = JS.JSON.stringify (v, space = sp)
+
+// Imported binding
+[<Import("Thing", "my-lib")>]
+type Thing =
+    abstract doIt: x: int * ?y: obj -> string
+
+let imported (t: Thing) (n: obj) : string = t.doIt (1, y = n)
+
+// Erased binding
+[<Erase>]
+type Erased =
+    abstract run: a: int * ?b: obj -> string
+
+let erased (e: Erased) (n: obj) : string = e.run (1, b = n)
+
+// Emit member
+type IFoo =
+    [<Emit("$0.foo($1, $2)")>]
+    abstract foo: x: int * ?y: obj -> string
+
+let emitted (foo: IFoo) (n: obj) : string = foo.foo (1, y = n)
+
+// Plain F# member: the implementation consumes the argument as `obj option`,
+// so the `some(...)` wrapper must be kept.
+type Native() =
+    static member Show(x: int, ?y: obj) : string = string x + string (defaultArg y (box "-"))
+
+let fSharpMember (n: obj) : string = Native.Show(1, y = n)

tests/Integration/Integration/data/optionalArgs/OptionalArgs.jsx.expected

@@ -0,0 +1,41 @@
+import { class_type } from "./fable_modules/fable-library-js.5.4.0/Reflection.js";
+import { int32ToString } from "./fable_modules/fable-library-js.5.4.0/Util.js";
+import { toString } from "./fable_modules/fable-library-js.5.4.0/Types.js";
+import { some, defaultArg } from "./fable_modules/fable-library-js.5.4.0/Option.js";
+
+export function jsBinding(v, sp) {
+    return JSON.stringify(v, undefined, sp);
+}
+
+export function imported(t, n) {
+    return t.doIt(1, n);
+}
+
+export function erased(e, n) {
+    return e.run(1, n);
+}
+
+export function emitted(foo, n) {
+    return foo.foo(1, n);
+}
+
+export class Native {
+    constructor() {
+    }
+}
+
+export function Native_$reflection() {
+    return class_type("OptionalArgs.Native", undefined, Native);
+}
+
+export function Native_$ctor() {
+    return new Native();
+}
+
+export function Native_Show_21A309C4(x, y) {
+    return int32ToString(x) + toString(defaultArg(y, "-"));
+}
+
+export function fSharpMember(n) {
+    return Native_Show_21A309C4(1, some(n));
+}

tests/Integration/Integration/data/optionalArgs/optionalArgs.fsproj

@@ -0,0 +1,16 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <TargetFramework>net10.0</TargetFramework>
+    <RollForward>Major</RollForward>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <Compile Include="OptionalArgs.fs" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Fable.Core" Version="4.5.0" />
+  </ItemGroup>
+
+</Project>

tests/Integration/Integration/data/signatureHidesFunction/HideType.jsx.expected

@@ -1,6 +1,5 @@
 import { Union } from "./fable_modules/fable-library-js.4.13.0/Types.js";
 import { union_type, string_type, int32_type } from "./fable_modules/fable-library-js.4.13.0/Reflection.js";
-import { some } from "./fable_modules/fable-library-js.4.13.0/Option.js";
 
 class Foobar extends Union {
     constructor(tag, fields) {
@@ -18,6 +17,6 @@ function Foobar_$reflection() {
 }
 
 export function someFunction() {
-    console.log(some("meh"));
+    console.log("meh");
 }
 

tests/Js/Main/MiscTests.fs

@@ -560,6 +560,13 @@ let tests =
         parsed.widget.text.vOffset |> equal 500.
 #endif
 
+    testCase "Named optional argument to a JS binding is not wrapped in an option" <| fun () ->
+        // `space` is an optional parameter of the JSON.stringify binding; passing it as a
+        // named argument must forward the raw value (not a Fable option), otherwise the
+        // generated TypeScript references an Option<number> and fails to type-check.
+        let json = Fable.Core.JS.JSON.stringify({| value = 1 |}, space = 2)
+        json.Contains("\n") |> equal true
+
     testCase "Lambdas returning member expression accessing JS object work" <| fun () -> // #2311
         let x = inlineLambdaWithAnonRecord (fun x -> x.A)
         x() |> equal 1