-
Notifications
You must be signed in to change notification settings - Fork 4
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
Move this one over to FL #2
Comments
Definitely. It would need some work getting it up to date with the latest versions of the spec. Beyond that it needs some effort spent on documentation and optimising for performance. |
My lens library was originally based on FL'ish implementation, but it didn't quite work for reasons that you have also encountered. Sometimes a type class constrained type appears only at a covariant position, so a naïve type classes only as object methods translation doesn't work—you have to pass an extra argument of some sort. I haven't really looked at FL after realizing this. I switched to SL, which can basically be seen as the same dictionary translation that is described in the early papers on type classes. Aside from just being a simple working solution to translating type class based code to JS, SL also doesn't require wrapping values with object wrappers. This tends to translate to a major performance advantage. So, it would be interesting to see a carefully optimized version of FL based optics to see how large/small the performance hit really is. My expectation is that the extra allocations for FL wrappers will kill performance (order of magnitude), but it would be nice to be proven wrong as the JS world seems to be determined to go with FL. Even something simple like viewing a nested property seems to require an extra allocation to wrap the constant result. More complex things like folding over a traversal, say to compute a sum of numbers, seems to require an extra allocation per element. |
Might be a bit hold, but would you take any thoughts of putting this into FL directly and superseding the current implementation? If not, nps worth asking!
The text was updated successfully, but these errors were encountered: