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

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

Open
AngelMunoz opened this issue Dec 2, 2021 · 5 comments
Open

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

AngelMunoz opened this issue Dec 2, 2021 · 5 comments

Comments

@AngelMunoz
Copy link
Collaborator

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
Copy link
Collaborator Author

AngelMunoz commented Dec 2, 2021

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
Copy link
Member

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
Copy link
Collaborator Author

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
Copy link
Member

alfonsogarciacaro commented Dec 3, 2021

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
Copy link

Any update on this?

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

3 participants