fable-node icon indicating copy to clipboard operation
fable-node copied to clipboard

Try to replace U2 arguments with overloads or common interfaces

Open alfonsogarciacaro opened this issue 6 years ago • 17 comments

alfonsogarciacaro avatar Apr 01 '19 09:04 alfonsogarciacaro

Totally for this one :)

It's how I update my bindings now and it's so much easier/natural to use

MangelMaxime avatar Apr 01 '19 09:04 MangelMaxime

ok I will refine my current WIP

whitetigle avatar Apr 11 '19 09:04 whitetigle

Question how do I transform this?

static member fileURLToPath: U2<URL,string> -> string = jsNative

whitetigle avatar Apr 11 '19 09:04 whitetigle

The error from F# is indeed "funny" when you do:

type URL = int

type Test () =
    static member fileURLToPath: string -> string = jsNative
    static member fileURLToPath: URL -> string = jsNative

Error:

Duplicate method. The method 'get_fileURLToPath' has the same name and signature as another method in type 'Test'.

So it's comparing only the number of arguments and the types... I would vote for using overload on abstract because it works here.

And for static member, something like where we rename the member but it's will not always be good when you have U3, U4, U5, etc. or several arguments that use UX...

I don't really know when Fable is generating the static member but I guess we could use a module to mock the type name and use a type with abtract member on it.

This is actually what I did to mock a function from FontAwesome which I think is similarly resolve in term of constraints as static member from F# compiler.

export function icon(icon: IconName | IconLookup, params?: IconParams): Icon;
module Exports =
    open Fable.Core.JsInterop

    type IconModule =
        [<Emit("$0({iconName : $1, prefix: $2},Object.assign({}, $3))")>]
        abstract Find: iconName: string * prefix: string * ?parameters: IconParams -> Icon
        [<Emit("$0({iconName : $1, prefix: 'far'},Object.assign({}, $2))")>]
        abstract FindRegular: icon: IconName * ?parameters: IconParams -> Icon
        [<Emit("$0({iconName : $1, prefix: 'fas'},Object.assign({}, $2))")>]
        abstract FindSolid: icon: IconName * ?parameters: IconParams -> Icon
        [<Emit("$0({iconName : $1, prefix: 'fab'},Object.assign({}, $2))")>]
        abstract FindBrand: icon: IconName * ?parameters: IconParams -> Icon
        [<Emit("$0({iconName : $1, prefix: 'fal'},Object.assign({}, $2))")>]
        abstract FindLight: icon: IconName * ?parameters: IconParams -> Icon

    let icon : IconModule = importMember "@fortawesome/fontawesome-svg-core"

And so like that, I am able to kind of mock the JavaScript API in a nice way for F# code. The emit is complex here this is to make the API even nicer to consume.

MangelMaxime avatar Apr 11 '19 09:04 MangelMaxime

hmm. why not. I'm going to try this. Thanks @MangelMaxime!

whitetigle avatar Apr 11 '19 09:04 whitetigle

Also, if the code looks ugly or is not possible to be supported. I guess we could only support one of the methods with an annotation stating it.

For example, because we can pass an URL or a string we could explicitly say we support only string and so the user need to convert URL into a string before using the method.

That's not perfect, but at least the user experience is still free of "hacks" like U2.Case1 myUrlOb or U2.Case2 myUrlAsAString or !^ myUrlAsAString

MangelMaxime avatar Apr 11 '19 09:04 MangelMaxime

I agree with you!

whitetigle avatar Apr 11 '19 10:04 whitetigle

Hmm, why you need static member? Ideally bindings should contain only interface definitions and then the module values to expose the constructor types.

In any case if you need to use static member you need to use an "implementation" signature. In the example above, the compiler thinks you're defining two getters with the same name that return two different types of functions, thus the error. Overloading would work like this:

type Test () =
    static member fileURLToPath(x: string): string = jsNative
    static member fileURLToPath(x: URL): string = jsNative

alfonsogarciacaro avatar Apr 11 '19 11:04 alfonsogarciacaro

Oh I didn't know.

If what @alfonsogarciacaro says is right then it's better than any hacks if we need static member :)

MangelMaxime avatar Apr 11 '19 11:04 MangelMaxime

I currently did that:

module Static = 
    let domainToASCII: string -> string = importMember "url"
    let domainToUnicode: string -> string = importMember "url"
    let fileURLToPath: string -> string = importMember "url"
    let fileURLToPathFromURL: URL -> string = importMember "url"
    let pathToFileURL: string -> URL = importMember "url"

The question I was asking myself this morning, was how to map static members for the Url type? So a type with static members and a single import * would work. But I had the overloading problem for fileURLToPath function.

Honestly, I'm trying to make my way and there are a few things I quite don't grasp yet like the format problem I raised in the last PR. Here's the definition. url.js defines exports like this:

module.exports = {
  // Original API
  Url,
  parse: urlParse,
  resolve: urlResolve,
  resolveObject: urlResolveObject,
  format: urlFormat,

  // WHATWG API
  URL,
  URLSearchParams,
  domainToASCII,
  domainToUnicode,

  // Utilities
  pathToFileURL,
  fileURLToPath
};

How do I access the format member? Because there seems to be a trick there: there's actually the old format method which is available from the Url prototype. I guess that's the root of my current problem: format is found but the prototype I'm refering to in teh bindings is not what is really found in the .js source code.

Again These are the kind of things I'm patiently learning ;)

whitetigle avatar Apr 11 '19 12:04 whitetigle

@whitetigle Which format API are you speaking about?

For url.format(urlObject) the following code works:

// url.format({
//   protocol: 'https',
//   hostname: 'example.com',
//   pathname: '/some/path',
//   query: {
//     page: 1,
//     format: 'json'
//   }
// });
//
// => 'https://example.com/some/path?page=1&format=json'

module URL =
    type FormatOptions =
        abstract member protocol : string with get, set
        abstract member hostname : string with get, set
        abstract member pathname : string with get, set
        abstract member query : obj with get, set

    let format (options : FormatOptions) : string = importMember "url"

let options =
    jsOptions<URL.FormatOptions>(fun o ->
        o.protocol <- "https"
        o.hostname <- "example.com"
        o.pathname <- "/some/path"
        o.query <- createObj [
            "page" ==> 1
            "format" ==> "json"
        ]
    )

let formattedURL = URL.format(options)
JS.console.log formattedURL
// Print: https://example.com/some/path?page=1&format=json

MangelMaxime avatar Apr 11 '19 12:04 MangelMaxime

Nope, the relatively new one.

whitetigle avatar Apr 11 '19 12:04 whitetigle

Ok, I have a working version which supports both format method.

Please note, that I wrote the binding from scratch without thinking if this is breaking the existant API and if this is easily scalable.

// url.format({
//   protocol: 'https',
//   hostname: 'example.com',
//   pathname: '/some/path',
//   query: {
//     page: 1,
//     format: 'json'
//   }
// });
//
// => 'https://example.com/some/path?page=1&format=json'

module URL =
    type LegacyFormatOptions =
        abstract member protocol : string with get, set
        abstract member hostname : string with get, set
        abstract member pathname : string with get, set
        abstract member query : obj with get, set

    type FormatOptions =
        /// `true` if the serialized URL string should include the username and password, `false` otherwise. Default: `true`.
        abstract member auth : bool with get, set
        /// `true` if the serialized URL string should include the fragment, `false` otherwise. Default: `true`.
        abstract member fragment : bool with get, set
        /// `true` if the serialized URL string should include the search query, `false` otherwise. Default: `true`.
        abstract member search : bool with get, set
        /// `true` if Unicode characters appearing in the host component of the URL string should be encoded directly as opposed to being Punycode encoded. Default: `false`.
        abstract member unicode : bool with get, set

    type [<AllowNullLiteral>] URL =
        abstract hash: string  with get, set
        abstract host: string  with get, set
        abstract hostname: string  with get, set
        abstract href: string  with get, set
        abstract origin: string
        abstract password: string  with get, set
        abstract pathname: string  with get, set
        abstract port: string  with get, set
        abstract protocol: string with get, set
        abstract search: string with get, set
        // abstract searchParams: URLSearchParams // commented to make reproduction easier
        abstract username: string with get, set
        abstract toString: unit -> string
        abstract toJSON: unit -> string

    type IExports =
        abstract member format : LegacyFormatOptions -> string
        abstract member format : url : URL * ?options: FormatOptions -> string
        [<Emit("new URL($1...)")>]
        abstract Create: input:string -> URL

let url : URL.IExports = import "default" "url"

// Legacy code

let legacyOptions =
    jsOptions<URL.LegacyFormatOptions>(fun o ->
        o.protocol <- "https"
        o.hostname <- "example.com"
        o.pathname <- "/some/path"
        o.query <- createObj [
            "page" ==> 1
            "format" ==> "json"
        ]
    )

let formattedURL = url.format(legacyOptions)
JS.console.log formattedURL
// Print: https://example.com/some/path?page=1&format=json

// New code

// const myURL = new URL('https://a:b@測試?abc#foo');

// console.log(myURL.href);
// // Prints https://a:b@xn--g6w251d/?abc#foo

// console.log(myURL.toString());
// // Prints https://a:b@xn--g6w251d/?abc#foo

// console.log(url.format(myURL, { fragment: false, unicode: true, auth: false }));
// // Prints 'https://測試/?abc'

let myUrl = url.Create("https://a:b@測試?abc#foo")

JS.console.log myUrl.href
// Prints https://a:b@xn--g6w251d/?abc#foo

JS.console.log (myUrl.toString())
// Prints https://a:b@xn--g6w251d/?abc#foo


let options =
    jsOptions<URL.FormatOptions>(fun o ->
        o.fragment <- false
        o.unicode <- true
        o.auth <- false
    )

let formatted = url.format(myUrl, options)
JS.console.log formatted
// Prints: https://測試/?abc

MangelMaxime avatar Apr 11 '19 14:04 MangelMaxime

Great! I'm going to see how to merge this.

whitetigle avatar Apr 11 '19 15:04 whitetigle

Ok found why it would not work!

working:

[<Import("*", "url")>]
let URL: Url.URLType = jsNative

vs

not working: let [<Global>] URL: Url.URLType = jsNative

  • put format into good place.

Thanks @MangelMaxime!

whitetigle avatar Apr 11 '19 16:04 whitetigle

Ah yes indeed:

let [<Global>] URL: Url.URLType = jsNative is compiled to something like URL

while

[<Import("*", "url")>]
let URL: Url.URLType = jsNative

compiles to

var url = require("url");

The first one is accessing a global variable while the second one is loading a module.

MangelMaxime avatar Apr 12 '19 07:04 MangelMaxime

Yes should have better read the doc:

Global Objects: These objects are available in all modules. The following variables may appear to be global but are not

whitetigle avatar Apr 12 '19 07:04 whitetigle