fabulous-dev / Fabulous

Declarative UI framework for cross-platform mobile & desktop apps, using MVU and F# functional programming
https://fabulous.dev
Apache License 2.0
1.14k stars 121 forks source link

Add CreateCode to allow customizing type instantiation #837

Closed Dolfik1 closed 3 years ago

Dolfik1 commented 3 years ago

Continuing the discussion, I added parameter to code generator which allows to instantiate types with parameters.

image image image

There are two unresolved issues at the moment:

  1. We should always add constructorParameters key to each type (it cannot be inherited from the base type);
  2. We can not use it with Fabulous since Fabulous does not support custom parameters.

image

@TimLariviere you said in previous discussion about optional obj parameter in Create method. But as you can see on the screen there are unit -> 'T0 function only.

Dolfik1 commented 3 years ago

I think that this way to pass custom parameters to view`s constructor is wrong. I found out that in case of RecyclerView we need to pass some parameters from Fabulous DSL: RecyclerView recyclerView = new RecyclerView(new ContextThemeWrapper(context, R.style.ScrollbarRecyclerView)); (https://stackoverflow.com/questions/3589929/how-to-set-androidscrollbars-vertical-programmatically)

In this case we need to pass R.style.ScrollbarRecyclerView from Fabulous DSL.

View.RecyclerView(
  theme = R.style.ScrollbarRecyclerView)

member x.Create(context: Android.Content.Context) =
  match theme with
  | Some theme ->
    new RecyclerView(new ContextThemeWrapper(context, theme))
  | _ ->
    new RecyclerView(context)
Dolfik1 commented 3 years ago

I added InstantiateType key to code generator. This allows you to set expression to instantiate the type.

/// The expression to instantiate type ({0} can be used to pass type name in expression)
member val InstantiateType: string option = None with get, set

A few examples:

"instantiateType": "new {0}(context)"
"instantiateType": "ViewHelpers.instantiateRecyclerView context theme"

This is not complete solution because we can not specify parameters in Create function. I think it should be root property in json config like outputNamespace or assemblies.

TimLariviere commented 3 years ago

@Dolfik1 Like last time, I did not have time to read thoroughly your changes. But I might have a few pointers that can help you:

If you need to customize the instantiation of a type (like with RecyclerView), you may be able to do like the updateCode attribute in the JSON file.

https://github.com/fsprojects/Fabulous/blob/984ead5d9bd200f3de143adbcfba2ee79992b285/Fabulous.XamarinForms/src/Fabulous.XamarinForms/Xamarin.Forms.Core.json#L1942

In the CodeGenerator step, CodeGen will check if the attribute is defined. If it's the case, it will simply call that function. Otherwise it will generate a "default" update code. I think you can reuse this logic for your instantiateType (createCode?) to provide your own implementation of the instantiation of a type.

And in the case you don't have defined createCode, you can make CodeGenerator to output a default new %s(context). Otherwise it would output %s viewElement additionalCtorData where %s is the function defined in the JSON file that would look like.

let createRecyclerView viewElement additionalCtorData =
    let context = match additionalCtorData with ?: AndroidCtorData as d -> d.Context | ...
    let theme = viewElement.TryGetAttributeKeyed(ViewAttributes.RecyclerViewTheme...)
    new RecyclerView(new ContextWrapper(context, theme))

Here's the place updateCode is used: https://github.com/fsprojects/Fabulous/blob/984ead5d9bd200f3de143adbcfba2ee79992b285/Fabulous.CodeGen/src/Fabulous.CodeGen/Generator/CodeGenerator.fs#L160-L204

The advantage of that is you don't need to specify anything for the majority of the controls.

Dolfik1 commented 3 years ago

I renamed InstantiateType to CreateCode.

At the moment we are forced to add the property CreateCode to each type. I think the best solution is add InheritCreateCode boolean property. This will allow reuse CreateCode in types inherited from base type.

TimLariviere commented 3 years ago

Read all the code, looks great!

At the moment we are forced to add the property CreateCode to each type. I think the best solution is add InheritCreateCode boolean property. This will allow reuse CreateCode in types inherited from base type.

The way I imagined it, CreateCode (just like UpdateCode) is really specific to a type and is there to override the generated code when the CodeGenerator step can't do it by itself.

If something needs to be applied to many types, I would redefine the part that generate Create%s() method inside CodeGenerator through the Program record so it can generate the correct code without having to set it in the mapping file. (might need to be a little more flexible in the Program record)

For example:

let result =
    Program.mkProgram
        Reflection.loadAllAssemblies
        Reflection.tryGetProperty
        configuration
    (...)
    |> Program.withGeneratorConfiguration (fun configuration ->
        { configuration with
             generateCreateMethod = FabulousAndroidGenerators.generateCreateMethod }
    )
    |> Program.run options.MappingFile options.OutputFile

module FabulousAndroidGenerators

let generateCreateMethod (data: CreateData option) (w: StringWriter) =
    match data with
    | None -> w
    | Some data ->
        w.printfn "    static member Create%s (context: Android.Context) : %s =" data.Name data.FullName

        if data.TypeToInstantiate = data.FullName then
            w.printfn "        %s(context)" data.TypeToInstantiate
        else
            w.printfn "        upcast (%s(context))" data.TypeToInstantiate

        w.printfn ""
        w

This will make Fabulous.CodeGen call your FabulousAndroidGenerators.generateCreateMethod instead of the default CodeGenerator.generateCreateFunction.

So the way you wrote it now looks OK for me.

/// The expression to instantiate type ({0} can be used to pass type name in expression)

Continuing from what I was saying just now, I wouldn't even allow {0} in CreateCode since the creation code should be really last resort for when CodeGenerator can't do it by itself.

If you're ok with it, I think we can merge what you did so far (after fixing the unit tests, and removing {0}). We will continue the initial idea of adding ConstructorParameters in another PR.

Dolfik1 commented 3 years ago

If you're ok with it, I think we can merge what you did so far (after fixing the unit tests, and removing {0}). We will continue the initial idea of adding ConstructorParameters in another PR.

Thank you! I think we do not need {0} in CreateCode if we add generateCreateMethod. I will fix it later and make new MR with generateCreateMethod.