Replies: 41 comments 1 reply
-
|
Beta Was this translation helpful? Give feedback.
-
|
Beta Was this translation helpful? Give feedback.
-
|
Beta Was this translation helpful? Give feedback.
-
|
Beta Was this translation helpful? Give feedback.
-
This is exciting. My only hesitation with |
Beta Was this translation helpful? Give feedback.
-
The proposed API would add significant maintenance complexity and needless surface area to StyleX. We have a design principle in React which is to only have one way to do things, even if it is more verbose in some cases. Defining styles directly on an element only looks readable in the most simple of cases. We already have 3 functions to resolve styles (one is legacy) and 2 ways to merge them (include and arrays). We could be simplifying the API rather than adding more ways to do the same thing |
Beta Was this translation helpful? Give feedback.
-
If flexibility is paramount would it not make sense to implement your concerns as ESLint warnings? The reality of not implementing Another implementation would be having strict mode, which disciplines developers to write code one way, like @necolas is suggesting. But not having a need for strict mode is even better. For what it's worth I'd be willing to give up import * as stylex from "@stylexjs/stylex";
export type ExtendProps = { extend?: stylex.StyleXStyles };
export function Title({ extend, ...props }: ExtendProps & JSX.IntrinsicElements["div"]) {
return <div {...props} {...stylex.props(styles.title, extend)} />;
} Then I can simply use Anyway, if StyleX makes For this reason, the StyleSheet API, although somewhat ceremonious at times, reinforces the idea that correctness is about narrowing decisions, which I certainly appreciate. A little bit of pain up front can be worth if it if it narrows the overall complexity space of what you're trying to achieve. @nmn Does |
Beta Was this translation helpful? Give feedback.
-
I share this concern. I have received repeated requests for something on these lines and so I'm trying to gather feedback to see if makes sense. The "Constrains & Considerations" section also describes some limitations that make me way of implementing this API. @zaydek This RFC should not yet be considered as something we're about to land. This issue exists to gather feedback. There are still many reasons against doing this. The benefits would need to great outweigh those downsides.
This is another valid reason against adding this API. Although a bit subjective, I believe enforcing the usage of
This is possible and what I'm working towards. I don't want to bring this into "core" anytime soon.
Did you mean RSD? RN supports inline style objects. RSD does not support this API. |
Beta Was this translation helpful? Give feedback.
-
@nmn I actually meant RN but I didn't know RN supports inline style objects already. Also, it sounds like RSD is effectively strict mode for StyleX so that seems aligned. Thanks for clarifying this is not 'about to land' as I assumed. FWIW everytime I've tried to write inline style-based UIs first it's always come back to haunt me. It tooks some getting used to StyleX's StyleSheet approach but honestly it's really just like CSS Modules but typed and colocated. Now it's a no brainer for me to use StyleX and I love the API. So a little friction is fine and totally valid if you're trying to do something different, which StyleX is. Experiments help gather information, so perhaps there's some opportunity that I do use inline styles alongside StyleX at present, but it's simply because I'm too lazy. It's probably only 2-5% so it doesn't really matter either way. I genuinely appreciate there are so few ways to do things in StyleX, as this tweet reads:
This is very good 'north star' for StyleX. |
Beta Was this translation helpful? Give feedback.
-
I have been using stylex for a long time,To be honest, the current method is too cumbersome for react, So i mad a babel-plugin for this problem. test-case You might notice i'm using inspired by style9-jsx-prop |
Beta Was this translation helpful? Give feedback.
-
AFAIK. The dynamic values are very difficult to calculate, So i have to treat each attribute as a token. But i don't think is a good way. I try to merge static token to a whole object until the token meet dynamic or |
Beta Was this translation helpful? Give feedback.
-
<div {...stylex.props({display: 'flex', flexDirection: 'column'}, ...)}>
...
</div>
const flex = stylex.inline({display: 'flex'})
const flexCol = stylex.inline({flexDirection: 'column'})
<div {...stylex.props(flex, flexCol)}>
...
</div> I feel like 2 can be solved, if only 1 is allowed. if you want to reuse styles, then you should put them in a |
Beta Was this translation helpful? Give feedback.
-
|
Beta Was this translation helpful? Give feedback.
-
Is the proposed name (
No need for a new prop name, we just pass styles down using the |
Beta Was this translation helpful? Give feedback.
-
removed to keep this RFC concise |
Beta Was this translation helpful? Give feedback.
-
@zaydek This may be of interest to you: https://gist.github.com/nmn/b98c21fbf3ee02c35319940b14030f62
It does. The main concern with this pattern is about detecting unused styles. JS tooling usually can't catch unused variables across module boundaries. I have some ideas to tackle and optimise this pattern, but it's on the back-burner for now. |
Beta Was this translation helpful? Give feedback.
-
In fact, I tend using import React from 'react'
import { useScale, withScale } from '../../composables'
interface Props {
inline?: boolean
}
export type SpacerProps = Omit<React.HTMLAttributes<any>, keyof Props> & Props
function SpacerComponent({ inline = false, ...props }: SpacerProps) {
const { SCALES } = useScale()
return (
<div
stylex={{
width: SCALES.width(1),
height: SCALES.height(1),
padding: `${SCALES.pt(0)} ${SCALES.pr(0)} ${SCALES.pb(0)} ${SCALES.pl(0)}`,
margin: `${SCALES.mt(0)} ${SCALES.mr(0)} ${SCALES.mb(0)} ${SCALES.ml(0)}`,
display: 'block',
...(inline && { display: 'inline-block' })
}}
{...props}
/>
)
}
SpacerComponent.displayName = 'Spacer'
export const Spacer = withScale(SpacerComponent)
import * as stylex from '@stylexjs/stylex'
const styles = stylex.create({
// ... a long style sheet
})
function Component() {
return <div {...stylex.props(styles.icon,inline({position:'static'}))}></div>
} |
Beta Was this translation helpful? Give feedback.
-
Let me explain what i did. The // input
const Component = () => {
return <div stylex={{ color: "red" }} />;
};
// output
import { create, props } from "@stylexjs/stylex";
const styles = create({
xxHash: {
color: "red",
},
});
const Component = () => {
return <div {...props(styles.xxHash)} />;
}; Back to topic. In my option is that // input
import { create, props } from "@stylexjs/stylex";
const styles = create({
test: {
// ... a long style sheet
},
});
const Component = () => {
return <div {...props(styles.test, inline({ position: "static" }))} />;
};
// output
import { create, props } from "@stylexjs/stylex";
const styles = create({
test: {
// ... a long style sheet
},
});
const _styles = create({
xxHash: {
position: "static",
},
});
const Component = () => {
return <div {...props(styles.test, _styles.xxHash)} />;
}; |
Beta Was this translation helpful? Give feedback.
-
@nonzzz Is that a custom Vite plugin? What's doing the translation for you? That's really interesting. |
Beta Was this translation helpful? Give feedback.
-
Just to follow up, @nonzzz did manage to implement |
Beta Was this translation helpful? Give feedback.
-
At the cost of performance. React components have a non-trivial performance cost. In the future, maybe the React Compiler can flatten components defined like this within a single file.
It's not just that. Tailwind can lead to large amounts of "inline" class names that make the structure and behavior of the tree very hard to read, because the elements and events are lost within a sea of styles. This is less often a problem with Flutter and SwiftUI code, because they provide components for features that are part of the CSS API on web. And frankly we tend to implement more complex, responsive components on web too. Styles - like event handlers - are generally more portable and readable when they're not defined inline on elements. The
How is that going to work for custom components, which may or may not use We see a similar problem in the existing RN ecosystem, where a change to the |
Beta Was this translation helpful? Give feedback.
-
I took some encouragement from this reply and wanted to double down on a simple approach: https://gist.github.com/zaydek/6a3abb586c6bb82405d4f4b9c4ce7ee2.
If anyone's struggling with wanting to be able to more easily define inline styles in StyleX and doesn't want to depend on plugins, I've been using a simplified version of the This is simpler than my previous implementation because it doesn't hard code what values you can use and defers to the export const u = stylex.create({
accentColor: (value: CSSProperties["accentColor"]) => ({ accentColor: value }),
alignContent: (value: CSSProperties["alignContent"]) => ({ alignContent: value }),
alignItems: (value: CSSProperties["alignItems"]) => ({ alignItems: value }),
alignSelf: (value: CSSProperties["alignSelf"]) => ({ alignSelf: value }),
alignTracks: (value: CSSProperties["alignTracks"]) => ({ alignTracks: value }),
...
}); Looks like this, you probably always want to specify <header {...stylex.props(dialogStyles.header, u.paddingBlockEnd(0))}>
<div {...stylex.props(dialogStyles.hero, u.alignSelf("center"))} />
<h1 {...stylex.props(typographyStyles.title, u.textAlign("center"))}>
Are you sure you want to continue? This action is irreversible.
</h1>
</header> I found StyleX to be a lot easier having something like this in my back pocket, particularly for prototyping and scaffolding ideas before I'm ready to name them. |
Beta Was this translation helpful? Give feedback.
-
This is worse for performance as when you use dynamic styles with functions, you're relying on inline styles to set variables in addition to setting classNames. |
Beta Was this translation helpful? Give feedback.
-
A friendly ping. @nmn This RFC hasn't been updated for a long time. FWIW, I implemented it and using for a long time. I found it very useful for simple cases. |
Beta Was this translation helpful? Give feedback.
-
@nonzzz The current decision is to not implement this in StyleX itself in order to keep the API small. But the issue remains open as a central place for people to be able to chime in with their opinions. |
Beta Was this translation helpful? Give feedback.
-
Could you make a jsx parser to pull out inline styles? like <div stylex={{
borderColor: "red"
}}>
My div
</div> that then pulls out the data, throws it into |
Beta Was this translation helpful? Give feedback.
-
Moving this to Discussions |
Beta Was this translation helpful? Give feedback.
-
Describe the feature request
Motivation
It has often been suggested that it should be possible to define styles inline. There are many reasons why such a feature might be desirable:
stylex.create
is too much ceremony for one-off stylesstylex.create
doesn't benefit for Prop type constrains when passing styles as props.Naming Bikeshed
There has been much discussion about what the function name for this API should be.
stylex.inline
- Slightly verbose, slightly unclear.stylex.atom
- Terse but unclearstylex.createOne
- Very verbose but clearstylex.make
- Short and clear, but confusing alongsidestylex.create
.I would love feedback to help make a decision here. Please leave a 👍 on one of the first four comments below to register your vote.
Proposed API
Assuming the
stylex.inline
name, the usage would still be what you would expect from a hypotheticalstylex.createOne
function. i.e. Instead of defining:You'd be able to do:
Of course, it would also be possible to use this function call directly
with a
stylex.props()
call or a component prop.Since the function accepts an object, it would be possible to define a whole object of styles.
It would be possible to apply the styles conditionally:
And it would be be possible to reduce verbosity with the import.
Constrains and Considerations
One of the biggest concerns when implementing this feature is dynamic styles.
stylex.create
enforces the usage of functions to define styles that can be dynamic. However, doing the same with the new API would be awkward.Instead, the developer expectation would be the ability to use dynamic values directly within the objects
This has two issues:
One: It is difficult to implement this correctly and detect all the ways the value of the styles may be known statically. We don't accidentally want to generate dynamic styles when the styles are actually statically know.
Two: It makes it far too easy to define dynamic styles. One of the reasons for requiring functions within
stylex.create
to define dynamic styles is that the vast majority of use-cases should not depend on dynamic styles. Instead conditional styles, and compositions should be sufficient. Requiring functions makes the need for truly dynamic styles explicit and enforces intentionality. Automatically generating dynamic styles with the new API would encourage patterns that would lead to bloated and slower styles.Therefore, the current proposal would disallow the usage of dynamic styles within the new API entirely. Using dynamic values of styles within a
stylex.inline()
(or whatever we call it) call would be a compile-time error. It would still be possible to define constants and use them, but the amount expressivity possible would be limited by the capabilities of the compiler.Reasons against implementing this API
There are also many great reasons why this API should not implemented. To name a few:
stylex.create
arguably creates more readable and maintainable code in the long run at the cost of some minor inconvenience upfront.Thoughts?
I'm posting this proposal early to gather feedback and help improve this idea before implementation.
To vote for the API name, simply react with a 👍 on one of the first four comments on this issue. For any feedback about how the API works, respond with a comment.
Beta Was this translation helpful? Give feedback.
All reactions