fable-compiler / Fable.Lit

Write Fable Elmish apps with Lit
https://fable.io/Fable.Lit/
MIT License
91 stars 13 forks source link

Lit Controllers don't work with Fable.Lit WC #26

Open AngelMunoz opened 2 years ago

AngelMunoz commented 2 years ago

If you add a "native" Lit component and a controller inside a Fable.Lit application they work just fine: https://github.com/AngelMunoz/ControllersRepro/blob/master/src/Controllers/controllers.js https://github.com/AngelMunoz/ControllersRepro/blob/master/src/Components/Navbar.fs#L51

but if you want to add the same controller to a Fable.Lit Web component, every time a render happens the controller gets added time and time again to the controller's list I made a reproduction repository to check it out https://github.com/AngelMunoz/ControllersRepro

Although we may not author controllers from Fable.Lit there will be some packages in the wild that may be useful to consume

AngelMunoz commented 2 years ago

The usual way a controller is added to a LitElement is like this

class SampleController {
  // where host is the LitElement (or any other ReactiveHost)
  constructor(host) {
    this.host = host;
    host.addController(host)
  }
}

the main reason it's done like that is because controllers tend to have some power on when a host should update e.g.

sample() {
  this.state = "new state"
  this.host.requestUpdate()
}

I'm not sure if we may need a let ctrl = Hook.useController(host -> new Controller(host)) or something like that or if we want to support them at all.

Controllers actually may serve a similar purpose that of hooks although they are more of a "mutable bag" (if I can put it that way, i.e. self-contained state) but that's another conversation

alfonsogarciacaro commented 2 years ago

I did try to make controllers compatible with HookComponent (well, it was the animate directive but it uses a controller under the hood) but I was unsuccessful because controllers require a more fine grained control of the host lifecycle than allowed by Async directives in which HookComponents are based. So I wouldn't add Hook.useController because controllers will likely require lit elements.

Adding a controller in the render function results in the controller being added multiple times as expected. I assume it should be possible to use Hook.useEffectOnce to add the controller only once. But we can also try to provide a more specific way to add the controller. As in your example, this is usually done in the constructor function in Lit (not sure if there are occasions when you want to add/remove controllers during the lifecycle of the host). In Fable.Lit the equivalent would be the LitElement.init function. The problem is the way it's designed we cannot pass the host as argument, so we would need to have an init function within the init function. Looks a bit redundant though, any ideas?

LitElement.init (fun config ->
    config.props <- ...
    config.styles <- ...
    config.initHost(fun host ->
        host.addController(host)
    )
)
AngelMunoz commented 2 years ago

It is very unfortunate then :(

I gave it a shot during the day but I was unable to get something working, it didn't occur to me about using Hook.useEffectOnce, but after a couple of tries (thanks) I got this

    let host = LitElement.init ()

    Hook.useEffectOnce (fun () -> host?mouseCtrl <- createMouseCtrl host)
    let x, y =
        match host?mouseCtrl with
        | Some ctrl -> ctrl?x, ctrl?y 
        | None -> 0, 0

In the end the host is still a LitElement and we should be able to use it for our purposes I think although, I can't come up with clean alternatives 😭 however, there has to be a cleaner and type safe way to do it.

alfonsogarciacaro commented 2 years ago

Ah, true! You need to keep a reference to the controller so "just initializing" them won't work 😸 Ok, I gave it a quick try at declaring controllers in LitElement.init in a similar fashion to props. It seems to work, have a look and let me know what you think. The only issue is it will be a small breaking change: https://github.com/fable-compiler/Fable.Lit/pull/27/files#diff-6ae8b2234a0adbee56cc2b42f1e9f31422b8c45ca089818404a404bccec8952c

screencast

OnurGumus commented 1 year ago

Any update on this?