fable-compiler / Fable

F# to JavaScript, TypeScript, Python, Rust and Dart Compiler
http://fable.io/
MIT License
2.92k stars 300 forks source link

Arguments not uncurried when inlining methods #2436

Open alfonsogarciacaro opened 3 years ago

alfonsogarciacaro commented 3 years ago

For example, in this case the handler will be uncurried:

static member onConnect(handler: Event -> Args -> unit) = "onConnect" ==> handler

But if we inline the function passed as handler will remain curried because Fable loses the information about the expected arity. We should uncurry the arguments when inlining the function.

AkosLukacs commented 3 years ago

Not sure if related, but "arguments" and "curried" applies to my case. Context: trying to create a function that will be passed to a third party library. The caller expects a function with 3 parameters. So something like this should work:

    let setCellValue (newData: obj) (value: obj) (oldData: obj) =
        JS.console.log("@setCellValue", newData, value, oldData)

The immediate value is good:

    const setCellValue = (newData, value, oldData) => {
        console.log(some("@setCellValue"), newData, value, oldData);
    };

But as soon as I try to pass that function anywhere, it gets curried, even a console.log does it:

console.log(some("setCellValue:"), curry(3, setCellValue));

Console.log is just for demonstration, the same happens when I try to pass in to the third-party code.

Creating a tupled function on Fable side doesn't help either, because the compiled JS will be like

    const setCellValuet = (tupledArg) => {
        const newData_1 = tupledArg[0];
        const value_1 = tupledArg[1];
        const oldData_1 = tupledArg[2];
        console.log(some("@setCellValuet"), newData_1, value_1, oldData_1);
    };

Not what the caller expects

Any idea about a workaround? And sorry if this issue is for something else...

MangelMaxime commented 3 years ago

For history, here is my answer from Gitter.

Yes in F# function are curried some they can partially called.

Fable has some specials mechanism to automatically uncurry the function making it easier to called it from JavaScript. For example, top level function are uncurry by default, etc.

That is the section of documentation speaking about it: https://fable.io/docs/communicate/fable-from-js.html#Functions:-automatic-uncurrying

You can also force Fable to uncurry the function by using System.Func

let heyImAString = System.Func<_,_,_,_> setCellValue
export function heyImAString() {
    return (delegateArg0, delegateArg1, delegateArg2) => {
        setCellValue(delegateArg0, delegateArg1, delegateArg2);
    };
}
PaigeM89 commented 3 years ago

I think I found some weird edge case bug with currying & partial application. I'm getting an error because a partially applied function is not being executed after having the remaining arguments applied, and thus not returning the expected type.

If you pull down the branch weird-fable-bug in my fork of FsLibLog, run yarn start from examples/JsConsoleExample, navigate to localhost:8091, and click the button, you'll see an error in the console of Uncaught TypeError: Types_Stack$1__Pop(...).Dispose is not a function.

This error is because the type returned from popping the stack isn't Disposable, but the weird part is that changing the type aliases of Logger and MapContext in src/FsLibLog/FsLibLog.fs (lines 21-25) to System.Func fixes the issue. I can't see any differences in the output javascript, but I'll admit that I'm not sure I'm looking in the right places.

So these type aliases cause errors later on, during runtime:

type Logger = LogLevel -> MessageThunk -> exn option -> obj array -> bool
type MappedContext = string -> obj -> bool -> IDisposable

And these type aliases work fine (with, of course, all the relevant code updated - it's not a lot):

type Logger = Func<LogLevel, MessageThunk, exn option, obj array, bool>
type MappedContext = Func<string, obj, bool, IDisposable>

I got this bug on Fable 3.2.14, with Fable.Core 3.2.9.

Please let me know what additional info I can provide or if I should make a new issue. Thanks!

alfonsogarciacaro commented 3 years ago

Well, it's not actually weird because it has happened other times :) It's a likely a different problem to the issue mentioned on top of this thread, but with functions returning functions sometimes the uncurrying mechanism has trouble because it only sees a single function type. Using delegates helps because then the compiler it's two different functions (actually there's no uncurrying from the beginning). Can you point to a place in your code where a function returns Logger or MappedContext? Maybe we can try to recognize the pattern and throw a warning just in case if it cannot be fixed.

PaigeM89 commented 3 years ago

In JsConsoleLogProvider.fs, line 89, GetLogger partially applies a value and the remaining arguments collectively create the Logger type.

// all inside type JsConsoleProvider()

let writeMessage name logLevel (messageFunc : MessageThunk) ``exception`` formatParams =
     // do things here
     true

interface ILogProvider with
    member this.GetLogger(name: string): Logger =
        // return a Func to avoid the runtime error
        //Func<_, _, _, _, bool>(writeMessage name)
        writeMessage name

This getLogger function is called once a name has been generated, or entered, such as from getLoggerByName:

// FsLibLog.fs, line 765
let getLoggerByName (name : string) =
    let loggerProvider = getCurrentLogProvider ()

    let logFunc =
        match loggerProvider with
        | Some loggerProvider -> loggerProvider.GetLogger(name)
        | None -> noopLogger

    { new ILog
        with
            member x.Log = logFunc
            member x.MappedContext = openMappedContextDestucturable }

I wasn't able to recreate the error when fiddling around with various partially applied functions, but there's a lot of interesting things happening here to consider. Among them is the fact that this function is then assigned to a record property, to be executed later. Some initial fiddling with this idea wasn't able to recreate the error.

Ultimately, it bombs on logger.fromLog (FsLibLog.fs, line 129), because the use __ value is disposed - or should be - at the end, but the values inside it are not IDisposable (they're the uncurried function, instead). It's also worth noting that logger.Log is not invoked at this point if we're using regular curried functions, so not only do we get the error, we also don't even get the log.

member logger.fromLog (log : Log) =
    use __ =
        log.AdditionalNamedParameters
        |> List.map(fun (key,value, destructure) -> logger.MappedContext key value destructure)
            //logger.MappedContext.Invoke(key, value, destructure)) // Invoking works as normal
        |> DisposableStack.Create
    log.Parameters
    |> List.toArray
    |> logger.Log log.LogLevel log.Message log.Exception // doesn't log
alfonsogarciacaro commented 3 years ago

Thanks a lot for the additional @PaigeM89! This helped me identify the issue (hopefully): Fable was not uncurrying the arguments passed to an object expression (ILog here) as it did with records. With this change your branch is working. I'll try to push a new release :+1: