-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
RFC: Use concrete types instead of generic type params? #87
Comments
To make it concrete, it looks like this: It's worth pointing out that although those may look like entirely different monadified types, secretly they are just opaque type aliases for the respective "raw" JS type. So if I'm reading correctly, am I currently the only known user of this capability? 😅 Unless some other |
If the scala-js-dom versions just extend the abstract traits and override every method with a call to |
It appears so with my limited knowledge, but I can't be sure, and I don't know if I'm ready to give up on the promise of flexibility just yet, both for the IO crowd and the backend tasks. Like if someone wanted to make a static site generator in Scala, they should be able to use SDT (although if I personally did it, I would probably run it on Laminar in node.js)
Right, I forgot to mention that approach. ScalaTags does it this way (possibly for different reasons). In principle, both option (1) and option (2) could use such inheritance – just add |
Meh, turns out to be moot. |
Thanks for the cc @raquo - interesting discussion!
😂 If it's any consolation, he said that to me enough times that I put them in. 😉 I'm only on my first coffee of the day but I'm not totally following your point about the design being done the way it has been to allow usage on the JVM...? Tyrian's tags all work just fine on the JVM so that you can use them for SSR.... but I feel like I'm missing something? Perhaps for generating funky email content? Anyway, I'm a massive fan of code generation these days, so I thought I might offer some thoughts on the disadvantages you called out based on my experience, but for the record I have no skin in this game so feel free to ignore me. I seem to recall that I looked at using scala-dom-types for Tyrian because having "one Scala implementation" is indeed appealing, but the lifecycle differences between Laminar/Outwatch/Calico (FRP) vs Tyrian (Elm) are sufficiently different that it didn't feel like a good fit for me at the time.
For performance and bundle size (I'm know you know, but for the casual reader) this makes no odds because Scala.js will strip all that stuff out, and on the JVM... what's a few KB between friends? So then it's just that every tag perhaps has a bulky definition to the reader, here is Tyrian's output for def div[M](attributes: Attr[M]*)(children: Elem[M]*): Html[M] =
Tag("div", attributes.toList, children.toList)
@targetName("div-list-repeated")
def div[M](attributes: List[Attr[M]])(children: Elem[M]*): Html[M] =
Tag("div", attributes, children.toList)
@targetName("div-no_attrs-repeated")
def div[M](children: Elem[M]*): Html[M] =
Tag("div", Nil, children.toList)
@targetName("div-repeated-list")
def div[M](attributes: Attr[M]*)(children: List[Elem[M]]): Html[M] =
Tag("div", attributes.toList, children)
@targetName("div-list-list")
def div[M](attributes: List[Attr[M]])(children: List[Elem[M]]): Html[M] =
Tag("div", attributes, children)
@targetName("div-no_attrs-list")
def div[M](children: List[Elem[M]]): Html[M] =
Tag("div", Nil, children)
@targetName("div-repeated-list-plaintext")
def div[M](attributes: Attr[M]*)(plainText: String): Html[M] =
Tag("div", attributes.toList, List(text(plainText)))
@targetName("div-list-list-plaintext")
def div[M](attributes: List[Attr[M]])(plainText: String): Html[M] =
Tag("div", attributes, List(text(plainText)))
@targetName("div-no_attrs-list-plaintext")
def div[M](plainText: String): Html[M] =
Tag("div", Nil, List(text(plainText))) All of those are really just alternate convenience constructors to make the syntax nicer for the user. It is a little noisy, but it is also so maintainable and the results work so nicely that the trade off is totally worth it in my view. If I find a bug, I fix it once and regenerate the nice, simple, concrete implementations for all.
I don't care about this one way or the other. Who among us is not using an IDE and cannot click though to the source in the lib (which you cannot do with macros)? The other thing is though, that generated code (at least, I've found) tends to trade off code cleverness/generic-ness for sheer volume. You don't need to write clever code in order to keep it terse and maintainable here, you can just output a lot of much simpler code that you wouldn't dream of maintaining by hand. So even though the code is invisible (to git, it exists in the jar), it's pretty simple - or at least the code I generate is. My point being that an IDE prompt and a compiler error are enough to get a user through it as there are no really clever definitions to absorb and reflect on. I suppose though, if you were writing one shared implementation for everyone to use then having the code visible on github would be important. Maybe it's just a difference in use-case? 🤔
I think this is the only real drawback in truth. Having said that, I've had a surprisingly large number of contributions in that area - maybe because Tyrian's code gen system is really quite simple? So I wouldn't be put off doing it again. Indeed, I am doing it again on a new project! |
Thanks for your comments!
(For context – Laminar does not, and can not, run on the JVM, because it requires native JS DOM types) In SDT / Laminar every tag has a unique "native JS element type" associated with it (to the extent that such unique types exist in JS / scalajs-dom) – for example, However, under the original design constraints of Scala DOM Types, we can't put That's why I made those types into abstract type params instead. But now we have a billion type params in all the SDT traits, and all that indirection is hard for users to follow. I mean not necessarily conceptually, but practically it's hard to get to the definition of I suspect some kind of JVM compatibility might be achieved by "stubbing" scalajs-dom types like |
Thanks for the effort! What is the source of truth for all those types currently? |
Do you mean where we bind abstract type params to concrete scalajs-dom
types? It's here:
https://github.com/raquo/scala-dom-types/blob/master/js/src/main/scala/com/raquo/domtypes/jsdom/defs/package.scala
|
…ion. Fixes #87. This commit parses existing SDT traits into the new structures, and clean up scaladocs, and adds a bunch more documentation links.
This is happening: #89 |
(sorry for the late reply, too many notifications...)
I was asking, where are the types originally came from? Did you enter them by hand, crawl them, etc. ? If they could be generated from an official spec, it would be ideal. This is what I found after a quick search: |
Type definitions originally came from ScalaTags, and have been manually updated since. Not sure how they ended up in ScalaTags in the first place, but I suspect they were entered manually or semi-manually. I'm not sure, but generating SDT types from official specs might not be feasible. Some concerns:
When all is said and done, the crawler/generator will be pretty complicated, mostly due to the need to override generated data in various ways. Meanwhile, due to the backwards compatible nature of web standards, existing keys essentially never change, so If anyone wants to take this up, I suggest focusing on the smaller use case of generating a set of SDT KeyDef-s that you want to add (e.g. grid styles), and then committing the output to SDT. Such a crawler / generator might well live in SDT, but the point is, it should never be run again on the same set of keys, to allow for trivial edits / additions to the generated key defs, without needing to code complex mechanisms for that. Hope that makes sense. With that kind of scope in mind, I'm not opposed to adding such an importer to SDT, but I don't have the time to develop it myself. |
I see. I don't have any objections here. A few more references:
|
Alright, this took way too much time, but it's done now. The generators are ahem not exceedingly elegant, but they work. Just finished integrating the changes into Laminar, and pretty happy with the result. For reference: Notable non-generated files in the latter commit:
To be clear, I wanted to make those changes to Laminar's API – that's the whole reason I did all of that in the first place. But in your own library, you can still generate pretty much exactly the same traits as you currently use, you'll basicaly just need to configure the generator with different params, and copy over Codecs and the "complex" keys. If anyone wants to give this a shot in your library:
Because the audience for this code is limited to a perhaps handful of experienced devs, and because we have two examples (GeneratorSpec and Laminar) to go off, I don't think I'll be writing any docs beyond putting a more polished version of that bullet list in the readme. If you're stuck, or if it seems that the customization method required for your use case is disproportionately arduous, please let me know either here or in discord, and we'll figure it out. Note: if you want to compile / run the new laminar tests locally, you need the latest commit from this SDT PR and also the latest commit from scala-dom-testutils master. |
One thing I'm not happy about is needing the sbt-buildinfo plugin in Laminar's build setup. It's used in The file is nested in It does work as-is, but I'm offensively bad at sbt, so it's quite possible that I made my own life harder than needed somehow. |
This looks great! :) |
Amazing! 👏 👏 What about dropping |
Yep, that's easily configured, and I'll try it later.
|
Pushed updated README for library authors. Note – it has a bunch of links to source files, but for now I haven't specified the URLs of those links, since the stuff isn't in master yet. |
This is amazing! Thank you for all the work, you have put into this. I am super curious to try it out and build new customizations. It will be interesting to test |
I've just released Scala DOM Types 17.0.0-M1 with the new generators. The README has instructions about integrating the new generator into your library. If you want to reference Laminar code for this, see the I plan to cut 17.0.0 together with Laminar 15.0.0. |
Currently, the design of SDT type listings is maximally generic, providing a lot of flexibility to esoterically minded library authors.
The issues below mostly apply to all types of keys – props, attrs, css style props, and event props, but we'll be looking at specific event props as a concrete example:
A developer who uses a UI library like Laminar (or Outwatch or Calico) might see
onWheel --> whatever
in their code, and ctrl-click on theonWheel
symbol in their IDE to go to its definition. The snippet above is where they will land.The definition of
onWheel
has a short ScalaDoc comment, but there is no concrete information about the value we're dealing with. We don't know what thisEP
type is, or what shapeDomWheelEvent
is – these types are both abstract.We could improve on
DomWheelEvent
by replacing it withorg.scalajs.dom.WheelEvent
– the concrete type that it resolves to for 99% of the users. Then users could proceed to ctrl-click into that type and see what kind of events they will get. This would improve ergonomics for most people. This benefit is obvious, the main issue is maintaining functionality for the other 1%, the use cases where we do want abstract type params.Currently
DomWheelEvent
is an abstract type param because ourtrait MouseEventProps
is in ashared
project – it's cross-compiled for Scala.js and the JVM, and so it does not have access to the JS-onlyorg.scalajs.dom
types.I designed it that way because I was hoping that Scala DOM Types would be used on the JVM as well, for example in tools that need to render HTML (web pages or email content) on the server. To my knowledge, as of today no such tools exist that use Scala DOM Types, but it's possible that they do exist, just that their authors are overly modest, and didn't let us know.
I also hoped that cross-compiling libraries would be using Scala DOM Types on both client and server, however, again, I'm not aware of any such libraries.
Also, @armanbilge pointed out that obviously there aren't enough IO monads in scalajs-dom, so he's looking into using our generic SDT design with a more FP-friendly alternative to scalajs-dom.
How could we provide concrete org.scalajs.dom types to the 99% of SDT users, while maintaining SDT's flexibility – one of our main design goals? I don't want developers to need to fork SDT or copy-paste our type listings into their project. Ideally there should be a single source of truth for SDT-style type listings in the Scala world. And thus it needs to be flexible enough for anyone to use.
I think one way or another, the answer has to involve code generation.
We could keep existing type listings as-is in the
shared
project, with all their abstract params, and generate ScalaJS-specific copies of all those traits. A bunch of dumb stuff like replacingDomWheelEvent
withdom.WheelEvent
. Or, do it the other way, doesn't really matter. Laminar would use the ScalaJS version, other libraries which need more flexibility would use theshared
project version with abstract type params. If someone is using both libraries in their ScalaJS project, redundant near-copies of SDT traits will be loaded into the JS bundle.Alternatively, we could keep some kind of "source of truth" for the type listings in the SDT project (whether in the shape of Scala traits, or something more structured and easier to parse), and have every consuming library (Laminar, Outwatch, etc.) generate their own bundle of DOM keys. We would need to provide a configurable generator for this, and I'm not quite sure how. Otherwise, same issues as option (1) in case you use more than one such library in your project, but there are some extra advantages too:
a) In addition to making
DomWheelEvent
a concrete type, we could also makeEP
a concrete type, so in Laminar you would see a fully concrete type:lazy val onWheel: EventProp[dom.WheelEvent] = eventProp("wheel")
b) Each library like Laminar could better customize what they want to generate, e.g.
click
vsonClick
,typ
vs`type`
, etc. (this can be seen as a disadvantage too).c) For special types like
DomElementMouseEvent
(canonicallyTypedTargetMouseEvent
), libraries would be able to choose the strategy they want, AND have that type be concrete too.d) Just like today, libraries could choose which traits they want to use, but the generator could also flatten the traits into a single fat trait, or into one trait per key type (props / attrs / etc.). Currently libraries need to create an
object
that mixes in 20+ traits. I have no specific insights, but I suspect that there are ways in which this is unhealthy in JS / ScalaJS.e) Libraries would be able to choose if they want
val
-s orlazy val
-s (See Switch definitions from lazy vals to vals? #86)f) All the DOM types would be contained in the library project itself, no need to jump into SDT.
g) Esoteric libraries that need something other than scalajs-dom would get the benefit of concrete types as well.
We could "backport" some of the advantages of option (2) to option (1) if we generated a set of "blessed" SDT configurations inside SDT, but I'm not sure if that's a good tradeoff, e.g.
EP
type would still remain abstract.Disadvantages of code generation:
The inelegance of redundant code, whether contained in SDT, or spread across several projects. There is no escape.
Working with generated code can sometimes feel like working with macros, i.e. "invisible code". We will avoid this by committing all generated code into github, so that all published code matches what you see on Maven Central, in the IDE, and on github. We do it this way in Airstream already.
Harder for users to figure out how to contribute
Examples of code generation:
Tyrian: https://github.com/PurpleKingdomGames/tyrian/blob/2501309271cd9cb07af4a63d616d0eec0b1d3a2e/project/TagGen.scala (cc @davesmith00000 in case this SDT issue is of interest to you)
Airstream (Not DOM related): https://github.com/raquo/Airstream/blob/master/project/GenerateCombinableEventStream.scala
Requesting comments!
I'm not yet sure which option is best, or whether the tradeoffs are worth it. There might be better options than what I've considered, or some other use cases I haven't realized. Any opinions or insights are welcome. Let me know what yall think.
Personal cc: @sjrd @keynmol @armanbilge @fdietze @cornerman @yurique @sherpal but the discussion is open to everyone.
The text was updated successfully, but these errors were encountered: