-
-
Notifications
You must be signed in to change notification settings - Fork 24
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
RFC composable class names #47
Comments
Whoa, this is truly remarkable, thank you! While I’m not sure about the static analysis benefits this provides, a Implementing this requires some serious re-architecturing, though. Also, I’m not sure whether On the long term, otion should be rewritten with rigorous testing in mind. |
I would like to share my opinion on my otion use cases in relation to what was proposed. About returning a string for all class namesThis: const styles = css({ color: 'blue', background: 'red' })
function MyComponent() {
return (
<div className={styles.background}>
<ChildComponent className={styles.color} />
</div>
)
} Can be easily replaced by: function MyComponent() {
return (
<div className={css({ color: background: 'red' })}>
<ChildComponent className={css({ color: 'blue' })} />
</div>
)
} or function MyComponent() {
return (
<div className={css({ color: background: 'red' })}>
<ChildComponent className={css({ color: 'blue' })} />
</div>
)
}
function ChildComponent(cssProperties) {
return <div className={css(cssProperties)}>Example<div>
} And if use typescript, you can use the type function ChildComponent({ cssProperties }: { cssProperties: ScopedCSSRules }) {
return <div className={css(cssProperties)}>Example<div>
} About
|
As much as I don't see a use in my current cases, I believe the proposal is quite interesting. So my proposals for changes if you choose to implement them are: Before implementation, separate otion into 3 packages:
After the separation, implement a fourth package, callend It could also be called why: I think the package size can increase considerably if everything is added in a package and otion would have two directions, one to generate classes as currently and the other to create combined classes Tell me what you think about it. |
Thank you for sharing your thoughts in detail, @etc-tiago! While object spreading is possible, someone may decide to pass stringified class names generated by otion to a component, e.g.: type GreetingProps = {
className: string;
}
function Greeting({ className }: ComponentProps) {
return (
<p
className={css(
{ color: "red" }, // Pre-defined styles
className // Overrides specified as a string of class names
)}
>
Hello!
</p>
);
}
function App() {
return <Greeting className={css({ color: "green" })} />;
} For this case, otion should keep track of its injected rules in a class-keyed Map. When composing an object with a list of class name strings, the latter should be mapped back to objects and then deep-merged with the initial object. I think the library should be re-architectured to accommodate room for implementing this functionality. |
@etc-tiago @kripod this proposal is a bit out of date 😅 FYI, that PR is waiting for comments/discussion since it's mostly a PoC rather than a final implementation and I don't want to work on it further before that happens 😅 |
@eddyw Thank you for your great work and the PR for making that concept a reality. I decided to rewrite otion from the ground up with high test coverage and this idea in mind. During the weekend, I even wrote a tiny library for deep-merging objects to help with this goal. It may take a while to get all these things together. Until then, the current version of otion can be used safely. |
Motivation & Examples
Currently
css
returns a single string with all class names, so it isn't possible to get a single class name for a specific property. For example:It'd be great if the result of
css
was like:An example use case:
This would offer many benefits:
& > div
) and just pass down class names as props. In this case, a child component that is re-usable but it should be styled differently depending on where it is used. For example, a re-usableHeading
component which could have different color.Static analysis is difficult or impossible
However, this is better and makes it possible to make static analysis of the code:
Details
stylex
was shown in ReactConf by Facebook, so the original idea comes from there:Based on that:
css
could return an object instead of serializing all classnames:This could allow to access any classname:
Note: also add
valueOf()
? 🤔otion
could export acreate
method (or choose better name) that could allow to create a styles object. This could basically just be (overly simplified version):So, similarly as with
css
:otion
could export acombine
method (or choose better) to combine style objects and deduplicate styles:TL;DR
It's a breaking change but I believe this could allow for:
Summarizing how the new API could look like and it's usage:
Let me know what do you think 😅 and if it's within the scope of
otion
to support this. I can help with the implementation 😄The text was updated successfully, but these errors were encountered: