mikesol / purescript-deku

A PureScript web UI framework
https://purescript-deku.surge.sh/
Apache License 2.0
123 stars 12 forks source link

Introduce tidy-codegen and Deku.DOM.Indexed #101

Closed jterbraak closed 10 months ago

jterbraak commented 11 months ago

I really like what you have done with the framework but the one thing i miss from Halogen is the way it defined elements using row types. The current implementation also seems to increase startup-time of the compilation by about 2-3 seconds. This pull-request aims to add an alternate set of modules for element generation that is still compatible with the primary DOM-elements.

This will also replace the existing codegen from the python scripts to a project written in Purescript itself. Additionally it sources its data mostly from the available spec instead of scraping MDN.

mikesol commented 11 months ago

Oh wow, this is neat! Let me know when you'd like some review and I can start reading through the PR and leaving comments.

mikesol commented 11 months ago

Also, one thing to note is that a lot of work is going into deku for a 1.0, starting with the backend optimizer, working its way through purescript-hyrule and purescript-bolson and finally hitting this repo. There are many PRs preparing this, so given how extensive this PR is, I thought I'd give you the heads up just so that you don't hit merge conflict hell with it. We're still at least a week off (if not more) from merging any of that stuff. Here are some links:

The basic idea is to make Deku (and a few other projects) as fast as Astro by doing aggressive inlining when possible. This only works, though, if several things change in concert.

If you're interested in participating in this effort, a few of us are hacking away at it & I'd be happy to give you more info.

jterbraak commented 11 months ago

The branch is working again and the tests are passing. There is still an issue with a missing unset function in deku-dom-indexed but I want to merge fantasy-land first as the new attribute representations makes the implementation easier. I didn't want to force deku-dom-indexed on every user so I also split the package using new the spago version and added a spago.yaml to every sub-package. I also went ahead and split off the CSS module.

How would you like to handle #100? In this implementation they both would be DOM.Type. I tried to minimize escaping where it was not necessary but I don't know why Xtype was named like that in the original version.

Another issue is the namespace support. There is some code in https://github.com/mikesol/purescript-deku/blob/c5c90450885592aca78c5e18ab91f9ab4603c959/src/Deku/Interpret.js#L272 that hacks in SVG support for elements. I would like to patch this by extending elementify and friends but the original DOM modules only have one function per tag name. Elements like title are in both SVG and HTML so there should be a way to select either one. My preferred way would be to split the giant DOM module into SVG and HTML like i've done for DOM.Indexed.

jterbraak commented 10 months ago

The old DOM modules are removed as requested. I fixed up the tests to use the indexed version and they pass. elementify has been patched now supports proper namespaces.

One more thing about namespaces: I opted to put elements, attributes and values into the same module/namespace. Obviously this creates some conflicts between, for example, the element title and the attribute. So I prefixed attributes with a _. This way all HTML/SVG/MathML related values still fit into one module. If you think this convention is ugly or confusing I can still split them up like Halogen does. I.e. a HTML module for elements and Attributes for the rest.

When rewriting applicatons these regexes helped me in VS Code: D\.([A-Z])(\w+)\s+!?:= replace to D._\l$1$2_

mikesol commented 10 months ago

If you think this convention is ugly or confusing I can still split them up like Halogen does. I.e. a HTML module for elements and Attributes for the rest.

I don't think it's ugly nor confusing, but in the absence of any other factors, I'd opt for the Halogen one because it's something that is likely familiar to folks.

However, vision trumps prior art, so if there's a reason you want them all in one place & feel somewhat strongly about it then let's go with that.

jterbraak commented 10 months ago

What do you mean with generating stuff like click_? I could overload _onClick and add cases for Effect Unit and Cb. I wanted to avoid that because I find that typeclasses and overloads are bad for disoverability in the editor. I still kinda want to axe the whole Keyword system.

mikesol commented 10 months ago

I still kinda want to axe the whole Keyword system.

What do you mean by the Keyword system. Stuff like D._id_?

jterbraak commented 10 months ago

For _type i generate all valid cases. For example Keyword "number". An overload of _type then allows only valid keywords and String. So you can write:

D.input [ D._type_ D.__number ] ...
jterbraak commented 10 months ago

Also after writing the last comment I'm coming around to moving attributes to another module.

mikesol commented 10 months ago

For example Keyword "number".

If you axed this, what would you replace it with?

jterbraak commented 10 months ago

Well the old implementation left them stringly typed so we could go back to that or generate an actual concrete type with a String escape. ala:

data InputType
    = InputTypeNumber
    | InputTypeText
    ...
    | InputType String

Kinda like what Halogen does :). Although they use handwritten code to convert from the Web.HTML modules and I'm not about to do that.

mikesol commented 10 months ago

Got it. So my general philosophy is that if there's gonna be an ADT for one enum in the Dom spec for attributes (in this case an input's type), we should have ADTs for every enum in the Dom spec for attributes. This is the rabbit hole I went down with purescript webgpu, which is completely free of string-typing. But it only makes sense if the spec is ironclad, not meant to be broken, and explodes in unpredictable ways when you use anything other than spec conformant values (that's WebGPU in a nutshell). For more permissive and lenient stuff, I'm a fan of strings.

jterbraak commented 10 months ago

So the input type was an example. There are loads of keywords for autocomplete, sandbox etc. But the "Spec" does not always specify whether the keywords are exclusive. In HTML anything goes. So most of the time we have to allow String. I now tend towards applying the keywords beforehand:

type = ...
type_ = ...
typeNumber = type_ "number"
typeText = type_ "text"
typeDate = type_ "date"

So when you type Attr.type in the editor all the options get listed. And the escape is still there as type.

mikesol commented 10 months ago

Would it make sense, then, to do this for all of the enums (autocomplete, type, sandbox, etc) - ship the standard ones and always provide a backdoor for custom ones?

jterbraak commented 10 months ago

Yeah, that's my plan. I'll get this and the separate Attribute modules done in the morning.

Still not sure what you mean with the click_ changes?

mikesol commented 10 months ago

I mean something like:

import Deku.DOM as D
import Deku.Attr as DA
import Deku.Listeners as DL

foo = D.button [DA.id_ "foo", DL.click_ (alert "hello") ] []

That sort of thing. Basically that we'd have a namespace for the listeners as well that is as exhaustive as the attributes. Which is a bit harder because there's more diversity there, for example certain listeners are "smart" enough to listen to numbers (like slider), keys (like keyUp) etc.

jterbraak commented 10 months ago

With the changes it's now possible to write

import Deku.DOM as D
import Deku.DOM.Attributes as DA
import Deku.DOM.Listeners as DL

foo = D.button [DA.id_ "foo", DL.click_ \_ -> alert "hello" ] []

I might be possible to add more variants for every event but I don't really see the point of making it any simpler. I also moved the old Listeners module into Combinators and reexported it from every Listeners module. That enables code like:

import Deku.DOM as D
import Deku.DOM.Attributes as DA
import Deku.DOM.Listeners as DL

foo = DL.div_
  [ DL.input [ DA.xtypeNumber, DL.valueOn DL.input alert ] []
  , DL.input [ DA.xtypeRange, DL.numberOn  DL.input $ show >>> alert ] []
  , DL.input [ DA.xtypeCheckbox, DL.checkedOn DL.input $ show >>> alert ] []
  ]

With the change to the modules type, in and class have to be escaped. This is done in Common.escape, which generates xtype, xin and klass.

mikesol commented 10 months ago

Awesome! Lemme know when it's ok to merge & I'll merge it!

jterbraak commented 10 months ago

Still got some ideas but I'm about done with this part. You can merge it.