Fable.Lit icon indicating copy to clipboard operation
Fable.Lit copied to clipboard

Lit Controllers don't work with Fable.Lit WC

Open AngelMunoz opened this issue 3 years ago • 5 comments

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 avatar Dec 02 '21 18:12 AngelMunoz

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

AngelMunoz avatar Dec 02 '21 18:12 AngelMunoz

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)
    )
)

alfonsogarciacaro avatar Dec 03 '21 02:12 alfonsogarciacaro

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.

AngelMunoz avatar Dec 03 '21 03:12 AngelMunoz

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

alfonsogarciacaro avatar Dec 03 '21 05:12 alfonsogarciacaro

Any update on this?

OnurGumus avatar Jan 14 '23 05:01 OnurGumus