Extending Styles

[TaeKimJR]

It's a common use-case to re-use a component's styles but make slight stylistic changes related to your product, business, etc. react-with-styles doesn't support this... until this PR! Extending styles is a common feature in css-in-js solutions (styled-components) and provides the necessary flexibility without bloating components.

Extending styles within reason
Styles on a component should be extendable... within reason! We do not want to open up the entire className and style props on components, those flood gates should remain tightly sealed. When the component can be entirely re-styled, any internal changes to a component could break unknown external usages.

In order to allow a style to be extended, it must be defined in the extendableStyles option that gets passed to the withStyles call on the component.

const Button = withStyles(
  () => { ... },
  {
    extendableStyles: {
      container: {
        background: () => true,
        color: (value) => value === 'red' ,
        font: (value, theme) => value === theme.fontSize.medium,
      },
    },
  },
)(Component);

Any styles that are later extended but are not explicitly defined here, will throw an error (in dev).

Deeply nested extension
It is possible to extend a component that has already been extended. This way you can build up a component level by level, slowly piecing together the styles you care about.

const ButtonWithColor = Button.extendStyles(
  () => ({
    button: {
      color: 'white',
    },
  }),
);

const ButtonWithColorAndBackground = ButtonWithColor.extendStyles(
  () => ({
    button: {
      background: 'black',
    },
  }),
);

Notice that ButtonWithColorAndBackground extends upon the already extending ButtonWithColor.

API / Usage
Creating a Base Component

import { withStyles } from 'react-with-styles';

const Button = ({ css, styles }) => (
  <button {...css(styles.button)}>Click Me</button>
)

export default withStyles(
  (theme) => ({
    button: {
      background: theme.colors.primary,
      color: theme.colors.white,
    },
  }),
  {
    extendableStyles: {
      button: {
        background: (value, theme) => value === theme.colors.secondary,
      },
    },
  },
)(Button);

Extend a Component's Styles

import { withExtendStyles } from 'react-with-styles';
import Button from './Button';

export default Button.extendStyles(
  (theme) => ({
    button: {
      background: theme.colors.secondary
    }
  }),
)(Button);

Invalid Extension of Component's Styles

import { withExtendStyles } from 'react-with-styles';
import Button from './Button';

export default Button.extendStyles(
  (theme) => ({
    button: {
      // These will fail.
     // "background" value of 'purple' will not pass the predicate defined in the base component
      background: 'purple',
      // "color" is not defined in the Base Component's "extendableStyles"
      color: theme.colors.black,
    }
  }),
)(Button);

[ljharb]

This seems in conflict with the principles here - ie, the idea is that nothing but the component itself has any direct control over its own styling.

[TaeKimJR]

Spoke w/ @ljharb. Sounds like this is a reasonable approach with some tweaks (although the need to extend styles could be solved at a design level). The component would continue to maintain control over its own styling - "it’s effectively performant sugar for making explicit props to allow styles to be controlled'". 🍬

🥅 Goals

1. allow the component author to specify certain styles as overrideable/extensible
2. preserve referential identity of the styles object across renders
3. ensure nothing gets slower at render time

⚙️ Tweaks

• Move HOC-logic to a static property that gets created on the component. This way the HOC can be tightly coupled to the component itself.

🛠 Future Tooling

• Tooling to allow validation against the styles values. Component could provide a predicate function that could validate the value ('container.color': value => value === 'red' || value === 'blue'). Could be run in PropTypes.
• Tooling to encourage best practices: no component wrapping within render, immutable extending styles (===),

[lencioni]

I'd love to get @janpaul123's perspective on this!

[TaeKimJR]

@ljharb @lencioni
I've updated the API based on @ljharb 's awesome suggestions! We now tightly couple the extending styles function w/ the component itself. This update also simplified a ton of code.

New API:

import Button from './Button';

const PrimaryButton = Button.extendStyles((theme) => {
  button: { background: theme.colors.primary },
});

[TaeKimJR]

I ran some performance tests to compare master vs this PR. My performance tests measured creating and rendering 10,000 components in various manners. There is a minimal perf difference between the two branches and between using withStyles and extendStyles.

Given these results, I feel that the goals have been met.

(on master)
Creating and rendering a Component wrapped in withStyles with the same style
average total: 1999 ms

Creating and rendering a Component wrapped in withStyles with a unique style
average total: 2185.2 ms

(on this branch)
Creating and rendering a Component wrapped in withStyles with the same style
average total: 2064 ms

Creating and rendering a Component wrapped in withStyles with a unique style
average total: 2448.4 ms

Creating and rendering a Component wrapped in extendStyles with the same style
average total: 2139.8 ms

Creating and rendering a Component wrapped in extendStyles with a unique style
average total: 2327.6 ms

See all results here

You can see the code I used to run the perf tests here.

[ljharb]

Let's get as many stamps on this as possible before merging it, but i think all my concerns are addressed. Thanks for iterating!

[lencioni]

It would be really great to have an eslint rule that ensures that the shape of the extendableStyles object stays in sync with the shape of the styles object.

https://github.com/airbnb/eslint-plugin-react-with-styles

[TaeKimJR]

Created a rule for the above.
airbnb/eslint-plugin-react-with-styles#37

[lencioni]

It would be good if there was a babel plugin to remove these validator functions in production builds.

[lencioni]

I know that TypeScript currently has some troubles with HOCs, but it looks like it might be getting better soon (microsoft/TypeScript#30215). Have you given any thought into how this might work with TypeScript?

[TaeKimJR]

Yeah! I think TS's ability to autocomplete and validate arguments will be super nice with extendStyles. I'm sure we could re-use a lot of the PropsType work that Brie has already setup and apply it to extendStyles argument. This should be a super nice for developers so they don't have to dig into the component to know which styles can be extended.

[noratarano]

I love this new direction. This such a needed feature. Thanks for all the work to drive this @TaeKimJR

[lencioni]

I was chatting with @majapw about this yesterday and I think it would be really valuable to sketch out what a workable compile target for static CSS would look like with this change in place. In other words, in a world where we are compiling all of react-with-styles out at build time and generating static .css files, what would the module with extendable styles end up looking like and what would the module that is extending files end up looking like?

I think @wonnage and @monokrome would be good allies on helping you sketch this out.

[lencioni]

Can you think of a good way to surface this validation before runtime (e.g. in the editor and at build time)--maybe via TypeScript or ESLint? Most linter rules don't cross module boundaries, but there are some in eslint-plugin-import that do.

[TaeKimJR]

@lencioni Thanks for the feedback! I've worked with @monokrome on building out a plan for static analysis.

I think the general gist is...

• Similar to components that have a withStyles call in a separate file, we will pass down a className through props.
• When we find a withStyles call in a separate file than the css() call, we will generate the classNames and pass them to the wrapped component via props (e.g. styles.container)
• Using this same method, we can find all extendStyles calls, generate the classNames, and pass them to the wrapped component via props (concat'n along the way).

[lencioni]

I think the main things to think through there are:

• Can we find a solution that allows each module's styles to be self-contained? In other words if Button allows container.color to be extended, and BlueButton extends it to be blue, can the built BlueButton module only contain that style and not also include all of the base styles for Button?
• If we end up using multiple classnames on the component, how will that work with CSS specificity and the ordering of styles?

[TaeKimJR]

If we end up using multiple classnames on the component, how will that work with CSS specificity and the ordering of styles?

I think we take advantage of ordering in the stylesheet here. I spoke w/ @monokrome about this, and it sounds like the order that the files are loaded into the loader should go from base -> extend -> extend, which fits our needs.

[ahuth]

LGTM 🤘

[TaeKimJR]

Can we find a solution that allows each module's styles to be self-contained? In other words if Button allows container.color to be extended, and BlueButton extends it to be blue, can the built BlueButton module only contain that style and not also include all of the base styles for Button?

If we are already generating a className for the Button, is there a loss in also applying that to the BlueButton?

I think I get what you are hinting at though. If we can generate a single className, we don't have to worry about specificity/ordering and we can de-dupe. This is would be an awesome goal, but may be a future goal (would be a behind-the-scenes enhancement).

[lencioni]

I think we take advantage of ordering in the stylesheet here. I spoke w/ @monokrome about this, and it sounds like the order that the files are loaded into the loader should go from base -> extend -> extend, which fits our needs.

I think that might not always be the case, since the base component might be built into a different chunk (and thus a different stylesheet) than the extending component and its styles. At this point the order would depend partly on how the bundler decides to split things up and the order it puts them in, and potentially in the order that they happen to be downloaded by the browser. It might be a good idea to have the safety of specificity built into the selectors themselves to avoid having to rely on the order they happen to end up in.

If we can generate a single className, we don't have to worry about specificity/ordering and we can de-dupe.

One option would be to generate a single className, but I think we should avoid that because it will cause a lot of duplicated styles in the build stylesheets. Instead, I think we should aim at something that gets the specificity correct for extending classNames, regardless of ordering.

[lencioni]

Similar to airbnb/eslint-plugin-react-with-styles#37 (comment)

Can we add some valid and invalid test cases for nested styles? Media queries, @supports queries, pseudo-elements, pseudo-selectors, etc.

[TaeKimJR]

I keep running into the same issue w/ specificity where psuedo-classes are overwritten (or not overwritten) based on ordering. I put together a fiddle to highlight my thoughts.
https://jsfiddle.net/s7drg03k/5/

Are there good ways of handling psuedo-classes?

[TaeKimJR]

Will hold-off on merging until we can solve for static analysis. There are concerns around ordering of stylesheets for chunks.

[yepitschunked]

@TaeKimJR @lencioni Some thoughts on static build:

1. Ordering of selectors within the same css call. I think this is a self-resolving problem specific to aphrodite. If you were writing normal css, you would just be concatenating classnames anyway, and those classnames would have to be properly ordered/scoped. e.g, in the pre-aphrodite world, you'd have <button className={cx('button', primary && 'primary')} /> and the order of those classnames shouldn't matter. In aphrodite, you have to order them as css(styles.button, styles.primary) to avoid merging the objects the wrong way. Therefore, the order that people have written styles today is actually in precedence order already. I can't think of a situation where someone would write css(styles.foo, styles.bar) on one element and css(styles.bar, styles.foo) on another, assuming that foo and bar override each other. If the styles don't intersect, then the order doesn't matter anyway. This means that it's possible to order the CSS when generating it by inspecting the css calls (i.e, we know that foo will always come before bar).

2. Ordering of selectors across files. My hunch is this might not be a problem. For context, static styles will be fetched by webpack via injecting link tags. This show up right before the existing script tags that are already being injected. I think that webpack inserts tags for dependencies in reverse order (i.e, children will appear before parents), so their CSS will also be in the correct order. I could be wrong about this though.

3. Handling extensible styles (this PR). I think we should meet in person on this, we would need to have classname information from the base component available when processing the extending component, which might be tricky.