css prop always converted to className prop?
Closed this issue · 10 comments
I was updating a project today to use the css prop and found myself adding it to 3rd-party components without thinking expecting it to work. But of course as they don't know know what to do with the css prop, things failed. Since almost all open-source react components handle className — perhaps that's what the css prop should be turned into?
yes, now that classnames work I should use that for the css prop. will do!
awesome! This should pretty much make Glamor Just Work™ across the React ecosystem :D
Well... I do purposely avoid non dom tags because I thougt I'd end up clashing with custom components' 'css' prop if it accepted any. Maybe that's not an issue?
I'd guess a component having a css prop is really rare. All theming options I've seen are className or inline styles based.
Oh but also what I'm suggesting (if this wasn't clear already) is if the tag is non-dom, then you convert the css prop to a className prop and pass that into the component instead.
So <Custom css={{ margin: 10 }} /> becomes <Custom className="hashed-classname" />
You could detect I guess if the prop value passed in isn't an object or an array and just pass it through untouched...
But regardless, I think it's fine throwing your weight around a bit. Glamor is legit and the css prop is a very sensible name for this sort of thing and I can't remember seeing anyone use it before. Also it'd make Glamor even more legit amazing because there's a lot of components that only let you style them with css (via setting a class or using there's) and this would let us have our inline styles cake and eat it too.
It'd also be a lot less confusing to explain Glamor to newbies as it stands right now, you'd have to say something like "you can use Glamor here but not there" which wouldn't quite make sense.
What if instead of using the css prop you just use just spread the existing glamor functions (style, merge, etc) on the component and instead of looking for that css prop, the custom createElement function instead looks for the data-css, data-glamor-theme, or glamor generated classNames and performs the necessary merging into a new className?
that's a bit too much 'magic' for me, I'm afraid. That said, there's some work happening in making a helper to do similar #91
Fair enough. Just curious, which part pushes it past too much? I realize after looking back that what I suggested is trying to solve more than just the problem at hand - i.e. the possibility of conflicting prop names. I do think it's a bit optimistic to think that it won't be an issue in the future - a custom component wanting to use css as a prop name isn't unreasonable. You could still use a function instead of a prop (e.g. <Button {...css(stuff)} />) that applies whatever unlikely prop name you want under the hood to be used by createElement.
#91 was what I was thinking of with my earlier suggestion.
This is live in 2.17.16, give it a spin and tell me how it feels and whether you like it?
It's working great! Used the css prop on a number of 3rd party components and the className got passed through just fine! Inline styles nirvana :-D
Thanks!!!