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

Value Restriction and Type Signatures #3

Open
AngelMunoz opened this issue Apr 27, 2023 · 9 comments
Open

Value Restriction and Type Signatures #3

AngelMunoz opened this issue Apr 27, 2023 · 9 comments

Comments

@AngelMunoz
Copy link
Contributor

Looks like vide has been designed to be fully typed and safe this however is quite weird for type signatures from my perspective as a consumer

Having code like this

module Components.Card

open Fable.Core
open Vide
open type Html


[<Erase; Mangle false>]
type Card =
  static member Card
    (
      content: Vide<_, _, FableContext>,
      ?header: Vide<_, _, FableContext>,
      ?footer: Vide<_, _, FableContext>
    ) =
    vide {

      div.class' ("card") {

        match header with
        | Some header -> Html.header.class' ("card-header") { header }
        | None -> Vide.elseForget

        div.class' ("card-content") { content }

        match footer with
        | Some footer -> Html.footer.class' ("card-footer") { footer }
        | None -> Vide.elseForget
      }
    }

And then in using it as in the following code

module Components.Demo

open Fable.Core
open Vide
open type Html
open Components.Card
open type Card

let MyCount (count: int) = vide {
  div.class' ("my-count") { $"The current count is {count} :)" }
}

let AsyncComponent (counter: MutableValue<int>) = vide {
  return MyCount counter.Value

  do! async {
    do! Async.Sleep 500
    counter := 42
  }

  return MyCount counter.Value

  do! Promise.sleep 1000 |> Async.AwaitPromise
  counter := 100
  return MyCount counter.Value
}

let cards = vide {
  let! currentCount = Vide.ofMutable 0

  Card(
    vide {
      let! result = AsyncComponent currentCount

      p {
        $"Result is: "
        result
      }
    },
    header = vide { "Async Component" },
    footer = vide { "This is the footer" }
  )

  Card(
    vide { "This is just another Usage" },
    vide {
      section {
        h1 { "This is a section" }
        p { "This is a paragraph" }
      }

      section {
        h1 { "This is another section" }
        p { "This is another paragraph" }
      }
    }
  )
}

let view = vide { article.class' ("app") { main { cards } } }

Produces a compilation error:

error FSHARP: Value restriction. The value 'view' has been inferred to have generic type
./src/Components/Demo.fs(60,5): (60,9) error FSHARP: Value restriction. The value 'view' has been inferred to have generic type
    val view: Vide<unit,NodeBuilderState<Browser.Types.HTMLElement,NodeBuilderState<Browser.Types.HTMLElement,(MutableValue<int> option * (NodeBuilderState<Browser.Types.HTMLDivElement,(NodeBuilderState<Browser.Types.HTMLElement,TextNodeProxy<Browser.Types.Node>> option * (NodeBuilderState<Browser.Types.HTMLDivElement,((unit option * AsyncState<unit> option * (unit option * AsyncState<unit> option * unit option) option) option * NodeBuilderState<Browser.Types.HTMLParagraphElement,(TextNodeProxy<Browser.Types.Node> option * NodeBuilderState<Browser.Types.HTMLDivElement,TextNodeProxy<Browser.Types.Node>> option)> option)> option * NodeBuilderState<Browser.Types.HTMLElement,TextNodeProxy<Browser.Types.Node>> option) option)> option * NodeBuilderState<Browser.Types.HTMLDivElement,(NodeBuilderState<Browser.Types.HTMLElement,(NodeBuilderState<Browser.Types.HTMLElement,(NodeBuilderState<Browser.Types.HTMLHeadingElement,TextNodeProxy<Browser.Types.Node>> option * NodeBuilderState<Browser.Types.HTMLParagraphElement,TextNodeProxy<Browser.Types.Node>> option)> option * NodeBuilderState<Browser.Types.HTMLElement,(NodeBuilderState<Browser.Types.HTMLHeadingElement,TextNodeProxy<Browser.Types.Node>> option * NodeBuilderState<Browser.Types.HTMLParagraphElement,TextNodeProxy<Browser.Types.Node>> option)> option)> option * (NodeBuilderState<Browser.Types.HTMLDivElement,TextNodeProxy<Browser.Types.Node>> option * NodeBuilderState<Browser.Types.HTMLElement,obj> option) option)> option) option)>>,FableContext>    
Either define 'view' as a simple data term, make it a function with explicit arguments or, if you do not intend for it to be generic, add a type annotation. (code 30)

Weirdly enough, even when the editor and fable complain about it, fable actually compiles, I'm not sure if that's a bug somewhere but at least it lets me continue.

I noticed that these type signatures change a lot depending on what are you using inside the vide {} blocks, in the Card example I had to add those type signatures otherwise the compiler was assuming the header and the footer were unit type.

even if I add Vide<unit, _, FableContext> as a type annotation the error still shows

I have to provide the type signature `let cards: ...`
Vide<unit,(MutableValue<int> option * (NodeBuilderState<Browser.Types.HTMLDivElement,(NodeBuilderState<Browser.Types.HTMLElement,TextNodeProxy<Browser.Types.Node>> option * (NodeBuilderState<Browser.Types.HTMLDivElement,((unit option * AsyncState<unit> option * (unit option * AsyncState<unit> option * unit option) option) option * NodeBuilderState<Browser.Types.HTMLParagraphElement,(TextNodeProxy<Browser.Types.Node> option * NodeBuilderState<Browser.Types.HTMLDivElement,TextNodeProxy<Browser.Types.Node>> option)> option)> option * NodeBuilderState<Browser.Types.HTMLElement,TextNodeProxy<Browser.Types.Node>> option) option)> option * NodeBuilderState<Browser.Types.HTMLDivElement,(NodeBuilderState<Browser.Types.HTMLElement,(NodeBuilderState<Browser.Types.HTMLElement,(NodeBuilderState<Browser.Types.HTMLHeadingElement,TextNodeProxy<Browser.Types.Node>> option * NodeBuilderState<Browser.Types.HTMLParagraphElement,TextNodeProxy<Browser.Types.Node>> option)> option * NodeBuilderState<Browser.Types.HTMLElement,(NodeBuilderState<Browser.Types.HTMLHeadingElement,TextNodeProxy<Browser.Types.Node>> option * NodeBuilderState<Browser.Types.HTMLParagraphElement,TextNodeProxy<Browser.Types.Node>> option)> option)> option * (NodeBuilderState<Browser.Types.HTMLDivElement,TextNodeProxy<Browser.Types.Node>> option * NodeBuilderState<Browser.Types.HTMLElement,obj> option) option)> option) option),FableContext>

And then only then the compiler is happy, I'm not sure if this is actually addressable or there is a way to use simpler types from the consuming side.

Note: Hey there! I saw your tw message, don't feel rushed to answer these I can wait for sure; I'll keep digging and see what I can find!

@SchlenkR
Copy link
Owner

SchlenkR commented Apr 27, 2023

That's an interesting issue - thanks for providing.

Type Signatures (shown in editor / compiler messages)

The resulting type signature of a vide CE reflects it's compile time structure. The variant parts are 2 type parameters: 1 for the return value, and another one for the state. The way in which those functions are composed results in a state type that nests the state for one function and the state for another functions to compose (and so on!). This is basically explained here. Vide differs from the experiments I made before, because it relies more on side effects. So in early prototypes, I tried to hide the nested type signatures for the price of using a little bit of reflection and losing compile time safety (which seemed to be acceptable), but I ran into many problems, and I decided to discontinue working on that idea (maybe in a dotnet runtime, I could be possible, but not in Fable due to a lack of reflection capabilities). In the end, it's a tooling decision: "How to print type signatures?"

Value restriction / explicit type signatures

There's a primary requirement that has to be met: Everything must compile without specifying the Vide types parameters (return value, state, context) explicitly. The most specific type annotation that could be needed is Vide<_,_,_>. There were many of these issues coming up during development, and it was usually a trade-off between features and getting rid of value restriction issues.

This topic definitely deserves a doc section in form of: "You do this?`Try this. You do that? Try that"... So more of a guidance with recipes, providing quick solutions for known edge cases, because in my opinion, a developer should not be forced do dig in so deeply (they propably would just not use it).

So first: What's the problem?

I narrowed down your example:

open Vide
open type Vide.Html

let Card
    (
        content: Vide<_,_,_>,
        headerContent: Vide<_,_,_> option
    ) =
    vide {
        match headerContent with
        | Some headerContent -> header { headerContent }
        | None -> Vide.elseForget

        div.class' ("card-content") { content }
    }

let cards = vide {
    Card(
        vide {
            let! counter = Vide.ofMutable 0
            $"The current count is {counter.Value} :)"
        },
        Some (vide { "Header is here" })
    )

    Card(
        vide { "This is just another Usage" },
        None
    )
}

let view = vide { article { main { cards } } }

Remember: A Vide function looks like: type Vide<'returnValue, 'state, 'context> = ...

The type signature of the Card component is infered by the compiler and looks like this:

grafik

For all involved Vide functions, the compiler has infered the type FableContext for context. This specialization is done on purpose in the "Vide.Fable" part, because it resolves a lot of potential value restriction issues (but also closed the door for some cool features I had in mind - but ok). The compiler also generalizes 'returnValue and 'state for "content", as well as 'returnValue for "footerContent". 'state for "footerContent" is infered to type unit, because Vide.elseForget can only provide the unit instance. Side note: The 'returnValue for "content" is generalized because "content" is yielded in a div, and the yield overload for Vide functions implicitly ignores their return values.

The question from above (still unanswered) was: What's the problem? In other words: Which type parameter cannot be infered, and why?

When no footer is supplied (it's optional), the generalized type parameter for the footer's state remains open - and it will remain open forever, and that's a problem because no default type exists that can instanciate that type parameter.

Here's a compiling workaround that instanciates the 'state type parameter with unit.

let Card<'vc,'sc,'sf>
    (
        content: Vide<'vc,'sc,_>,
        footerContent: Vide<_,'sf,_> option
    ) =
    vide {
        div { content }
        
        match footerContent with
        | Some footerContent -> header { footerContent }
        | None -> Vide.elseForget
    }


let cards = vide {
    Card(
        vide {
            let! counter = Vide.ofMutable 0
            $"The current count is {counter.Value} :)"
        },
        Some (vide { "Footer is here" })
    )

    Card<_,_,unit>(
        vide { "This is just another Usage" },
        None
    )
}

let view = vide { article { main { cards } } }

A final solution in form of documentation or technical solution - I currently don't know. The procedure shown in the workaround is nothing that is expected to be done by the programmer - that's for sure.

So, now I have a good reason for postponing writing docs; this issue has prio :)

Thank you for this feedback, and have a good night... Hear you!

@AngelMunoz
Copy link
Contributor Author

This is interesting, I find these mechanisms quite neat, although I'm still confused, I made it work with the workaround you provided.

If you don't mind in the future, maybe we can have a session so I can learn more about the framework I'd be glad to write docs if you need help in that area I'm liking it so far!

@SchlenkR
Copy link
Owner

Here's another "workaround" - maybe closer to a solution by turning the component into a function. This then must be respected in every place us usage, meaning: Also turning these in a function (like let view() = ...

open Vide
open type Vide.Html

let Card
    (
        content: Vide<_,_,_>,
        footerContent: Vide<_,_,_> option
    ) =
    vide {
        div { content }
        
        match footerContent with
        | Some footerContent -> header { footerContent }
        | None -> Vide.elseForget
    }


let cards() = vide {
    Card(
        vide {
            let! counter = Vide.ofMutable 0
            $"The current count is {counter.Value} :)"
        },
        Some (vide { "Footer is here" })
    )

    Card(
        vide { "This is just another Usage" },
        None
    )
}

let view() = vide { article { main { cards() } } }

Commit is here

https://github.com/RonaldSchlenker/Vide/blob/e2b16d6a925d4f20dd4af192b182decfb2454e61/Vide.Fable/src/DevApp/src/UseCases/Issue_2.fs

If you don't mind in the future, maybe we can have a session so I can learn more about the framework I'd be glad to write docs if you need help in that area I'm liking it so far!

Absolutely! That would be really great. Are you in the F# slack?

@SchlenkR
Copy link
Owner

another idea:

open Vide
open type Vide.Html

let Card
    (
        content: Vide<_,_,_>,
        footerContent: Vide<_,_,_> option
    ) =
    vide {
        div { content }
        
        match footerContent with
        | Some footerContent -> header { footerContent }
        | None -> Vide.elseForget
    }

let videNone : Vide<unit,unit,_> option = None

let cards = vide {
    Card(
        vide {
            let! counter = Vide.ofMutable 0
            $"The current count is {counter.Value} :)"
        },
        Some (vide { "Footer is here" })
    )

    Card(
        vide { "This is just another Usage" },
        videNone
    )
}

let view = vide { article { main { cards } } }

Still, videNone has to be provided by the caller, meaning functions with optional parameters won't work in any case.

SchlenkR added a commit that referenced this issue Apr 28, 2023
SchlenkR added a commit that referenced this issue Apr 28, 2023
SchlenkR added a commit that referenced this issue Apr 28, 2023
@AngelMunoz
Copy link
Contributor Author

Still, videNone has to be provided by the caller, meaning functions with optional parameters won't work in any case.

That's a shame, I think there would be some really good benefits to static methods with optional parameters to offer a nicer API for consuimers e.g like the card example where you can either have just the body or provide header/footer/media and you wouldn't be tied to provide None, None, None, None

@AngelMunoz
Copy link
Contributor Author

Absolutely! That would be really great. Are you in the F# slack?

Yeah! feel free to ping me there


I'll keep exploring and try the function calls approach, I think that works a little bit closer to what I'd like to use!

@AngelMunoz
Copy link
Contributor Author

AngelMunoz commented Apr 28, 2023

Following up with this, if you add more than one "slot" and try to bring for example both header and footer, looks like a case of "first one wins" so following structures have to match, I think I found a way around this by taking functions in the slots rather than plain Vide values

type Card =
  static member Card
    (
      // it was super important to make this a function
      // otherwise the header would "always win" and the footer would error out
      content: (unit -> Vide<_, _, _>),
      ?header: (unit -> Vide<_, _, _>),
      ?footer: (unit -> Vide<_, _, _>)
    ) =
    vide {
      div {
        match header with
        | Some header -> Html.header{ header () }
        | None -> Vide.elseForget
        content()
        match footer with
        | Some footer -> Html.footer { footer () }
        | None -> Vide.elseForget
      }
    }
// it may be a little funky, but I think this is tolerable to work with and may even 
// promote a style which has bettter composition (if you're already taking functions, why not keep it there?)
Card(
  (fun () -> vide {
    p {
      "Lorem ipsum dolor sit amet."
    }
  }),
  header = (fun () -> vide { header { h3 { "Header" } } }),
  footer = (fun () -> vide { footer { a.href ("#") { "Read more]" } } })
)

For the moment I'm just building a toy website so this is where I'm finding these observations

Note: Here's my current repo perhaps this will help to shed light on what I'm currently doing: https://github.com/AngelMunoz/VideOMDbSample

@SchlenkR
Copy link
Owner

SchlenkR commented May 2, 2023

I propably misunderstood your point - or I still don't understand it.

try to bring for example both header and footer, looks like a case of "first one wins" so following structures have to match

So this does not refer to the value restriction issue, correct? Then I don't understand the issue. Having this code:

    open Vide
    open type Vide.Html

    type Card =
        static member Card
            (
                // it was super important to make this a function
                // otherwise the header would "always win" and the footer would error out
                content: Vide<_, _, _>,
                ?header: Vide<_, _, _>,
                ?footer: Vide<_, _, _>
            ) =
            vide {
                div {
                    match header with
                    | Some header -> header
                    | None -> Vide.elseForget
                    
                    content
                    
                    match footer with
                    | Some footer -> footer
                    | None -> Vide.elseForget
                }
            }

    // it may be a little funky, but I think this is tolerable to work with and may even 
    // promote a style which has bettter composition (if you're already taking functions, why not keep it there?)
    let view = 
        Card.Card(
            vide { 
                p { "Lorem ipsum dolor sit amet." } 
            },
            header = vide {
                let! listState = Vide.ofMutable [1;2;3]
                p { $"Header with elements: {listState.Value}" } 
            },
            footer = vide { 
                let! intState = Vide.ofMutable 1
                p { $"Footer with number: {intState.Value}" } 
            }
        )

The provided header and footer have different structure of state (header with int list and footer with just an int). This is valid as long as they are not used in the same if/else or match expression.

The resulting rendered page is the one I would expect:

grafik

It's not clear to me what benefit the wrapping in a function has.

@SchlenkR SchlenkR changed the title Feedback on Type Signatures Value Restriction and Type Signatures May 8, 2023
@SchlenkR
Copy link
Owner

SchlenkR commented May 8, 2023

There's now the branch VideAsFunctionAndNotSingleCaseDU that might help solving value restriction issues in the future.

Removing the single case DU for the Vide function might also allow for the InlineIfLambda performance boost.

My initial thought on this was that removing the SCDU was much harder (also due to heavy use of builder method overload and the associated overload resolution), but that worked very well. As a drawback, we are confronted with even more obscure type annotations - but they are obscure and useless anyway (except when it comes to debugging the library; but I can live with that). On the other hand I thought that the value restriction issue will automatically disappear - which is unfortunately not the case.

I have to investigate this issue further.

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

2 participants