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

Experimental class bindings #41

Open
wants to merge 41 commits into
base: controllers
Choose a base branch
from

Conversation

AngelMunoz
Copy link
Collaborator

This is the class bindings PR

these are the first bits but I want to iterate over this and see if there's a way we can also apply some of these concepts to the LitElement decorated functions

I'm still missing some bindings like Directives and AsyncDirectives (they just have what they need now, but ideally they should be fully typed)

registerElement "element-with-controller" JsInterop.jsConstructor<ClassComponents.ElementWithController> {
css $"p {{ color: red; }}"
css $"li {{ color: rebeccapurple; }}"
Copy link
Member

@alfonsogarciacaro alfonsogarciacaro May 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit concerned that the class and the registration (with the props, styles...) are so far away and the compiler cannot tell you when you misspell a prop name. But let's see how it goes (it's true the compiler doesn't check the prop names in the html templates either).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is too far fetched from what current lit users do, but I share this concern arguably I think it would be nice if we stick slightly more close to lit's semantics so people don't have to learn Fable.Lit + Lit

ReactiveController, ReactiveControllerHost
this should give anyone the ability to create full class directives if they need it
Copy link
Collaborator Author

@AngelMunoz AngelMunoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alfonsogarciacaro I know you have a quite busy agenda but if any of this makes sense or not feel free to let me know when you have time, I really want to get a deeper understanding of this codebase so I can keep taking it further away

sample/App.fs Outdated
Comment on lines 76 to 81

let state: StateController<int> =
Controllers.GetController<StateController<int>>(10)

let twoArgs = Controllers.GetController<TwoArgsCtrl>(10, 20)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking on something like this to prevent having the LitElement generic initialization, we still do magic, but more javascrip'ty magic (or that's what I want to believe) in the end these are just properties that are in the class that is being used behind this function to use lit's mechanisms without diving too much into directive territory

Side note perhaps Context would be a better name rather than Controllers

registerElement "element-with-controller" JsInterop.jsConstructor<ClassComponents.ElementWithController> {
css $"p {{ color: red; }}"
css $"li {{ color: rebeccapurple; }}"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is too far fetched from what current lit users do, but I share this concern arguably I think it would be nice if we stick slightly more close to lit's semantics so people don't have to learn Fable.Lit + Lit

@@ -0,0 +1,76 @@
[<Experimental("This is an exploration space, anything can be added or removed in this module at any point")>]
module Lit.Experimental
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to learn how you did actually do the Decoration stuff as well as hot reload so any experiments and ideas I have in mind I will put them right here, hope you don't mind and also if you see things we can improve and take out to the real world stuff it would be nice

Comment on lines +10 to +18
type StateController<'T>(host, initial: 'T) =
inherit ReactiveController(host)
let mutable state = initial

member _.Value = state

member _.SetState(value: 'T) =
state <- value
host.requestUpdate ()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm playing with the idea of having hook-like abstractions but rather than backed by a directive backed by ReactiveControllers they would work nicely for both class function based LitElements, too crazy perhaps?

@AngelMunoz AngelMunoz marked this pull request as ready for review September 14, 2022 16:39
@AngelMunoz
Copy link
Collaborator Author

Hey @alfonsogarciacaro I don't really have more to add to this PR I think it has been idle for a while but the important bits are there.

I added the useController so users that are using function based lit elements can also grab controllers that may already exist in the wild but ideally the controllers match better with the class based API which mirrors what actual lit users do and might be simpler for some to copy or port existing components
I will be adding docs for these today and then feel free to review

@goswinr
Copy link

goswinr commented Nov 17, 2023

@alfonsogarciacaro thanks you for making Fable .Lit 🙏 I am just learning Lit and wondered why I can't do this example in Fable.Lit: https://lit.dev/docs/composition/controllers/#example:-mousemovecontroller
I found the answer here in this pull request. Thanks, @AngelMunoz. Any reason not to merge this ? At least to have the official samples working in Fable.Lit too?

@AngelMunoz
Copy link
Collaborator Author

No particular reason I never got to finish this as I got distracted.

I'd like more feedback on this as well if you want to try these bits we could revisit the work and make the appropriate updates if needed

Cc: @JordanMarr , @OnurGumus

Sorry to bother I've seen you are using this as on Twitter as well I guess this could be of interest

@OnurGumus
Copy link

I think the current approach is fine. One can just use the controllers the way you do. So far I haven't seen any need for this PR.

@JordanMarr
Copy link
Contributor

Sorry, all my free coding time has been consumed with the Elmish.Avalonia project (and my Fable.Lit project got iced).
TBH, I don't even know what this PR is about as there are no pretty pictures of code look at.
Based on Onur's comment, I'm assuming that you created bindings for Lit Controllers?

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

Successfully merging this pull request may close these issues.

7 participants