-
Notifications
You must be signed in to change notification settings - Fork 15
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
9 changed files
with
648 additions
and
87 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
{ | ||
"sdk": { | ||
"version": "6.0.100", | ||
"version": "8.0.300", | ||
"rollForward": "minor" | ||
} | ||
} |
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
98de8f6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@roboz0r I quickly glanced through your changes for Elmish 4, and as far as I can tell the changes look pretty solid. Is there a reason for it being WIP? What's missing? I'm planning to pull your changes and add support for Lit ContextProvider and ContextConsumer.
98de8f6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@juselius I think these changes were essentially fine however the other issue I was having with the library was the inline css styles didn't work. I wasn't getting errors they just weren't appearing at all. I couldn't understand what was going on with the css-related code at all and it seemed like the repo was otherwise inactive so I dropped what I was working on.
98de8f6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the heads up @roboz0r! I have a vague memory of having trouble with the inline css also in earlier versions, but I don't remember the details. I have grown very fond of Lit, and we'll try to breath new life into it now that Alfonso seems to have gone AWOL.
98de8f6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@juselius Just FYI, I identified the bug breaking styles over in the Fable compiler and trying to find a solution.
98de8f6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brilliant @roboz0r! Good work Sherlock :)
I have added basic support for Lit contexts in my Fable.Lit fork. The implementation is usable, but not entirely complete. I'll try to add the missing pieces during the weekend.
98de8f6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In debugging this I ended up producing a class based wrapper over Lit which I think I ultimately prefer as it follows the Lit docs much more closely.
In a brief reading of contexts it seems like that could become the storage for the state root for a Lit app serving a similar purpose to the Elmish style, maintaining an Elmish MVU pattern to these class based components would probably look more like
Elmish.WPF
where the View part is replaced by Bindings to a stateful component. I'm curious what you think98de8f6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good!
I think there is ample room for both approaches, depending on preferences. I came to React and Lit via Fable (my only real experience with JS is writing F# bindings). For a long time I thought that the Fable.Lit bindings closely followed the Lit API, because I never actually read the Lit docs...
What I like about the Fable.Lit bindings is that they closely mimic React, making it quite easy to convert between the two. In general, I prefer functional style over OO, as OOP has a much higher cognitive load on my brain.
I completed the Context implementation in Fable.Lit (not fully tested yet). We use contexts to store Elmish/Reducer models and dispatch functions, allowing us easily to issue updates between components in separate branches of the DOM tree. It's a nice way to manage complexity in large apps. Here is a nice reference: https://react.dev/learn/scaling-up-with-reducer-and-context.
98de8f6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I try to go with a functional first style. Wherever possible, write in terms of immutable data and pure functions but where mutable state is required, like with components, I prefer to switch to an OO style. That's a signal for me that I need to state thinking statefully rather than in terms of data flow.
I need to build something larger to get an idea how it works but I think making a distinction between:
Might be the sweet spot.
Reading the Reducer + Context docs is almost exactly what I had in mind, though I'd prefer to still pass in the context object rather than implicitly getting it via
useContext
. It's also similar to the react components andFeliz.UseElmish
hooks.98de8f6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that passing the context explicitly is clearer, but I justify the useContext() approach by thinking of it as a Reader monad, hehe.