yandex / reshadow

Markup and styles that feel right
https://reshadow.dev
Other
363 stars 15 forks source link

Allow implicit direct root styles #37

Open kizu opened 5 years ago

kizu commented 5 years ago

Right now we need to target each element explicitly.

const Button = ({children, styles, ...props}) => styled(styles)`
    button {
      border: 1px solid;
    }
`(
    <button {...props}>
        <content as="span">{children}</content>
    </button>
);

This makes it a bit cumbersome to restyle such component from outside (you need to mention the inner root component name explicitly):

<Button styles={css`button { background:red; }`}>
    click me
</Button>

My proposal: allow direct styles without a mention of the component name:

const Button = ({children, styles, ...props}) => styled(styles)`
  border: 1px solid;
`(
    <button {...props}>
        <content as="span">{children}</content>
    </button>
);

And then the “inline” styles become also much simpler:

<Button styles={css`background:red;`}>
    click me
</Button>

Other than reduced amount of code and more simple API for more simple components, this would be similar to how it is done in some other CSS-in-JS solutions like styled-components, making it easier for people to migrate.


Another similar feature that could be added: :root selector, which could be used to target the root element of our component. This would make it easier to use it as an explicit selector, but also without a need to know which element you need to target.


Some notes:

  1. Mixed declarations and rules: any declarations without a selector above them should be treated as wrapped with :root(), and rules alongside shouldn't be treated as nested inside the :root() unless they have a &.

  2. Both :root and implicit root styles should apply only to the one and only root component. So, in case there are some root styles, but then in HTML there is either a fragment or an array, those styles shouldn't be applied to anything. This would make the implementation easier and less confusing for such use-cases.

  3. While allowing multiple ways to do the same can be bad for consisting code, this can be enforced via a linting rules: allowing/disallowing direct root styles and/or :root ones.

lttb commented 5 years ago

thank you for the great proposal!

the one thing I'm not sure about is the second note about fragments

at this moment, we already support fragments with dynamic values, like

styled`
  button { color: ${color}; }
`(<><button /><button /></>)

and I don't see any caveats with implementation of root styles too here

but the semantics of root element is not really clear for fragments

maybe it would be also useful to add a linting rule to disallow the root styles for fragments? but support them out of the box?

kizu commented 5 years ago

When talking about fragments/array, it is more of “there can be always only one root element”, and then I'm thinking of a fragment in that case as more of an HTML element with display: contents, which would be an actual root, but as it renders to nothing, then anything applied to it won't matter (this is not exactly the same as with display: contents due to CSS inheritance, but I think the comparison is close).

And another moment: in the fragments and arrays there could be text nodes, and those won't get those root styles in any case.

Basically, for me targeting anything other than exact root looks confusing and very edge-casey, and feels like while it can be implemented to the array/fragment children, it wouldn't be that useful.

lttb commented 5 years ago

agreed

there is also an interesting edge-case:

styled`
  color: red;
`(
  'hello world'
)

or

styled`
  color: red;
`(
  children
)

it seems that we should also ignore styles here

kizu commented 5 years ago

Yep, if we do not know what is it there, we can ignore it.