Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New discovery on how to write bindings exports. Sharing and checking if there is something I am missing #2353

Closed
MangelMaxime opened this issue Jan 20, 2021 · 15 comments

Comments

@MangelMaxime
Copy link
Member

@alfonsogarciacaro I am currently working on bindings for Express API which requires me to bindings several others NPM packages and I just made a discovery which allows me to expose pretty clean API and avoid global namespace pollution for exposing the "imports" API.

I feel like I am pushing Fable and F# to a corner/limit and wanted to make sure it would keep working in the future.

I am still playing/improving with this discovery so not everything is set yet

In Fable.Npm package

  1. Declare an empty class which will host all the import statement. The different package will use type augmentation to add their import statement to it.
namespace rec Npm.Types

open System
open Fable.Core
open Fable.Core.JS
open Node

type [<AbstractClass>] IExports =
    class end
  1. Export the Npm.Types.IExports
namespace Npm

open Fable.Core


[<AutoOpen>]
module Exports =

    [<Global>]
    let npm : Types.IExports = jsNative

In any library which binds an npm packages

  1. Create a module Npm.Types to augments the type Npm.Types.IExports with the import statement specific to the package

⚠️ From my test it is important that the type augmentation is done in a module/namespace named Npm.Types other wise when consuming the code we only get the last type augmentation added (depends on the open order).

Package: Fable.BodyParser

[<AutoOpen>]
module rec Npm.Types

open Fable.Core


type Npm.Types.IExports with
    [<Import("*", "body-parser")>]
    member __.bodyParser with get () : BodyParser.Types.IExports = jsNative

Package: Fable.Mime

[<AutoOpen>]
module rec Npm.Types

open Fable.Core

type Npm.Types.IExports with
    [<Import("default", "mime")>]
    member __.mime with get () : Mime.Types.IExports = jsNative
  1. Use can now consume the body-parser bindings by doing:
open Npm
open Mocha

// Use npm package: "body-parser" 
npm.bodyParser.text()
// JS equivalent is
// var bodyParser = require("body-parser")
// bodyParser.text()

// Use npm package: "mime" 
npm.mime.getExtension("text/html")
// JS equivalent is
// var mime = require("mime")
// mime.getExtension("text/html)

So the rule is if you added the NuGet to your project in order to access the package just type npm.<name of the package>.

It makes easy to discover the APIs and also avoid global namespaces pollution.

I think this is a pretty cool way to defining bindings for Fable. What do you think? I am still experimenting with it and will see how it plays with ~10 npm packages/bindings.


I also made another discovery :)

Previously, when having a constructor on an API we were doing it that way:

// File: Mime.Type.fs
namespace rec Mime.Types

open System
open Fable.Core
open Fable.Core.JS

type [<AllowNullLiteral>] IExports =
    abstract Mime: MimeStatic
    abstract getType: path: string -> string option
    abstract getExtension: mime: string -> string option
    abstract define: mimes: TypeMap * ?force: bool -> unit

type [<AllowNullLiteral>] MimeStatic =
    [<EmitConstructor>] abstract Create: mimes: TypeMap -> Mime

// File: Mime.Api.fs

namespace rec Mime

open Fable.Core
open Fable.Core.JsInterop

[<AutoOpen>]
module Api =

    [<Import("default", "mime")>]
    let mime : Types.IExports = jsNative

    [<Import("default", "mime/Mime")>]
    let Mime : Types.MimeStatic = jsNative

Usage:

open Mime

let typeMap = createEmpty<Types.TypeMap>
typeMap.["text/a"] <- ResizeArray(["a"; "a1"])

let mime = Mime.Create(typeMap)

// In JavaScript you would do:
// var Mime = require("mime/Mime")
// var mime = new Mime({ "a" : "a1"})

Note that the F# code add a "fake" method Create which doesn't have an equivalent in JavaScript.

But if we change the Mime.Types.fs types to this:

type [<AllowNullLiteral>] IExports =
    [<Import("default", "mime/Mime");EmitConstructor>] // This line in particular
    abstract Mime: mimes: TypeMap -> Mime
    abstract getType: path: string -> string option
    abstract getExtension: mime: string -> string option
    abstract define: mimes: TypeMap * ?force: bool -> unit

Then we can use:

let typeMap = createEmpty<Types.TypeMap>
typeMap.["text/a"] <- ResizeArray(["a"; "a1"])

let mime = npm.mime.Mime(typeMap)

// In JavaScript you would do:
// var Mime = require("mime/Mime")
// var mime = new Mime({ "a" : "a1"})

If I "flatten" the Mime binding we have:

type Npm.Types.IExports with
    [<Import("default", "mime")>]
    member __.mime with get () : Mime.Types.IExports = jsNative

type [<AllowNullLiteral>] IExports =
    [<Import("default", "mime/Mime");EmitConstructor>]
    abstract Mime: mimes: TypeMap -> Mime
    abstract getType: path: string -> string option
    abstract getExtension: mime: string -> string option
    abstract define: mimes: TypeMap * ?force: bool -> unit

So here there is 2 things:

  1. We seems to be able to avoid the XXXStatic type with fake Create methods. More tests are needed to see if that's always possible or if that's just a special case here.
  2. It seems like we can have "nested" import on types. Is that something officially supported by Fable? It seems to works fine in Fable 3.1.1 but I wants to check with you :)
@MangelMaxime
Copy link
Member Author

If you want to play with the code/see the generated JavaScript it is https://github.com/MangelMaxime/Fable.Express/

This commit https://github.com/MangelMaxime/Fable.Express/tree/948f5a9d2afc1b8b7bcecc118f55eae87a243fd6 have the "npm extension style" available for the packages:

  • Mime --> there are some tests available
  • BodyParser --> No tests available yet

To run the project:

  1. dotnet tool restore
  2. npm i
  3. npm run watch-test

@alfonsogarciacaro
Copy link
Member

Thanks a lot for this @MangelMaxime! Using "npm" as the entry point for all bindings is a nice pattern. I hope we can convince all binding authors to use it :)

About the extra interface with the fake Create method for the static part, I agree it's annoying. When we only need a constructor, it makes sense to add it direcy to the exports as you say. When there are other static members it may get more complicated, especially if you need to inherit the class. Maybe we should recommend to use empty class declarations for the bindings again...

Btw, what do you mean exactly by "nested imports"? In the case of "mine/Mime" iirc the path is passed as is to JS.

@MangelMaxime
Copy link
Member Author

Maybe we should recommend to use empty class declarations for the bindings again...

Can you please remind me how it was done with empty class?

Btw, what do you mean exactly by "nested imports"? In the case of "mine/Mime" iirc the path is passed as is to JS.

"Nested imports" is a bad name but I had no idea how to express it ^^

type Npm.Types.IExports with
    [<Import("default", "mime")>]
    member __.mime with get () : Mime.Types.IExports = jsNative

// In another file

namespace Mime.Types =

type [<AllowNullLiteral>] IExports =
    [<Import("default", "mime/Mime");EmitConstructor>]
    abstract Mime: mimes: TypeMap -> Mime
    abstract getType: path: string -> string option
    abstract getExtension: mime: string -> string option
    abstract define: mimes: TypeMap * ?force: bool -> unit

If you look at the code above, you will see that:

  • Npm.Types.IExports.mime has an Import declaration.
  • Mime.Types.IExportsMime also got one.

However, Npm.Types.IExports.mime is exposing the types Mime.Types.IExport which have on one of it's member an Import instruction.

What I am trying to say is that a member can have an import statement and exposed a type which also have an import statement on one of it's member.

And when using npm.mime.Mime the generated code only exploit the [<Import("default", "mime/Mime");EmitConstructor>] instruction.

REPL demo

let deepImport = npm.mime.Mime(unbox null)

generates

import Mime from "mime/Mime";

export const Test_deepImport = new Mime(null);

let mime = npm.mime.getType("")

generates

import mime from "mime";

export const Test_mime = mime.getType("");

I tried to make explanation more clear I hope I succeeded 😅

@inosik
Copy link
Contributor

inosik commented Jan 21, 2021

I must say, I don't really like the npm part, it just doesn't look right to me. Especially if people use another package manager, or even if the dependency is provided by the runtime, like Node modules or APIs in VS Code.

We seems to be able to avoid the XXXStatic type with fake Create methods.

I'd love to see this. Maybe it's just me primarily being a .NET developer, but JS "classes" mapped as values of XYType is really confusing sometimes.

I'd even try to make bindings as idiomatic for F# as we can on the one hand, and on the other hand, try to write them in a way, so that devs can easily follow documentation of the respective packages. So, if the packages docs tells you to do this:

const T = require('pkg');
const instance = new T();

The corresponding code in F# should be this:

let instance = new Fable.Imports.pkg.T ()

It probably won't always be possible, but I'd like to see more of this. Modules that provide functions translate very nicely to F#, the above snippet gets harder already, though.

Did anybody other than me play around with the extern keyword? I created a sample some time ago, where I created bindings in C#, which worked great. I just tried to create bindings for a class, like in the example above, and it worked nicely, too. I'll update my example with some real-world JS dependencies.

@inosik
Copy link
Contributor

inosik commented Jan 21, 2021

I updated my experiment to Fable 3 and added another example for the URL class in Node.

@MangelMaxime
Copy link
Member Author

I must say, I don't really like the npm part, it just doesn't look right to me. Especially if people use another package manager, or even if the dependency is provided by the runtime, like Node modules or APIs in VS Code.

The goal is to avoid polluting global namespace and it also made it easy to just have open Npm at the top level of your import and not X open statement.

I am not fixed on this point, I am focusing on manually crafting the Express bindings and the package it used to be "easily" usable from Fable.

The automatic conversion by ts2fable doesn't make it easy to use and force you to use unbox or other hacks to use it.


Using C# to create the bindings is limited as you mentioned in your read-me because it can't host source code which can be needed to provide better user experience when a method return several types or easier to use code (more F# idiomatic)

For example, in one of the package a function returns:

    type ParseRangeResult =
        U2<Errored, Ranges>

In the module I provide an active pattern to safely handle the result of the function:

    [<RequireQualifiedAccess>]
    module ParseRangeResult =

        let (|UnkownError|ResultInvalid|ResultUnsatisfiable|Range|) (result : Types.RangeParser.ParseRangeResult) : Choice<Types.RangeParser.Errored, unit, unit, Types.RangeParser.Ranges> =
            if jsTypeOf result = "number" then
                let error = unbox<int> result

                if error = -1 then
                    ResultUnsatisfiable
                else if error = -2 then
                    ResultInvalid
                else
                    UnkownError (unbox result)
            else
                Range (unbox result)

Usage:

let range = parseRange.Invoke(1000, "bytes=0-499")

match range with
| ParseRangeResult.UnkownError error ->
    printfn "Result of parseRange.Invoke is an unkown error value... If this happen a fix is probably needed in the binding"

| ParseRangeResult.ResultInvalid error ->
    printfn "Result of parseRange.Invoke is an error of type ResultInvalid"

| ParseRangeResult.ResultUnsatisfiable error ->
    printfn "Result of parseRange.Invoke is an error of type ResultUnsatisfiable"

| ParseRangeResult.Range range ->
    // Here you can access your successful result
    Expect.equal range.``type`` "bytes" ""
    Expect.equal range.Count 1 ""
    Expect.equal range.[0].start 0 ""
    Expect.equal range.[0].``end`` 499 ""

Thank you for the example using C#. I am curious to see how it would handle handlers.

For example, connect package:

Right now I don't see a strong benefit for using C# for the types definition as it's just a little less verbose and prevents adding source code.

I suppose it could shine on more complex bindings which involves inheritance and stuff like that. As TypeScript is probably closer to C# than to F# but in general we find a way to make it work with F#.

My goal currently is to find a sweet spot for the bindings as the experience in bindings vary greatly and IHMO some of the current convention are not that good.

For example,

export type SimpleHandleFunction = (req: IncomingMessage, res: http.ServerResponse) => void;

converts to

type [<AllowNullLiteral>] SimpleHandleFunction =
        [<Emit "$0($1...)">] abstract Invoke: req: IncomingMessage * res: Http.ServerResponse -> unit

That's "ok" if the function is made available from JavaScript as you can write myFunction.Invoke(...) but how do you create that kind of function ?

You will have to do something like that:

let myFunction =
    fun _ _ -> 
        // ...
        ()
    |> unbox<SimpleHandleFunction>

The generated code looks like:

export const myFunction = (_arg2) => ((_arg1) => {
});

Not really what you would write in JavaScript and I often got bitten by the lack of curry/uncurry in case like that.

Also, you can easily make an error by forgetting an argument for example.

For this reason, I prefer the old way of doing it:

type SimpleHandleFunction =
            Func<IncomingMessage, Http.ServerResponse ,unit>

in this case you can still do myFunction.Invoke(...) and especially you can create the function in a typed way:

let myFunction =
    SimpleHandleFunction(
        fun _ _ -> 
            // ...
    )

generates:

export const myFunction = (_arg2, _arg1) => {
};

That's what you would write in JavaScript.

@MangelMaxime
Copy link
Member Author

@inosik Would you prefer to have each API prefixed by the package name?

open ServeStatic

// Call exposed function
let serve = serveStatic.serveStatic(dir, opts)

// Access exposed property
serveStatic.mime..

It is indeed not polluting the namespace and looks similar to what we do with Node API.

I think that my idea of hosting everything under an npm variable comes from a tweets I read this week where the person was complaing about [<AutoOpen>]. After some tought I am actually still using [<AutoOpen>] in every package in order to make the method extension happening "by default" when the library is added to the project.

I will design more api without the npm variable trick so I can compare both solutions. The difficult part of the binding is not really here

@inosik
Copy link
Contributor

inosik commented Jan 22, 2021

On namespace pollution

The goal is to avoid polluting global namespace

I'm all for this. But, has this been a problem lately? In general, each package should choose a proper namespace (more on that below) and try to avoid AutoOpening everything. But this is true for all F# code, not only Fable.

just have open Npm at the top level of your import and not X open statement.

But you're trading the ability to open Fable.Import.Package for having to type npm.Package every time, because Package is a member of an object now, and not a namespace or module anymore. But this is a general problem with the IExports pattern, and not specific to the npm prefix you proposed.

On functions and callbacks

I prefer the old way of doing it

Yes, I agree. An alias for Func<...> or a custom delegate (see below) is more natural for user provided callbacks than a custom type. But note that functions in JS can also have more members on them, so the pattern of "invokable objects" will still be needed sometimes. The serve-static package from your second post could be such a candidate. Or even express itself.

The sample you gave could also be translated to this:

// TS: type SimpleHandleFunction = (req: IncomingMessage, res: http.ServerResponse) => void;
type SimpleHandleFunction = delegate of IncomingMessage * ServerResponse -> unit

This creates a custom delegate type. It's very similar to creating an alias for a Func<...> or an Action<...>, so I'm not sure if this has any benefits.

@alfonsogarciacaro Is it intended that Fable generates a class for custom delegates?

On naming and structuring

Would you prefer to have each API prefixed by the package name?

Yes, I'd prefer if Fable.Import.Package was a namespace or a module, and not a member of an object. IMHO, this feels more natural from an F# perspective. Especially, if you have to write bindings yourself.

If you're interested, take a look at how I'd start out with bindings for Express.

On C#

First of all, I did this experiment more than three years ago, just to check if it was possible at all. Turns out, it was and still is! But, having to tell people coming from a JS background that they have to learn C# to use Fable is a hard sell. So don't take it too seriously.

I just found out that we can simply write constructors like in normal F#, and the limitations of extern can be worked around with Emit, so there's no point in using C# at all.

I also thought about reusing the bindings provided by the Retyped project. They generate and publish a lot of bindings, and I thought the Fable community could benefit from them.

@alfonsogarciacaro
Copy link
Member

alfonsogarciacaro commented Jan 24, 2021

Oh, my! How I never heard about your experiment @inosik (or did I and I forgot? 😅). Using C# for the bindings is a very interesting proposal I never thought of. I can think of the following pros and cons (some of which you already mentioned):

Pros:

  • Closer to Typescript (except for placement of type annotations)
  • No syntax duality as in F# for signature declaration
  • Can use extern to declare empty constructors, instance and static members without the jsNative trick
  • Much simpler to declare virtual methods if they need to be overridden in F#.
  • Classes can have nested classes so can be used to represents JS modules (that can have optional arguments and in TS "fake" overloads).
  • In C# we must use delegates (Func) to represent lambdas which avoids problems with currying... although this can be a con too sometimes.

Cons:

  • Another language to learn for non-.NET devs
  • Needs a separate project for the bindings (ok for packages, not so much when quickly writing custom bindings for your project)

So in general, C# can be a very good option when creating bindings for packages, but we will still need F# for quickly writing custom bindings and for the documentation. But it was actually this syntax duality in F# for signature declarations that made me push for using an extra interface both for the static parts.

Adding @Booksbaum to the discussion who has been making many contributions to ts2fable recently and may have some ideas on how to make bindings easier to write/consume.

@alfonsogarciacaro
Copy link
Member

@MangelMaxime The "empty" class declaration is just a class where you use jsNative for the method bodies. Example:

[<ImportMember(from="my-pkg")>]
type Foo() =
    member _.Foo(x: string): string = jsNative
    static member Bar(x: int, y: int): int = jsNative

let main() =
    let f = Foo()
    f.Foo("abc") |> printfn "%s"

This doesn't look so bad (although users need to remember to write the return type so the method doesn't become automatically generic) so maybe we should recommend it from now instead of the extra interface. My biggest issue is with virtual methods, that is, methods that can be overridden by a child class. In F# this becomes quite verbose because there's no virtual keyword so we need to declare the abstract signature and then a default implementation:

open Fable.Core

[<ImportMember(from="my-pkg")>]
type Parent() =
    member _.Bar: string = jsNative

    abstract Foo: string -> string
    default _.Foo(x: string): string = jsNative

type Child() =
    inherit Parent()
    override this.Foo(x) = x + this.Bar

let main() =
    let x = Child()
    x.Foo("abc") |> printfn "%s"

About "nested imports", you've hit here the secret sauce of Fable: this pattern matching where the Emitted/Imported/Replaced/Inlined active patterns decide how a call is resolved. I'm very careful not to change the order to avoid breaking changes, but if you want to make sure a certain behavior doesn't change you can PR test so we get a red a alarm when a regression happens.

In your case, the Import attribute over Mime wins over the fact that this is an abstract member (or a member of an imported type) so that's how Fable resolve the calls.

@MangelMaxime
Copy link
Member Author

About the empty class, I don't think this is require for my use case.

About "nested imports", you've hit here the secret sauce of Fable: this pattern matching where the Emitted/Imported/Replaced/Inlined active patterns decide how a call is resolved. I'm very careful not to change the order to avoid breaking changes, but if you want to make sure a certain behavior doesn't change you can PR test so we get a red a alarm when a regression happens.

Thank you for the explanation and if I keep it in my binding I will send a PR indeed ;)

On the subject of functions and callbacks the drawbacks of using System.Func or delegate is is that can't inherit from them.
Yes TypeScript allows to inherit from function declaration because they use interface to make them 🤷‍♂️

export interface RequestHandler<
    P = ParamsDictionary,
    ResBody = any,
    ReqBody = any,
    ReqQuery = ParsedQs,
    Locals extends Record<string, any> = Record<string, any>
> {
    // tslint:disable-next-line callable-types (This is extended from and can't extend from a type alias in ts<2.2)
    (
        req: Request<P, ResBody, ReqBody, ReqQuery, Locals>,
        res: Response<ResBody, Locals>,
        next: NextFunction,
    ): void;
}

export interface IRouter extends RequestHandler {

I am thinking of "faking" inheritance" in such case by adding a "Invoke" method which use System.Func so Fable transform the provided function in the correct way:

// Mimic `inherit RequestHandler` by adding the Invoke method
[<Emit("$0($1...)")>]
abstract Invoke : Func<ParamsDictionary, obj option, obj option, ParsedQs, Dictionary<obj option>, unit>

PS: I finally attacking the Express bindings after mapping it's dependencies so we should soon have a complex library mapped to see how it goes with all the things discuss in this issue.

@alfonsogarciacaro
Copy link
Member

Closing for now as the discussion seems to be over :) Please feel free to reopen if there are further questions.

@alfonsogarciacaro
Copy link
Member

Rereading this thread I realized that for some reason I missed this comment so I'll try to answer @inosik's comments:

Is it intended that Fable generates a class for custom delegates?

No, probably Fable see the as Type declarations in the F# AST so it transforms them to classes, we can open an issue to ignore delegate declarations.

I also thought about reusing the bindings provided by the Retyped project. They generate and publish a lot of bindings, and I thought the Fable community could benefit from them.

That would be a great idea. Unfortunately it seems that Bridge.Net and Retyped are discontinued. Do you know if its possible to adapt the tool they were using for the transformation?

@MangelMaxime
Copy link
Member Author

That would be a great idea. Unfortunately it seems that Bridge.Net and Retyped are discontinued. Do you know if its possible to adapt the tool they were using for the transformation?

It depends on what we want to do.

But Glutinum project goal is to explore and improve F# bindings generation. From what we discussed in a several places it seems like we still have a lot of possibility to explore in F# before asking people to write C#.

I am saying that because no generation tool can be perfect IHMO and people will probably often need to adapt it a bit. So they would have to learn F# and the runtime language like JavaScript, Python, PHP and then C# too (and how it is used by Fable).

That's a lot of concept, indirect code to have in mind when working with Fable code and I think it can be too much ^^

@inosik
Copy link
Contributor

inosik commented Aug 2, 2021

Unfortunately it seems that Bridge.Net and Retyped are discontinued. Do you know if its possible to adapt the tool they were using for the transformation?

Yeah, that's a pity. I've seen the deprecation notice just a couple of days ago when I was rereading this thread. It seems like they didn't even have a (public) tool for generating bindings. My idea back then (Fable v1) was to create a plugin, that would rewrite calls made from F# to retyped-APIs using the metadata they defined. But I didn't dive too deep into this.

@MangelMaxime Totally agreed. I ended up using C# because of the limitations of extern in F#. But the empty class approach isn't too bad actually, and I've gotten to like it in the last couple of months. I use it for bindings for Scriptable.app I'm working on from time to time. Here's a small example: https://gist.github.com/inosik/53bad32fd4447f7a3b14135cf79486a0. It takes quite a lot of time to clean up, because of the constant back and forth between the code, the docs and me always forgetting how the OOP syntax works in F# 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants