Nested option warning

[MangelMaxime]

Description

In Hink, I am using optional arguments and with the latest version of Fable, I got the warnings:

warning FABLE: Nested option in option won't work at runtime

I am having this warning since I updated Hink to fable 1.3-beta-001 before I was using fable 1.2.1. I am not sure, if fable 1.2.1 was already having the warning feature or if it's a regression.

Repro code

open FSharp.Reflection
open FSharp.Core
open Fable.Core
open Fable.Core.JsInterop
open System
open System.Text.RegularExpressions

open System
open System.Text.RegularExpressions

    type BugRepro =
        { Dummy : float }

        member this.Check(?options) =
            let options = defaultArg options 3
            this.Check2(options)

        member this.Check2(?options) =
            3

Expected and actual results

Not warning displayed or perhaps allow users to not print them. It's make a lot of noise in the console output and make it harder to spot the "real" warnings/errors.

Related information

• Fable version (dotnet fable --version): Fable (1.3.0-beta-001)
• Operating system: Linux

[alfonsogarciacaro]

I've been thinking about this. Maybe we can have a mix of erased options (as it's been until now) with runtime options for things like Some (), nested options, etc. This would make the semantics of options in Fable more robust, the only drawback would be a bit more overhead for Option.isNone checks, which right now are just x == null.

[ncave]

@alfonsogarciacaro Looks like this issue is what's causing the error in repl when using netstandard2.0 assemblies.

[alfonsogarciacaro]

I just merged #1189 to enable nested options (but still erasing options in most cases), let's see how it works :)

[ncave]

Thanks @alfonsogarciacaro, unfortunately #1189 does not help, it does not seem to expose the option value properly in some cases (perhaps in deeply nested options). The compiled js has the option object instead of the option.value, if that makes sense. I'll try to extract a min repro if I can.

[alfonsogarciacaro]

Sorry! While checking your comment I noticed I forgot to update the replacements for the Option module 😅 so it's normal that in some cases the value is not properly obtained (now fable-core/Util/getValue must always be used to extract the value of an option). Give me a few hours (I cannot do it right now) and I'll fix it :)

[alfonsogarciacaro]

So commit number 2000 is an attempt to fix the issues created by #1189 with options :) Could you please check if this works now with FCS @ncave? Thanks a lot for your help!

[ncave]

@alfonsogarciacaro Congratulations on the 2K commit!

I'm getting Fable build errors though:

1294 passing (4s)
  1 failing

  1) TypeTests Types can instantiate their parent in the constructor:
     TypeError: Object(...) is not a function

[ncave]

@alfonsogarciacaro Also, I'm afraid the regression in #1193 has to be fixed too before the repl can be declared functional again. Sorry about dumping issues on you, hopefully it's just fable-core (ts) that needs to be fixed.

[alfonsogarciacaro]

Ups, you're too fast ;) I noticed after writing the comment the CI build was failing and amended the commit. Hopefully it should be fine now.

And don't apologize about issue reports, they're very helpful Also, I caused the regression myself so I have to fix it too ;)

[amsterdamharu]

First of all; Thank you for all the effort you put into this project. If ever I can help with anything I'd be glad to do so.

I get the same warning in 1.2.4 but not in 1.3.0.beta_002, the transpiled code however does not work.

Very long story short (tried to figure this out for many hours):

Browser.console.log(null)

Compiles to:

console.log(new __WEBPACK_IMPORTED_MODULE_1__nuget_packages_fable_core_1_3_0_beta_002_fable_core_Util__["f" /* Some */](null));

The type hint of log says:

abstract member Browser.Console.log : ?message:obj * [<System.ParamArray>] optionalParams:obj [] -> unit

I assume this comes from a typescript definition file so ?message:obj means nullable or optional? However when an optional parameter is wrapped in Some and passed to native JS the native JS will not work (like the openCursor function of IDBObjectStore) because Some(null) is an object, not null.

Missing parameters in JavaScript should be undefined since null !== undefined so passing null as a value for an optional parameter can break a lot of native JS or external libraries.

The description and the horror story of trying to reverse engineer how these dotnet tools work can be found here: https://stackoverflow.com/questions/46932417/how-to-build-and-run-fable-compiler-from-source

[alfonsogarciacaro]

@amsterdamharu I adapted quickly the IndexedDb bindings from FunScript but I didn't have a look afterward, probably they need some more work. In any case, maybe the issue is fixed with dotnet-fable 1.3.0-beta-003, could you please give it a try?

[amsterdamharu]

Not sure how to do this.

cloned https://github.com/fable-compiler/fable-powerpack

changed paket.dependencies: clitool dotnet-fable 1.3.0-beta-003

Then run:

[harm@localhost fable-powerpack]$ mono ./.paket/paket.exe update
[harm@localhost fable-powerpack]$ dotnet restore
[harm@localhost fable-powerpack]$ dotnet fable yarn-run pretest
No executable found matching command "dotnet-fable"

I don't know where Fable.Import.Browser comes from (maybe typescritp definintion? can't find any f# files). But if the type definition of Browser.Console.log is:

abstract member Browser.Console.log : ?message:obj * [<System.ParamArray>] optionalParams:obj [] -> unit

Then ?message is optional right?

So Browser.Console.log() should be console.log(undefined) or console.log() not console.log(null) and certainly not console.log(new Some(null))

[alfonsogarciacaro]

Ah, ok, I see what's the issue now. I was a bit misled by the reference to IndexedDB. Thanks a lot for the clarification! This is indeed wrong, I'll have a look :+1:

[alfonsogarciacaro]

I had a look and this is a very interesting case. First the Browser.Console.log : ?message:obj * [<System.ParamArray>] optionalParams:obj [] -> unit signature is wrong because optional arguments must go at the end. I don't understand why the F# compiler didn't forbid this, but in any case I just published a new Fable.Import.Browser version to fix that.

Also, an obj optional argument is a bit deceptive. If you have this:

type Test =
  static member Foo(?arg: obj) =
    printfn "arg %O" arg

When calling Test.Foo(None), the compiler will assume that you're actually passing a None value, if you want to omit the argument you should do Test.Foo(?arg=None)

Would that work for you? Could you post the exact code you're having problems with?

[amsterdamharu]

@alfonsogarciacaro

Thank you for your suggestion.

I'd love to try, is there an easy way to use newer packages?

If I delete paket.lock and run dotnet restore I just get an error but mono ./.paket/paket.exe update takes about half an hour every single time. This is getting to be a bit time consuming (don't mind spending time doing something but waiting for some slow tool to finish every time is getting irritating).

I'll try your suggestions with your powerpack repo.

[alfonsogarciacaro]

Hmmm, deleting paket.lock and running mono ./paket/paket.exe install is another way of getting the latest version of the packages but, though it's true Paket can be a bit slow resolving netstandard dependencies, mono ./paket/paket.exe update never took half an hour for me. Maybe @forki or @matthid can help here?

[forki]

@amsterdamharu can you please create an issue in paket tracker with your deps file?

[forki]

also which paket version`?

[amsterdamharu]

@forki

Thank you for your time.

Longest time was almost 30 minutes but on average it takes between 5 and 15 minutes. Not a good experience compared to npm and especially compared to yarn that seems to use cache better and has spoiled my expectations a bit.

It probably has to do with bad availability of the site from Cambodia (sites like Facebook and YouTube are fine with my Internet). The following does prevent it from failing and having to retry many times:

export PAKET_RESOLVER_TASK_TIMEOUT=-1

Here is some more info

Paket version 5.120.0

The paket.references is:

FSharp.Core
Fable.Core
Fable.Import.Browser
dotnet-fable

paket.dependencies:

source https://www.nuget.org/api/v2

nuget FSharp.Core
nuget Fable.Core prerelease
nuget Fable.Import.Browser

clitool dotnet-fable 1.2.0-beta-005

group Build
framework: net46

    source https://nuget.org/api/v2
    nuget FSharp.Core  redirects:force, content:none
    nuget FAKE

paket.lock: https://github.com/fable-compiler/fable-powerpack/blob/master/paket.lock

[forki]

Yes. But npm and yarn don't use nuget.org. I assume vanilla nuget is slow as well?

Am 01.11.2017 09:21 schrieb "Harm Meijjer" notifications@github.com:

@forki https://github.com/forki

Thank you for your time.

Longest time was almost 30 minutes but on average it takes between 5 and 15 minutes. Not a good experience compared to npm and especially compared to yarn that seems to use cache better and has spoiled my expectations a bit.

It probably has to do with bad availability of the site from Cambodia (sites like Facebook and YouTube are fine with my Internet). The following does prevent it from failing and having to retry many times:

export PAKET_RESOLVER_TASK_TIMEOUT=-1

Here is some more info

Paket version 5.120.0

The paket.references is:

FSharp.Core Fable.Core Fable.Import.Browser dotnet-fable

paket.dependencies:

source https://www.nuget.org/api/v2

nuget FSharp.Core nuget Fable.Core prerelease nuget Fable.Import.Browser

clitool dotnet-fable 1.2.0-beta-005

group Build framework: net46

source https://nuget.org/api/v2
nuget FSharp.Core  redirects:force, content:none
nuget FAKE

paket.lock: https://github.com/fable-compiler/fable-powerpack/blob/ master/paket.lock

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/fable-compiler/Fable/issues/1174#issuecomment-341031314, or mute the thread https://github.com/notifications/unsubscribe-auth/AADgNIaLK8qTlT4OTekSJbReU1Jzhf2zks5syCohgaJpZM4PqAC3 .

[amsterdamharu]

Here are the changes I made to IndexedDB.fs

let openCursor(index: Browser.IDBIndex, keyCursor: bool, range: Browser.IDBKeyRange option, direction: DBCursorDirection option, step: uint32 option) =
    //removed the following
    // let range = defaultArg range Unchecked.defaultof<Browser.IDBKeyRange>
    let direction = (defaultArg direction DBCursorDirection.Default).ToString()
    let step = defaultArg step 1u
    //changed request
    let request =
        match keyCursor with
        | false -> 
          match range with
          | Some range -> index.openCursor(range, direction=direction)
          | None -> index.openCursor(?range=None, direction=direction)
        | true -> 
          match range with
          | Some range -> index.openKeyCursor(range, direction=direction)
          | None -> index.openKeyCursor(?range=None, direction=direction)

Build with all the versions specified in the paket.lock file of the project and tested the indexedDB, no errors there.

The normal tests (dotnet fable yarn-run test) don't compile with the following errors:

ERROR in ./tests/PromiseTests.fs
/home/harm/dev/fable-powerpack/tests/PromiseTests.fs(252,104): (252,114) error FSHARP: Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved.
 @ ./tests/Main.fs 2:0-53
 @ ./tests/Tests.fsproj

ERROR in ./tests/PromiseTests.fs
/home/harm/dev/fable-powerpack/tests/PromiseTests.fs(253,118): (253,128) error FSHARP: Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved.
 @ ./tests/Main.fs 2:0-53
 @ ./tests/Tests.fsproj

Those can be solved in PromiseTest.fs:

let! r3 = failing |> Promise.either (fun x -> failwith "Shouldn't get called") (fun (ex:Exception) -> !^ex.Message)
let! r4 = failing |> Promise.either (fun x -> failwith "Shouldn't get called") (fun (ex:Exception) -> !^(Promise.lift ex.Message))

I'll try and create a pull request with the fixes.

[alfonsogarciacaro]

Closing as Fable supports nested options now :+1: