raquo / scala-dom-types

Scala types for your library to represent HTML tags, attributes, properties and CSS styles
MIT License
91 stars 27 forks source link

RFC: Use concrete types instead of generic type params? #87

Closed raquo closed 1 year ago

raquo commented 1 year ago

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:

trait MouseEventProps[
  EP[_ <: DomEvent],
  DomEvent,
  DomMouseEvent <: DomEvent,
  DomElementMouseEvent <: DomMouseEvent,
  DomDragEvent <: DomMouseEvent,
  DomWheelEvent <: DomMouseEvent
] { this: EventPropBuilder[EP, DomEvent] =>

    /**
    * The click event is raised when the user clicks on an element. The click
    * event will occur after the mousedown and mouseup events.
    *
    * MDN
    */
  lazy val onClick: EP[DomElementMouseEvent] = eventProp("click")

  // ... more props here

  /**
    * Fires when the mouse wheel rolls up or down over an element
    */
  lazy val onWheel: EP[DomWheelEvent] = eventProp("wheel")

  // ... more props here
}

A developer who uses a UI library like Laminar (or Outwatch or Calico) might see onWheel --> whatever in their code, and ctrl-click on the onWheel 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 this EP type is, or what shape DomWheelEvent is – these types are both abstract.


We could improve on DomWheelEvent by replacing it with org.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 our trait MouseEventProps is in a shared project – it's cross-compiled for Scala.js and the JVM, and so it does not have access to the JS-only org.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.

  1. 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 replacing DomWheelEvent with dom.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 the shared 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.

  2. 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 make EP 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 vs onClick, typ vs `type`, etc. (this can be seen as a disadvantage too).

    c) For special types like DomElementMouseEvent (canonically TypedTargetMouseEvent), 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 or lazy val-s (See #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.

  3. 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:


Examples of code generation:


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.

armanbilge commented 1 year ago

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.

To make it concrete, it looks like this:

https://github.com/armanbilge/calico/blob/671ff27cb87f3ddc73294f00d6b67e591a147d69/calico/src/main/scala/calico/html.scala#L61-L154

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 IO-based project like Outwatch or ff4s is also interested in what I'm doing, honestly you should leave me in the dust 😛


cc @buntec https://github.com/buntec/ff4s

armanbilge commented 1 year ago
  • 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 replacing DomWheelEvent with dom.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 the shared 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.

If the scala-js-dom versions just extend the abstract traits and override every method with a call to super, I believe that would have the desired effect. It would be even cooler if that could be inlined away to avoid bloating the bundle, but I'm not sure about that.

raquo commented 1 year ago

So if I'm reading correctly, am I currently the only known user of this capability? 😅 Unless some other IO-based project like Outwatch or ff4s is also interested in what I'm doing, honestly you should leave me in the dust 😛

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)

If the scala-js-dom versions just extend the abstract traits and override every method with a call to super, I believe that would have the desired effect. It would be even cooler if that could be inlined away to avoid bloating the bundle, but I'm not sure about that.

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 extends domtypes.shared.TraitName[...] to the generated code – but I don't think there are significant advantages to inheritance. Calling super introduces indirection whereas we're trying to reduce indirection.

armanbilge commented 1 year ago

Calling super introduces indirection whereas we're trying to reduce indirection.

Meh, turns out to be moot. super may be not be used on val

davesmith00000 commented 1 year ago

Thanks for the cc @raquo - interesting discussion!

Also, @armanbilge pointed out that obviously there aren't enough IO monads in scalajs-dom

:joy: If it's any consolation, he said that to me enough times that I put them in. :wink:

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.

The inelegance of redundant code

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 div:

  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.

Working with generated code can sometimes feel like working with macros, i.e. "invisible code".

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? :thinking:

Harder for users to figure out how to contribute

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!

raquo commented 1 year ago

Thanks for your comments!

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

(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, lazy val div is not just an HtmlTag, it's an HtmlTag[dom.html.Div], and lazy val input is HtmlTag[dom.html.Input]. Those types are useful on the frontend in Laminar because we don't have virtual dom, it's not uncommon to refer to the real DOM node.

However, under the original design constraints of Scala DOM Types, we can't put dom.html.Div or dom.html.Input in the definitions of div and input, because those scalajs-dom types do not exist on the JVM, and the div and input values needed to exist there too.

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 dom.html.Div type if you're looking at a usage of div in your code, because the definition of div only has abstract types in it. I guess the boring Div is a bad example, but there are also various weird element types and event types which you might want to look up.

I suspect some kind of JVM compatibility might be achieved by "stubbing" scalajs-dom types like dom.html.Div on the JVM, but I've largely ignored that approach as relatively resource intensive (to stub them all, and then keep up with changes to scalajs-dom) and not-very-useful since all you get are fake empty types. Also, wouldn't help with IO monads.

fdietze commented 1 year ago

Thanks for the effort!

What is the source of truth for all those types currently?

raquo commented 1 year ago

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

raquo commented 1 year ago

This is happening: https://github.com/raquo/scala-dom-types/pull/89

fdietze commented 1 year ago

(sorry for the late reply, too many notifications...)

Do you mean where we bind abstract type params to concrete scalajs-dom types?

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:

raquo commented 1 year ago

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.

fdietze commented 1 year ago

I see. I don't have any objections here.

A few more references:

raquo commented 1 year ago

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.

raquo commented 1 year ago

One thing I'm not happy about is needing the sbt-buildinfo plugin in Laminar's build setup. It's used in build.sbt to read the ScalaDomTypes version defined in project/project/ProjectVersions.scala.

The file is nested in project's project because the SDT is listed as a dependency in project/build.sbt, because I think this is needed for me to be able to call into SDT's generator method from build.sbt.

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.

sjrd commented 1 year ago

This looks great! :)

yurique commented 1 year ago

Amazing! 👏 👏

What about dropping lazy val-s? I think I've seen it mentioned somewhere, in the context of scala.js now being able to strip unused val-s, right?

raquo commented 1 year ago

Yep, that's easily configured, and I'll try it later.

raquo commented 1 year ago

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.

cornerman commented 1 year ago

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 lazy vals, vals and inline defs with the new code generator :+1:

raquo commented 1 year ago

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 next-0.15 branch there.

I plan to cut 17.0.0 together with Laminar 15.0.0.