readable classnames
Closed this issue · 22 comments
I think I've figured out how to do 'readable' classnames. a quick example -
css({
label: 'customStyle',
color: 'red',
':hover': {
color: 'blue',
fontSize: 20
}
}) becomes ...
css-customstyle_color-red_hover-color-blue_fontsize-20
i.e, [label]-[styles]
another variant would be [label]-[hash]
some notes -
- we can revert to hashed classnames in prod, keeping it tight/efficient.
- label(s) accumulate at the beginning of the class, for quick readability
open to feedback/questions/ideas.
How do you feel about a second parameters rather than mixed with the styles? This would define purpose more clearly.
Don't want to change/break current api. Making a wrapper shouldn't be hard tho.
So long as we keep a standard prefix so I can programmatically determine that a class name is from glamor, I'm good. 👍
I dunno. I have no big opinion on this. I never needed that for now and I could imagine that this can become quite unreadable with many styles...?
On Twitter you said that this is the number one request. Can you explain what the people would like to achieve with this? Better debuggability? Maybe this could be achieved in a different way (Source Maps for inline styles, dev tools, ...)
I could imagine that this can become quite unreadable with many styles
Agreed. I think it should just be css-[label]-[hash] and it could be that in prod for all I care (could write a babel plugin to remove the label as well for people who really care about bytes).
@donaldpipowitch Can't do sourcemaps with dynamic code, investigated it a bunch. Can cap at X characters to prevent extra long classnames.
Heck, I could try to make this userspace configurable, so you could drop in an emoji classname generator if you'd like. :D
Can't do sourcemaps with dynamic code
I'm very interested to see what sourcemaps would look like with glamorous! That'd be super cool!
If you cap at X characters, I could see collisions happening fairly easily though 😖 Maybe require a label? That seems kinda messy though... :-(
Can attach the hash to truncated classnames
But it's generated at that point right? So no sourcemaps for those?
No, not generated till runtime. Can't generate sourcemaps except for very trivial cases.
I still think that long class names would be kinda annoying :-/
Noted, will keep in mind! This won't land immediately, will share progress and what I learn
FWIW, my vote is css-[label]-[hash].
Do you plan to propagate this change to glamor-Aphrodite as well?
Is this change in the works? Wondering if it's in progress or it already exists. I would definitely agree with comments above that css-[label]-[hash] would be great; or even just [label]-[hash], no need for the css prefix.
I would ask that some identifying thing is still available. Glamorous relies on the css- prefix to not have to look up every class name in glamor's styleSheet. Would be a shame to lose that optimization :)
in the works, life getting in the way. the css-prefix should remain.
css-customstyle_color-red_hover-color-blue_fontsize-20
Are these directly from glamor, or would it make sense to use the JS css naming convention?
css-customstyle_fontSize-20
The modifiers do present a different problem. Do we handle them all the same? Would it be 'hover-color-#f5f5f5' and 'css-before-customstyle_'?
I'll write a longer comment in an hour or so with details and learnings. Hang tight my peeps!
(In the next 24 hours 😔)
No stress man. You've got lots going on!
I've published v2.20.32 with an implementation of the above. basically css-[label]-[hash], as discussed. give it a spin if you're interested! I'll leave this issue open for a day or so while I try to break it, will close soon after that.
seems alright, closing this, feel feel to reopen/open a new issue if you find a problem.