This is the way re-classnames concats css classnames. open technically works but if local opens will be available at some point, I don’t mind to wait. Current react components in.re work so no reason to touch them and introduce odd code that will be obsolete soon and will be a bad example for new developers.
We ended up just adding (+++) which just joins two strings with space if they’re both non-empty. And since +++ lives in a module that is open everywhere, we don’t need local opens.
That said, it’s not a day and night improvement over the bare Cn.make.
To be honest I’ve never fully understood the urge to test on runtime if the CSS strings are non empty, worst case scenario you have an extra space, is there any browser that would trip on that? I’m by no means a CSS specialist so don’t hesitate to prove me wrong ^^
2 Likes
Never understood the benefits of classnames libraries either. Have been happily using string concatenation and string interpolation for that matter ever since and enjoy predictable performance, zero dependencies and a syntax everyone understands (especially our designer).
%tw("flex items-center") + className->take
What does this even do?
%tw produces string and className is of option<string> type. take does this. Though I do ~className="" so it’s of string type within the component.
Why not just do ["flex", "items-center"]->Js.Array2.joinWith(" ")? If you really want to, you can shorten that: cx(["flex", "items-center"]).
May I suggest the example README, converted:
(+) infix operator
Before:
Cn.("one" + "two" + "three")
After:
cx(["one", "two", "three"])
append
Before:
Cn.append("one", "two")
After:
cx(["one", "two"])
fromList
Before:
Cn.fromList(["one", "two", "three"])
After:
cx(["one", "two", "three"])
on
Before:
Cn.("one" + "two"->on(condition))
After:
cx(["one", condition ? "two" : ""])
onSome
Before:
Cn.("one" + "two"->onSome(myOption))
After:
cx(["one", myOption == None ? "" : "two"])
mapSome
Before:
type t =
| One
| Two
| Tree;
Cn.(
"one"
+ mapSome(
Some(Two),
fun
| One => "one"
| Two => "two"
| Tree => "three",
)
)
After:
type t =
| One
| Two
| Tree
[
"one",
switch Some(Two) {
| Some(One) => "one"
| Some(Two) => "two"
| Some(Tree) => "three"
| None => ""
}
]->cx
take
Before:
Cn.("one" + Some("two")->take)
Cn.("one" + None->take)
After:
let take = Belt.Option.getWithDefault // or whatever
cx(["one", take(Some("two"), "")])
cx(["one", take(None, "")])
onOk
Before:
Cn.("one" + "two"->onOk(Ok("ok")))
Cn.("one" + "two"->onOk(Error("err")))
After:
open Belt // this works too
cx(["one", Result.isOk(Ok("ok")) ? "two" : ""])
cx(["one", Result.isOk(Error("err")) ? "two" : ""])
mapOk
Before:
type t =
| One
| Two
| Tree;
Cn.(
"one"
+ mapOk(
Ok(Two),
fun
| One => "one"
| Two => "two"
| Tree => "three",
)
)
After:
type t =
| One
| Two
| Tree
[
"one",
switch Ok(Two) {
| Ok(One) => "one"
| Ok(Two) => "two"
| Ok(Tree) => "three"
| Error(_) => ""
}
]->cx
onErr
Before:
Cn.("one" + "two"->onErr(Ok("ok")))
Cn.("one" + "two"->onErr(Error("err")))
After:
open Belt
cx(["one", Result.isError(Ok("ok")) ? "two" : ""])
cx(["one", Result.isError(Error("err")) ? "two" : ""])
mapErr
Before:
// example has a bug
After:
//
none
Before:
Cn.(
switch (x) {
| Loading => Css.loading
| Loaded => ""
}
);
// vs
Cn.(
switch (x) {
| Loading => Css.loading
| Loaded => none
}
);
After:
// same
Benefits:
- No infix addendum, combinator, warning 44, local open, extension point, curry/uncurry.
- Only api/concepts needed: function, array, switch.
- Readability++.
- Juniors and seniors end up writing the same simple code.
- Check the amazing output difference!
- Free syntax upgrade =)
I understand the tidiness with the spacing and all, but you can just swap that generic joinWith with another one that collapse spaces if you really want that.
Likewise, you can do:
`one ${condition ? "two" : ""} three`->cleanUp
Equivalent conversion here.
Another perspective is that generating css classnames is the simplest scenario of string concatenation. If we’re resorting to extension point, combinator, infix and others then maybe we need to reconsider.
3 Likes
The main reason of switching to infix api was perf since a + b doesn’t introduce additional allocations. I’m not sure if I checked arrays (should be faster then lists I guess, I will run benchmarks later today) but list api was more than 4 times slower than using infix.
It does introduce lots of intermediate string allocations, though JS fares well with those (at least V8). Theoretically a join should be much faster but in reality I doubt so; JS engines are good but not that good nowadays… so yeah a bajillion string concatenations is likely faster even. The JS output really isn’t great though.
Even if it’s a bit slower, I think the benefit of basically removing the entire cognitive overhead of a task that reduces to concatenating a few strings, is worthwhile and seems right, philosophically speaking.
Also, worst case, you can still use the new interpolation syntax!
2 Likes
Small update: wrote a small join that eliminates spaces and all, and tried interpolation too. Here’s the benchmark using re-classnames’ bench:
Join is third best and interpolation the obvious best. So yeah it’s not too surprising! But at this point the benchmark isn’t even accurate anymore because most of the conditions have been eliminated (or in a sense, actually more accurate of real-work workloads now).
Output difference between infix:
join:
and interpolation:
A little bit of history: I was the original maintainer of the React classnames package before handing it to someone else who later heavily regretted spinning the API out of control. I think we have a chance to get back to the basics here and just join/interpolate. The simplicity will be a refreshing step up from the JS React ecosystem I believe.
5 Likes
I did manage to fully upgrade my package to the new ReScript syntax. It’s been great so far! The formatting is so much better than ReasonML ever way. It’s a much cleaner syntax too!
4 Likes
welcome to the future! 
2 Likes
@chenglou This looks good to me. The only question is if infix api should be preserved in rescript-classnames since it’s as twice as fast as join. I mean it might be confusing for some new comers but providing option that is 2x faster for free for those who prefer using this utility and comfortable using infixes is ok imo. Mind I create a separate poll to get the community feedback?
I’m not sure this makes sense. I think after looking through the examples, it’s clear that join is the the easiest api for newcomers and if you are doing advanced things for performance then you should be doing interpolation. The infix api exists in an awkward middle ground where it’s orders of magnitude worse than best possible perf and also harder to read than an option that is same order of magnitude perf-wise.
1 Like
Agreed. // 20 characters
Btw I’m rather positive (as I was positive about + being faster in my post before the benchmarks) that the + speed doesn’t translate into actual code speed, because there’s a large difference giving the JIT the same function a bajillion times vs interspersed a few times here and there in a codebase, not to mention the associated learning overhead. Also, interpolation is like 170x faster in synthetic and likely still faster in real-world. And the output of both alternatives are really important.
This is a tangent but for interpolation, we might have some extra nice things planned. Stay tuned.
1 Like
@chenglou @rickyvetter Sorry, I didn’t look at the join implementation close enough. Apparently, it produces the same output with multiple spaces as interpolation except it’s 350+ times slower. I kinda don’t see a point in join at all?
Where do you see that 350x slowdown lol.
Also I can just paste you a join that does clean up spaces. Here’s a random one:
let join = arr => {
let result = ref("")
for (i in 0 to Js.Array2.length(arr) - 1) {
switch (Js.Array2.unsafe_get(arr, i)) {
| "" => ()
| name => result := i == 0 ? name : result.contents ++ " " ++ name
}
}
result.contents
}
@chenglou Ah I’ve been searching through the thread for the join implementation that you used for benchmarks and thought you used this one
Why not just do
["flex", "items-center"]->Js.Array2.joinWith(" ")?
Now I re-read your post and you just didn’t post here the one that you used int the benchmarks and it does eliminates additional spaces. My bad.
Aaah yeah sorry for the confusion. I meant something like that. Basically a quick loop over the array concatenating non-empty strings.
Since this seems settled, I’d like to add 2 addendums which are imo practically and spiritually important:
-
For this use-case, I’d avoid using
optionfrom a state perspective. If you use anoptionthen you’ve accidentally got yourself an extraSome("")that might cause bugs. Basically you end up with 3 states:Some(actualClassname),Some("")andNone. Using more combinators would have hidden those deeper (in fact, the readme examples don’t catch that, simply because e.g.Some(emptyString)->takebenignly hid this problem, now you end up debugging a funny empty string problem somewhere else in the codebase).For classnames you’re aiming for 2 states:
actualClassnameand"". This is akin to what I’ve said in the big DOM thread; folks gotta be careful putting the cart before the horse trying to use combinators and other features and end up with a higher bug rate, higher cognitive rate and a higher confidence (“because FP concept!”). We’ve had to deal with many such bugs internally, including a double digit amount of exactlySome("").A situation where you actually want tri-state is e.g. profile nickname, where you explicitly distinguish between no nickname, an empty nickname and a nonempty nickname.
-
At the end of the day, we need to realize that we’re doing string juggling only because HTML preferred the syntax sugar of a string DSL for classnames instead of a proper array, or in the case of HTML/XML, some admittedly unwieldy
<names><value>flex</value><value>col-3</value></names>.It’s ironic because that DSL gymnastic ended up fostering the wrong mental model for classnames, which in turn gave the wrong mental model here of using various tricks instead of treating the problem as what it is: specifying an array of strings or ids. Thus the FP <-> production disconnect we often see (at this point we’ve come back full circle to the thread’s topic). So yeah, going back to a boring array of strings is the right thing to do. That, and interpolation, which isn’t the most correct but definitely has the shortest amount of mental translation distance, which itself is worth a lot.
3 Likes
Is custom infix operators back?