Skip to content
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

New dsl, step 3/3: migrate tag-specific prop types from Html to Dom_html #136

Open
jchavarri opened this issue Feb 5, 2022 · 4 comments

Comments

@jchavarri
Copy link
Collaborator

After original api implementation in #119 and update of ppx to support it in #128, the last step of this new api consists on migrating the "specs" implemented with types in ppx Html module to the Prop module in Dom_html. The goal being to make impossible to pass wrong attributes to e.g. a div.

I am not sure about the best way to implement this, probably could use an approach similar to bs-css, with polyvariants + subtyping to reflect inheritance or composition based on smaller subsets of props, like here.

@glennsl
Copy link
Contributor

glennsl commented Feb 5, 2022

This sounds very complicated, and I'm not really sure it's worth it. I can't remember a single instance of having used the wrong attributes for a tag, which might also be because the consequence of doing so is entirely benign. In almost all cases it would just not do anything. You might get a warning in the browser console, and HTML validation would fail, but that's about it.

There is one feature that I think would make it worthwhile, however: The ability to auto-complete just the valid props for a tag. Unfortunately I have no idea how to even approach a solution for that problem.

@glennsl
Copy link
Contributor

glennsl commented Feb 5, 2022

I also think it was actually me who came up with the scheme of using polyvariants to type CSS. At least I had a proper go at it with bs-typed-css quite a few years ago, and came to two conclusions from that: 1) it's very hard, and 2) there's very little benefit. Today I just use strings as values, and have very few issues with that approach.

It is a very fun challenge to try to come up with a scheme to make such a complicated system well-typed, however. So please don't let this discourage you from experimenting with it. I'm just not convinced this is all that valuable for production code.

@jchavarri
Copy link
Collaborator Author

jchavarri commented Feb 5, 2022

Yeah I think these points are worth considering. What you mention makes me think about tyxml, which also left me (as a user) with similar feeling: the time spent trying to debug type errors was > time saved by elements being created with wrong attributes, just because the latter happens so rarely.

In general, considering how few resources jsoo-react has :) I tend to pick the simplicity path over the complexity one, unless the complexity is absolutely needed.

On the other hand, to make full argument, the constraints (I mean, the set of attributes that each element allows) are very unlikely to change over time, and also, regarding how complex the implementation is, users will not perceive this complexity, because the api design does not need to change from their perspective.

@davesnx what do you think?

@davesnx
Copy link
Member

davesnx commented Feb 6, 2022

I fully agree, I personally prefer to keep the API very unsafe in those cases. bs-css makes it look easy for simple cases, but for using more advanced CSS features is totally a nightmare, is one of the reasons that I started styled-ppx in the first place!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants