facebook / stylex

StyleX is the styling system for ambitious user interfaces.
https://stylexjs.com
MIT License
8.34k stars 307 forks source link

[RFC] Descendent and Sibling selectors #536

Closed nmn closed 3 weeks ago

nmn commented 5 months ago

Describe the feature request

Disclaimer

This proposal is not "ready" or "approved". This proposal is being shared publicly to help consolidate all discussions about adding such a feature.

Motivation

StyleX has a few constraints in service of making things work consistently and generating styles that are performant and scalable. StyleX has disallowed descendent and sibling selectors to maintain it's core principles such as:

However, the alternative to such selectors is to use Javascript which may often not be desirable and cause other trade-offs.

Constraints

Any design we adopt for descendent selectors must not compromise on StyleX's core principles.

We must also ensure that such a feature won't significantly affect the performance or size of the generated CSS.

Core Concept

When it comes to avoiding styling at a distance, .csuifyiu:hover > div is unsafe, but div:hover > .ksghfhjsfg is safe.

Inspiration

group and peer from Tailwind.

Proposal

This proposal would add at least two new functions to the StyleX API surface. Although the names could change, for the sake of this explainer, assume they're stylex.id and stylex.when.

The core concept is that stylex.when can be used toe read the state of an element targeted with stylex.id and apply styles conditionally. stylex.when would be allowed to be used where pseudo classes are allowed today.

stylex.id would have the same constraints as stylex.defineVars.

// target.stylex.ts
import * as stylex from '@stylexjs/stylex';
export const myId = stylex.id();
import {create, props, when} from '@stylexjs/stylex';
import {myId} from './target.stylex';

const styles = create({
  button: {
    color: {
      default: 'black',
      ':hover': 'red',
      
      [when.ancestor(myId, ':hover')]: 'blue',
      // .a-group:hover .xjygtsyrt
      [when.peerBefore(myId, ':hover')]: 'blue',
      // .a-group:hover + .xjygtsyrt
      [when.peerAfter(myId, ':hover')]: 'blue',
      // .xjygtsyrt:has(+ .a-group:hover)
      [when.descendent(myId, ':hover')]: 'blue',
      // .xjygtsyrt:has( .a-group:hover)
    },
  }
});

<Card {...props(styles.card, ...,  myId)}>
  <button {...props(styles.button} />
</Card>

Additionally, stylex.id.default should be available as a "default" id that can be used instead of creating ids manually.

Concerns

CSS Bloat

Adding this capability can lead to CSS becoming bloated.

One way to minimise bloat would be disallow creating custom ids and enforce that the same id is used for all use-cases. This would cause the same constraints as group and peer in Tailwind where there can only be a single "layer" of combinator styles at a time. This should be sufficient in the vast majority of use-cases and we can suggest using JS for the edge-cases

API Bloat

This API makes StyleX harder to learn and increases the API surface area.

Naming Bikeshed

The naming of the APIs is up for debate.

Feedback

How important is this feature?

Is the ability to create multiple custom IDs for targeting in styles important?

aspizu commented 5 months ago

What happens when multiple parent nodes use the same id ?

zaydek commented 5 months ago

Would it be possible to support an API like:

const id = stylex.id()

...

[`${id}:hover &`]: {
  // ...
}

I would think there could be a couple of valid string interpolations the user could do, and I think these are possible to have typed in TypeScript because you can make expressions like ${string}:hover & and so on. I understand that perhaps you want to limit users from being able to put whatever they want there, but you can always have StyleX error on illegal expressions.

That being said, I can respect that you are trying to limit the API surface area as much as possible by relying on explicit, imported function calls rather than magical strings.

One other thing is I'd prefer if I don't need to create n+1 files everytime I want to do this kind of thing (+1 file for .stylex.ts). Perhaps this is a limitation of the Babel pipeline or something but I think one of the strongest principles of StyleX is colocation, so the decision to force users to having an extra .stylex.ts file around for these one offs isn't personally appealing to me.

Right now when I need to implement this kind of pattern, I use the CSS variable hack as alluded to in other threads. The one benefit this has over the ID approach is that I don't need to create a .stylex.ts file to adopt this pattern.

There is a real need for this kind of pattern overall and relying on JavaScript and event listeners is unwieldy and CSS variables feels like a work around.

nmn commented 5 months ago

What happens when multiple parent nodes use the same id ?

If two anscestor have the same id, then if either one matches the given pseudo-class, then the style would be applied. In practice, this is usually not a problem:

  1. With :hover if a child is hovered, then the parent is hovered anyway
  2. Same with :focus-within
  3. Same with :active
  4. With :focus, it's rare to have nested focusable elements and even rarer to need IDs on two parent focusable elements.

Most other pseudo-classes don't even make sense as something to observed IMO.

The use-case for multiple IDs is mostly specific to peers or descendents (using :has()).


@zaydek I considered an API with strings instead of function calls. There are a few reasons I don't prefer this.

  1. Even though TS has template string types, Flow doesn't and that's what we use internally
  2. Template Literal types are considerably slower at type-checking
  3. A generic string implies the ability to use arbitrary selectors and we want to explicitly limit the selectors that can be used.

One other thing is I'd prefer if I don't need to create n+1 files everytime I want to do this kind of thing

We might loosen the requirement for a .stylex.ts file in the future when values are not being exported and are only being used locally within a file. For this proposal, however, I think the extra indirection adds value as we want to discourage the use of custom IDs as much as possible. Custom IDs will generally result in unique CSS that will not be shared and we want to push the usage of the default ID if at all possible.

However, whether we loosen the requirement for .stylex.ts files is a separate discussion that we can have.

The one benefit this has over the ID approach is that I don't need to create a .stylex.ts file to adopt this pattern.

Officially, you need a .stylex.ts file to use the variable hack as well. The fact that string variables work is an escape hatch to support legacy usage internally. It's not officially supported part of the API. Variables in StyleX should be defined using stylex.defineVars.

zaydek commented 5 months ago

@nmn Got it, thanks. Other quick questions -- is when composable? I assume not but is it explicitly disallowed that you can do when.peerAfter(when.peerBefore(id, ":hover"), ":focus") or something zany like that? Not saying anyone would but if you haven't already, you might want to hard code some errors for these use cases. I say that because I've implemented anti patterns in the past without knowing, so having stricter errors up front would have prevented that. 🙂

Besides that I'm on board with this. I find when.ancestor slightly confusing because it's not the ancestor of the ID, it's the ID that is the ancestor in this case. Maybe when.is and when.has (instead of ancestor and descendent) would make this distinction more clear?

nonzzz commented 5 months ago

Too much freedom will lead to additional learning costs. And i noticed @zaydek mentioned whether when is composable. I think this is something that needs to be made clear. Repeating definitions creates extra work. Using API to define css selector is more friendly to javaScript string template. In my option, string template is hard to maintain and it is not friendly to the compiler.

keeganstreet commented 5 months ago

Examples are provided for the ancestor (descendant combinator), e.g. a-group:hover .xjygtsyrt, and peer (next-sibling combinator) e.g. .a-group:hover + .xjygtsyrt, but not the child combinator, e.g. a-group:hover > .xjygtsyrt, or subsequent-sibling combinator, e.g. a-group:hover ~ .xjygtsyrt.

Is this just for brevity of the examples or were you thinking these selectors conflicted with the constraints somehow?

keeganstreet commented 5 months ago

How about something like this? It prevents styling at a distance (you can still only target elements that you can set a classname on), offers the flexibility for additional selectors without expanding the API, avoids template string styles (apart from the $) placeholder. Not quite sure how the has() psuedo-class could fit in though.

import {create, props, combinator, child, descendant, nextSibling, subsequentSibling} from '@stylexjs/stylex';
import {myId} from './target.stylex';

const styles = create({
  button: {
    color: {
      default: 'black',
      ':hover': 'red',

      [combinator(myId, ':hover', child, '$')]: 'blue',
      // .a-group:hover > .xjygtsyrt
      [combinator(myId, ':hover', descendant, '$')]: 'blue',
      // .a-group:hover .xjygtsyrt
      [combinator(myId, ':hover', nextSibling, '$')]: 'blue',
      // .a-group:hover + .xjygtsyrt
      [combinator(myId, ':hover', subsequentSibling, '$')]: 'blue',
      // .a-group:hover ~ .xjygtsyrt
    },
  }
});

<Card {...props(styles.card, ...,  myId)}>
  <button {...props(styles.button} />
</Card>
nmn commented 5 months ago

is when composable?

@zaydek It's not intended to be.

I say that because I've implemented anti patterns in the past without knowing, so having stricter errors up front would have prevented that

Thanks for this feedback. Having stricter errors is definitely on our roadmap.

Maybe when.is and when.has (instead of ancestor and descendent) would make this distinction more clear?

when.ascestor(id, ':hover'), is supposed to read like: "When an anscestor element with ID id is hovered, then this element gets the style value:" is and has don't make sense in the same way to me.

@nonzzz I hope your concerns/questions have been answered above as well.

but not the child combinator... or subsequent-sibling combinator,

@keeganstreet This is an intentional part of the design. It all flows from the core principles:

You explicitly set an ID on elements you want to observe, and then you observe them positionally in the general way possible. Using the more general combinators is enough achieve all use-cases with fewer unique CSS rules.

offers the flexibility for additional selectors

We don't want the additional flexibility. We want the most constraints while fulfilling use-cases. Please present me use-cases where you need the additional complex selectors.

predaytor commented 1 month ago

Is there some progress? Really hope to see it in some form soon. Thx.

nmn commented 3 weeks ago

@predaytor Sorry for the late reply. I was on a vacation. This work is currently on the back-burner as there isn't an obvious good API for this feature yet.

We are continuing to iterate on the right design for now.

necolas commented 3 weeks ago

Moving to Discussions