stylex icon indicating copy to clipboard operation
stylex copied to clipboard

[RFC] Defining styles inline

Open nmn opened this issue 2 months ago • 23 comments

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:

  1. Defining styles with stylex.create is too much ceremony for one-off styles
  2. stylex.create doesn't benefit for Prop type constrains when passing styles as props.
  3. Developer preference for more co-location.

Naming Bikeshed

There has been much discussion about what the function name for this API should be.

  1. stylex.inline - Slightly verbose, slightly unclear.
  2. stylex.atom - Terse but unclear
  3. stylex.createOne - Very verbose but clear
  4. stylex.make - Short and clear, but confusing alongside stylex.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 hypothetical stylex.createOne function. i.e. Instead of defining:

const styles = stylex.create({
  base: {color: 'red'}
});

You'd be able to do:

const red = stylex.inline({color: 'red'})

Of course, it would also be possible to use this function call directly with a stylex.props() call or a component prop.

<div {...stylex.props(
  stylex.inline({color: 'red'})
)} />;

<MyButton styles={stylex.inline({color: 'red'})} />;

Since the function accepts an object, it would be possible to define a whole object of styles.

stylex.inline({
  color: 'red',
  backgroundColor: 'black',
  textDecoration: {
    default: 'none',
    ':hover': 'underline',
    ':focus-visible': 'underline',
  },
  ...
})

It would be possible to apply the styles conditionally:

<MyButton styles={[
  isRed && stylex.inline({color: 'red'}),
  isBlue && stylex.inline({color: 'blue'}),
]} />;

And it would be be possible to reduce verbosity with the import.

import {inline as _} from '@stylexjs/stylex';

<MyButton styles={[
  isRed && _({color: 'red'}),
  isBlue && _({color: 'blue'}),
]} />;

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.

<MyButton styles={stylex.inline((color) => ({color}))(dynamicColor)} />;

Instead, the developer expectation would be the ability to use dynamic values directly within the objects

<MyButton styles={stylex.inline({color: dynamicColor})} />;

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:

  1. Increased maintenance burden - A bigger API is harder to maintain takes focus away from other improvements.
  2. Increased API surface area - Makes StyleX harder to understand
  3. Leads to less readable code - Enforcing all styles be defined in stylex.create arguably creates more readable and maintainable code in the long run at the cost of some minor inconvenience upfront.
  4. The lack of dynamic styles in the API could become a pitfall.

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.

nmn avatar Apr 11 '24 03:04 nmn