Open andywer opened 5 years ago
This is better. Some thoughts:
In the example above you import useStyles and useStyle though only one is used.
useCSS and useStyle should really be just one function. Just call it like this: useCSS()({ ...})
I understand static extraction is not a project goal, but a slight change will make it possible to write a Babel plugin to perform static extraction for those who desire it while also bringing it closer to most React hooks. The return signature should be an array with an empty object as the second item. Like so:
const [className, style] = useStyle(...
Users don't have to use the second return value but static extraction would be able to rewrite it so that a style object with CSS variables set is returned for those dynamic parts. This would allow one to use emotion in development and Linaria in staging and production without issue.
If you accepted a tagged template literal, your second argument IS the dependencies array, which is quite slick.
Other things to consider:
Sorry for the delayed response. Would have loved to see more responses, though.
Before we blow up the thread again, let's move detail discussions to https://gitter.im/threadsjs/community and focus on the big topics here and keep it short here.
Just generally:
useCSS and useStyle should really be just one function
They solve the same problem, but have a different API and require a different logic under the hood, so I think merging them into one hook would only be misleading. You will also very rarely use both of them together. You should probably decide on one style for consistency's sake.
static extraction &
const [className, style] = useStyle(...)
The hook will never return inline styles, but only class names, so sticking to returning the conventional { [originalClassName]: rewrittenClassName }
object that other CSS-in-JS solutions use too seems perfectly reasonable to me.
Plus: The only built-in hooks that return an array are the two state management hooks that return a state and some kind of update function. Those hooks here don't share either of those things.
I also do think that we could even make static extraction work with the API as outlined above in the long run. But let's ignore that for now, discuss it on gitter if you really want to 😉
If you accepted a tagged template literal, your second argument IS the dependencies array
Not sure how this is any different than with an object. In the end it's just a different syntax...
Other things to consider (...)
Composition can be achieved quite easily by using the object spread operator, like: const className = useStyle({ color: "white", ...otherStylesToBlendIn }, [])
It has a few limitations regarding nested selectors, but I think we can work around them with one or two lean helper functions. Details can be discussed on gitter or #4.
To have distinct utility and component styles and use them together in the component, you can always have a top-level component that renders the utility styles and puts the generated class names in a context, so child components can access them. We can always add helpers to make it convenient to do this kind of thing.
No styled() helper support.
- No styled() helper support.
+ No styled() helper yet.
Can be implemented easily if need be. Would wrap the component in another one calling the hook and passing the class name to the component as prop className
.
Hope that makes things a little clearer! Let's try to keep it short and concise here, though.
I'll leave a few notes.
Not sure how this is any different than with an object. In the end it's just a different syntax...
My point is tagged template users don't need to repeat the dependencies array:
const className = useStyle`
background: ${props.color};
color: white;
padding: 0.5em 1em;
`
Is JS syntax sugar for:
const className = useStyle([
"\n background: ",
";\n color: white;\n padding: 0.5em 1em;\n"
], [props.color])
So if you accept a tagged template literal, all you need to do is grab the second argument as the dependencies array as usual and then pass both arrays to the function that handles the tag (i.e. css) So by all means support both syntaxes natively as the the tagged template users don't need to type their variables twice.
Implementing this would be very simple. If the first argument is an array, include the second argument in the css call.
using the object spread operator
I was aware of this. The issue with this approach is that your CSS file grows massively with SSR as no two classes are ever applied to an element.
That is all.
Proposal for API change
This is a draft for an updated API, based on the experiences made with the current API and its limitations (like #1), the discussions in #2 and other styling hook implementations like react-jss.
Benefits
styleHooks()
call allows clear static injection orderstyleHooks()
allows grouping stylesheets by component file, allowing easier optimizationsuseStyles()
API is now consistent with other React.js hooksuseCSS()
as a simple solution to define the styles "inline" in the JSX elementDetails
Can optionally pass a label to
styleHooks()
, likestyleHooks("Button")
, to give the CSS classes readable names in development for a better debugging experience.Dynamic values would now be allowed to be put in styles directly, without a function as with the old API (
color: () => props.color
). This will prevent us from splitting the dynamic rules from the static ones in a CSS class, but that was mostly done to improve the performance before.My assumption is that the new API with the initial
styleHooks()
call in each component file will allow better performance anyway and will reduce the impact of that dynamic/static optimization to more or less zero. So let's give it a try and aim for a more convenient API.Relation to other styling libs
Should be able delegate the actual styling to one of those well-known and widespread styling libraries:
Static style extraction á la Linaria is considered out-of-scope. It is a cool project and interesting concept, but we are aiming for a solution that works for the majority of the CSS-in-JS users and without any custom Babel or Webpack setup.